Bug 40799

Summary: Web Inspector: bring XHR console records back.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, joepeck, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] That I am currently compiling (still worth reviewing - what if it compiles!)
none
[PATCH] Same with no monitorEvents("xhr") support. yurys: review+

Description Pavel Feldman 2010-06-17 14:21:51 PDT
Here is what I plan to do:
1) make (un)monitorEvents("xhr") enable / disable monitoring
2) add "Enable XHR Monitor" / "Disable XHR Monitor" actions to the console's context menu
3) Make the chosen option persistent in the settings.

Patch to follow.
Comment 1 Pavel Feldman 2010-06-17 14:36:01 PDT
Created attachment 59035 [details]
[PATCH] That I am currently compiling (still worth reviewing - what if it compiles!)
Comment 2 Joseph Pecoraro 2010-06-17 14:51:05 PDT
(In reply to comment #0)
> 2) add "Enable XHR Monitor" / "Disable XHR Monitor" actions to the console's context menu

How about also adding a status bar item (always visible), or
something in the scope bar (only visible in the dedicated
console panel)?

Looks like this could use a test!
Comment 3 Pavel Feldman 2010-06-17 14:58:00 PDT
(In reply to comment #2)
> (In reply to comment #0)
> > 2) add "Enable XHR Monitor" / "Disable XHR Monitor" actions to the console's context menu
> 
> How about also adding a status bar item (always visible), or
> something in the scope bar (only visible in the dedicated
> console panel)?
> 

I'd hate to see something that specific in the UI. We are doing this feature for users that really need it, but there are not too many use-cases involving it. I am also thinking of explicitly evaluating monitorEvents("xhr") in the console upon this menu action so that users could do it by hand later...

> Looks like this could use a test!

Lets agree on landing it first. And... I am not accepting requests like this from you since you've been way more lazy on test coverage than me lately :P
Comment 4 Pavel Feldman 2010-06-18 02:26:35 PDT
Created attachment 59088 [details]
[PATCH] Same with no monitorEvents("xhr") support.
Comment 5 Yury Semikhatsky 2010-06-18 03:24:39 PDT
Comment on attachment 59088 [details]
[PATCH] Same with no monitorEvents("xhr") support.

WebCore/inspector/InspectorController.cpp:1109
 +          m_monitoringXHR = true;    
Maybe just m_monitoringXHR = (monitoringXHR == "true") ?
Comment 6 Pavel Feldman 2010-06-18 05:27:46 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorBackend.h
	M	WebCore/inspector/InspectorBackend.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/InspectorBackendStub.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebCore/xml/XMLHttpRequest.cpp
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/js/InspectorControllerImpl.js
Committed r61397