Web Inspector: extract Inspector Instrumentation API as a class Remove references to InspectorTimelineAgent from WebCore.
Created attachment 69777 [details] Patch.
Created attachment 69783 [details] Patch. Comments addressed.
(In reply to comment #2) > Created an attachment (id=69783) [details] > Patch. > > Comments addressed. Wrong patch, sorry.
Comment on attachment 69777 [details] Patch. looks good to me
Comment on attachment 69777 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=69777&action=review > WebCore/dom/Node.cpp:2605 > + int instrumentationCookie = InspectorInstrumentation::willDispatchEvent(document(), *event, targetForWindowEvents, this, ancestors); Nit: InspectorInstrumentationCookie cookie? > WebCore/inspector/InspectorInstrumentation.cpp:313 > + if (InspectorTimelineAgent* timelineAgent = inspectorController->m_timelineAgent.get()) { Nit: I think it'd be better to have overloaded function for retrieving timelineAgent. like retrieveTimelineAgent(InspectorController*)
Created attachment 69953 [details] Comments addressed.
Comment on attachment 69953 [details] Comments addressed. View in context: https://bugs.webkit.org/attachment.cgi?id=69953&action=review > WebCore/ChangeLog:27 > + (WebCore::InspectorInstrumentation::willInsertDOMNodeImpl): You should nuke these scary verbose lines. > WebCore/bindings/v8/V8Proxy.cpp:414 > + InspectorInstrumentation::didEvaluateScript(m_frame, cookie); I think cookie should be the first parameter in all the calls. > WebCore/inspector/InspectorInstrumentation.cpp:-67 > - if (!inspectorController->hasFrontend()) Why did these go? Please explain in order to get r+. > WebCore/inspector/InspectorInstrumentation.h:116 > + static InspectorInstrumentationCookie willCallFunctionImpl(InspectorController*, const String& scriptName, int scriptLine); This is crazy amount of methods, really easy to make a mistake here. Can we cut the number?
> > > WebCore/inspector/InspectorInstrumentation.cpp:-67 > > - if (!inspectorController->hasFrontend()) > > Why did these go? Please explain in order to get r+. This checks were moved to InspectorInstrumentation::inspectorControllerForPage method.
Comment on attachment 69953 [details] Comments addressed. View in context: https://bugs.webkit.org/attachment.cgi?id=69953&action=review > WebCore/inspector/InspectorInstrumentation.h:61 > + static void willSendXMLHttpRequest(ScriptExecutionContext*, const String& url); LGTM given we migrate to a macro shortly.
Committed r69283: <http://trac.webkit.org/changeset/69283>
This change broke https://bugs.webkit.org/show_bug.cgi?id=47173 on all platforms.
(In reply to comment #11) > This change broke https://bugs.webkit.org/show_bug.cgi?id=47173 on all platforms. Oops, copy&paste error. This change broke http/tests/inspector/console-xhr-logging.html on all platforms.
(In reply to comment #12) > (In reply to comment #11) > > This change broke https://bugs.webkit.org/show_bug.cgi?id=47173 on all platforms. > > Oops, copy&paste error. > > This change broke http/tests/inspector/console-xhr-logging.html on all platforms. Fixed in r69286.