Open Bug 1707809 Opened 4 years ago Updated 10 months ago

Turn watchedByDevTools into a more powerful structure to replace our usage of sharedData

Categories

(DevTools :: Framework, task)

task

Tracking

(Not tracked)

People

(Reporter: nchevobbe, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

At the moment we're using Services.cpmm.sharedData to store information about DevTools (e.g. what kind of targets and resources we should listen to) and have access to in both parent and content process.
We were told by the platform team that sharedData could be racy though, and even if it seems to suit us at the moment, we should find a cleaner solution.

One thing we could do is to enhance/replace BrowsingContext#watchedByDevTools to have not only a simple boolean, but a more complex structure that would hold what's in sharedData right now.

Those data would be keyed by connection prefix so we won't have trouble with different toolboxes debugging the same document/target.

This property should, as other BrowsingContext properties, be propagated to the whole BrowsingContext tree, which would set the data very early in the BrowsingContexts lives, and would be available in both parent and content process.
This would allow us to cleanup DevTools framework code and getting rid of possible race conditions.

Note that I'm talking about BrowsingContext here as it's something we're used to deal with and have existing pattern that we'd need (propagation + access on all processes), but we shouldn't exclude thinking of doing this on another object that would get us the same advantages.

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #0)

We were told by the platform team that sharedData could be racy though, and even if it seems to suit us at the moment, we should find a cleaner solution.

The racy bits were rather experienced by us.
Because of races in the parent process, we can't rely on sharedData to be the source of truth in the parent. This is why we hold a distinct copy of the data in WatcherDataHelper.jsm. This is a first point that makes this API painful.

Then, we don't control when and how the data is communicated to the content process. We typically don't know when the updates are received in the content process. This is why we have manual queries to update the data (like "DevToolsFrameParent:addWatcherDataEntry"). The is a second point, which makes us used sharedData only for one thing.

We end up using sharedData only when we receive DOMWindowCreated. This even may occur very early on, so early that there is very few data already available coming from the parent process. At the time we started working on fission support in DevTools, sharedData were one of the only data already available. But we could be using any other means to convey the data.

That brings us to expose a method on BrowsingContext to store and communicate DevTools data to all meaningful BrowsingContext.
These method would replicate DevTools JS API " addWatcherDataEntry" and "removeWatcherDataEntry".
Ideally, they would only resolve after the data is processed by all remote processes.
We would have to ensure that the data would be available very early during the BrowsingContext startup, as early as DOMWindowCreated fires.
And this API would certainly be a reliable source of truth both in parent and content processes.

This would probably help getting rid of WatcherDataRegistry

The main challenge of this work would be to accomodate with content process debugging.
We are also using sharedData to communicate data for the content process targets. So that we could just drop sharedData by only working on frame/browsing-context targets.
Today, they are still using content process message managers. A first step to clarify things might be to migrate them to JSProcessActors (bug 1648499).
Then, we may be able to also expose a watchedByDevTools flag on nsIDOMProcessParent and do the exact same thing for processes.

Another challenge might be that BrowsingContext data isn't transferred to the content process early enough. i.e. before DOMWindowCreated?
But that sounds unlikely. We probably have to transfer some attribute before loading the document.
It may rather be a challenge for content processes. It is a known issue that sharedData is set late during a content process startup.
So that we already start running a few JSM and JS XPCOM before it is set.
It means that DevTools won't debug these modules correctly. Typically, breakpoints won't work and we may miss early resources.

You need to log in before you can comment on or make changes to this bug.