Bug 54112 – Web Inspector: Add Start/Stop commands for (sub-)frontend lifetime agents.

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

[Pavel Feldman]

I'd like to convert protocol from:

[domain=Inspector] void startTimelineProfiler();
[domain=Inspector] void stopTimelineProfiler();

To

[domain=Timeline] void start();
[domain=Timeline] void stop();

I'd also like to make following agents create lazily:

DOM, Runtime, DOMStorage, Database, AppCache agents

They will have start() and stop() methods as well, but will start on demand after calling one of their methors. stop() will make them stop sending notifications.

[Patrick Mueller]

You're talking about Inspector.idl, right?

There's may be a semantic issue here, in that you will have multiple "start" and "stop" methods defined in the IDL.  I realize that the Inspector.idl isn't strictly WebIDL, but it clearly is very close.  Does WebIDL support this kind of "overloading" in interfaces?  (I hope not!)

Alternatives:

- split Inspector.idl into separate IDL files, each with their own interface

- keep everything in Inspector.idl, but put multiple interfaces in it, instead of the singular interface "Inspector" today.  Obviously, every domain becomes a new interface.

I should mention that I'm also a consumer of Inspector.idl for weinre.  Part of my build process is to build a JSON representation of the IDL, so that I can build proxies and verify methods exist in implementations.  So I'd be hit by either of these changes.  But I'd love to see either of these changes over the current state of Inspector.idl today - I don't think it's a big hit to change my JSON-builder (which parses the IDL).  I'd prefer the second approach, I think.

I should mention also that I can actually cope with the original suggested change (multiple "start" methods in the IDL), it just doesn't seem like the cleanest story.

[Pavel Feldman]

Filed https://bugs.webkit.org/show_bug.cgi?id=54135 for the IDL part.

[Pavel Feldman]

Ok, now that IDL is split we can go on with this one.

Originally, I was pushing for agent lifetime being < than frontend lifetime. I.e. agent was created upon request from the front-end and then was destroyed once it was not needed upon stop. Timeline agent existed only between the start and stop calls, debugger agent only existed while debugging was enabled, etc. The idea is nice, but I now think it overcomplicates things for no good reason:

- Now that we want to make sart and stop a part of the domain itself, timeline agent should somehow create itself upon start and wipe itself out form its inspectorAgent container upon stop. To achieve this we either need to make start / stop static and allow agents register themselves in InspectorAgent or invent some other wipe-out mechanism that is going to add nothing but indirection. It sounds kinda complex.
- Originally, we were thinking "agent lifetime spans within frontend, should we need some instrumentation outside these bounds, instrumented data should be stored in some managers / storages that live in inspector agent". However, this is bad for following reasons: a) it provides coupling between InspectorAgent and subagents and b) we were lazy and there are InspectorCSSAgent, InspectorConsoleAgent that live as much as the page c) InspectorDatabase with its in-agent storage looks ugly.
- I really think we should nuke inspector agent -> subagent dependency in order to make addition of agents modular, with no need to modify core InspectorAgent structure.


It all adds up and here is another plan that I would suggest:
[PLAN B]

- Sub-agents (or now first-class-citizen agents)'s lifetime == InspectorController lifetime.
- They all share base class with the following API: setFrontend and clearFrontend.
- They live in a single Vector of pointers, InspectorController only knows their base API above, they are constructed in the InspectorController constructor. Exception is agents that should be called upon WebKit calls such as startUserInitiatedProfiling. They are both referenced in vector (owned) and in raw pointer of a more concrete class in InspectorController.
- InspectorInstrumentation is mapping page to InspectorAgent today. Instead, it will map page to the struct of "active agents". You can think of this struct as of a reduced InspectorAgent (without ownership of agents).
- Agents receive start / stop calls from the front-end and upon these calls, they register themselves in the inspector instrumentation as "active".
- There are inter-agent dependencies (BrowserDebugger -> Debugger, InspectorAgent -> DomAgent). Let agents receive their dependencies in the constructor for now.
- To handle didCommitLoad gently, InspectorInstrumentation will have two methods:
didCommitLoad and didCommitLoadForMainFrame. ResourceAgent will be called in the first, rest of the agents in the second. They all will have didCommitLoadForMainFrame in their APIs.
- Workers are likely to have their own instrumentation interface. They are free to reuse debugger agent and a part of its protocol, they will not expose databases (we can always explore databases from the main thread), they are likely to use their own network instrumentation.

[Yury Semikhatsky]

Created attachment 82963 [details]
Preliminary patch

This patch makes DOM and Console agent always exist and introduces NativeAgents class which is supposed to hold all agents that are currently used for WebCore instrumentation.

[Ilya Tikhonovsky]

Comment on attachment 82963 [details]
Preliminary patch

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

> Source/WebCore/inspector/InspectorAgent.cpp:427
>      m_cssAgent->setDOMAgent(0);

I think we can this line and the same call in InspectorAgent::setFrontend.

[Yury Semikhatsky]

After a series of changes all agents exist while inspected page exists and each agent handles its own start/stop commands. Closing this bug.