Bug 46614 – Web Inspector: extract Inspector Instrumentation API as a class.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Ilya Tikhonovsky]

There is a lot of places where we push information from WebCore to Inspector.
When we doing that we use Timeline agent, Debugger agent, InspectorController etc.
In some cases we need to have an instrumentation for Debugger agent and Timeline agent in the same place.
As example for XHR breakpoints.

I'd be great to extract an Inspector Instrumentation API which hide internal Inspector things from the rest of WebCore code. 

In this case we will have:
1) InspectorInstrumentation class as an API for WebCore;
2) inspector agents with business logic for different inspector's activities;
3) InspectorBackendDispatcher class as the api for front-end;
4) InspectorController as the controller for all Inspector stuff.

We already have last three things and want to have the first one.

I've plan to make next patches:
1) split Timeline Agent implementation. The first part will be used as a base for InspectorInstrumentation class in the following patches, the second is InspectorTimeline agent itself.
2) extract API part of InspectorTimeline agent into separate file and rename it to InspectorInstrumentation. A lot of places in WebCore will be fixed too.
3) move current implementation of DOM breakpoints in WebCore under control of InspectorInstrumentation.
4) move other things under control of InspectorInstrumentation too.

[Pavel Podivilov]

Created attachment 69183 [details]
Patch

[Pavel Podivilov]

Created attachment 69184 [details]
Patch.

[Early Warning System Bot]

Attachment 69184 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4216011

[Csaba Osztrogonác]

(In reply to comment #3)
> Attachment 69184 [details] did not build on qt:
> Build output: http://queues.webkit.org/results/4216011

You missed to add the new source and header to 
SOURCES and HEADERS variable of WebCore/WebCore.pro:
inspector/InspectorInstrumentationAgent.cpp
inspector/InspectorInstrumentationAgent.h

[Pavel Feldman]

Comment on attachment 69184 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=69184&action=review

> WebCore/CMakeLists.txt:1109
> +    inspector/InspectorInstrumentationAgent.cpp

InspectorInstrumentation

> WebCore/inspector/InspectorInstrumentationAgent.h:60
> +    static bool hasFrontends() { return s_frontendCounter; }

inline?

[WebKit Review Bot]

Attachment 69184 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4193006

[Pavel Podivilov]

Created attachment 69187 [details]
Patch.

[Pavel Podivilov]

(In reply to comment #4)
> (In reply to comment #3)
> > Attachment 69184 [details] [details] did not build on qt:
> > Build output: http://queues.webkit.org/results/4216011
> 
> You missed to add the new source and header to 
> SOURCES and HEADERS variable of WebCore/WebCore.pro:
> inspector/InspectorInstrumentationAgent.cpp
> inspector/InspectorInstrumentationAgent.h

Thanks, Csaba!

[Csaba Osztrogonác]

(In reply to comment #7)
> Created an attachment (id=69187) [details]
> Patch.

Didn't you want to mark this patch with r? flag?

[Pavel Podivilov]

(In reply to comment #9)
> (In reply to comment #7)
> > Created an attachment (id=69187) [details] [details]
> > Patch.
> 
> Didn't you want to mark this patch with r? flag?

Nope, I just want to make sure builds are green and commit, since I have r+ already.

[Csaba Osztrogonác]

(In reply to comment #10)
> Nope, I just want to make sure builds are green and commit, since I have r+ already.

EWS bots build only r? marked patches.

[WebKit Review Bot]

Attachment 69187 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4192010

[Pavel Podivilov]

Created attachment 69197 [details]
Fix linux build.

[Early Warning System Bot]

Attachment 69197 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4155011

[Pavel Podivilov]

Created attachment 69200 [details]
Patch.

[Eric Seidel (no email)]

Attachment 69197 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4198006

[WebKit Review Bot]

Attachment 69197 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4172008

[Pavel Podivilov]

Created attachment 69216 [details]
Patch.

[Pavel Podivilov]

Committed in r68767.

[Eric Seidel (no email)]

Comment on attachment 69184 [details]
Patch.

Cleared Pavel Feldman's review+ from obsolete attachment 69184 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Eric Seidel (no email)]

Comment on attachment 69200 [details]
Patch.

Cleared Pavel Feldman's review+ from obsolete attachment 69200 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Csaba Osztrogonác]

Patch landed in r68767, so the bug is resolved/fixed.

[Pavel Podivilov]

Created attachment 69775 [details]
Patch.

Remove references to InspectorTimelineAgent from webcore.