Bug 51250 – Web Inspector: introduce enabled/disabled flag for each domain handler

• Component: Web Inspector (Deprecated)
• Status: RESOLVED INVALID

[Ilya Tikhonovsky]

Now each function belongs to a domain and we have a protocol which will be used by third party clients like ChromeDevTools plugin for Eclipse.
As far as such clients use only small subset of commands we want to have a way to enable required subset of protocol functions explicitly.

[Ilya Tikhonovsky]

Created attachment 76880 [details]
[patch] first version.

[Ilya Tikhonovsky]

Comment on attachment 76880 [details]
[patch] first version.

wrong patch

[Ilya Tikhonovsky]

Created attachment 76882 [details]
[patch] initial version

[Pavel Feldman]

Comment on attachment 76882 [details]
[patch] initial version

This looks good, except for... I'd rather start with domain agents refactorings to make them independent and switchable on/off. Otherwise you risk ending up with a nice flag that means nothing (i.e. can't be backed up with proper implementation). Like turning off "Backend" is impossible. Or having CSS with no DOM is impossible. Give me some time to think about it please.

[Ilya Tikhonovsky]

(In reply to comment #4)
> (From update of attachment 76882 [details])
> This looks good, except for... I'd rather start with domain agents refactorings to make them independent and switchable on/off. Otherwise you risk ending up with a nice flag that means nothing (i.e. can't be backed up with proper implementation). Like turning off "Backend" is impossible. Or having CSS with no DOM is impossible. Give me some time to think about it please.

Proposed change works like a mute button.
It will not affect the data which we keep at backend side unconditionally like list of workers etc.

It can makes a problem if we have a notification method for some event and have no explicit getter for the data of such event. But it is not a problem for a frontend if it turns on the required domains immediately at connect stage. It is not a problem for WebInspector frontend at all because it turns on everything at connect stage :).

Right now we have such nice flags for Profiler and Debugger agents. These flags will be replaced with generic ones after this change.

We have no such flags for the other domains but it is not a problem for WebInspector frontend and just some amount of unnecessary CPU cycles if a domain's messages were muted by a frontend. We can fix such problems one by one for the each domain in separate patches.

I can introduce functions like enableDOMAgent, enableCSSAgent etc. but from the protocol point of view a single function for enabling/disabling the agents is much better than 12 different functions. 
1) less messages;
2) single single point of control;
3) no races between different domains if the domains have a cross dependency.
4) and the protocol will be closer to ready state a bit earlier.

[Eric Seidel (no email)]

Comment on attachment 76882 [details]
[patch] initial version

Did you mean to mark this for review?  Currently it only has cq?, not r?

[Pavel Feldman]

Comment on attachment 76882 [details]
[patch] initial version

Lets land it later, when all agents have clear lifetime.