Closed Bug 1652016 Opened 3 months ago Closed 2 months ago

Target registered from webextension aren't unregistered when toolbox closes

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(Fission Milestone:M6c, firefox80 fixed)

RESOLVED FIXED
Firefox 80
Fission Milestone M6c
Tracking Status
firefox80 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

Details

(Whiteboard: dt-fission-m2-mvp)

Attachments

(1 file)

Steps to reproduce

  1. Ensure you have devtools.testing.enableServerWatcherSupport set to true
  2. Navigate to data:text/html,<meta charset=utf8><script>console.log(Date.now())</script>
  3. Open the console, there should be a message
  4. Close the toolbox
  5. Reopen it: the message isn't there anymore
  6. Reload the page: the message does not appear either

From what I can see in my debugging session, in DevToolsFrameChild, _watchResources calls targetActor.watchTargetResources on the wrong actor.

It looks like _getTargetActorForWatcherActorID calls the TargetActorRegistry to retrieve the target actor matching a given browserId.
But in TargetActorRegistry, it seems like we still have actors from the first toolbox opening, and since getTargetActor returns the first target that has the same browserId than the one that is passed, we get the wrong one.

(It also seem that for a same "toolbox session", we can have multiple targets in the registry that share the same browserId, which makes getTargetActor unreliable?)

I'll continue to dig into this.

Tracking dt-fission-m2-mvp bugs for Fission Nightly milestone (M6c).

Fission Milestone: --- → M6c

Note that this is only happening when there are DevTools extensions installed.
In ExtensionParent.jsm, requesting the inspectedWindowFront will create a target for the tab (toolkit/components/extensions/ExtensionParent.jsm#678-681,715,720-722)

And this target doesn't appear to be destroyed when the DevTools toolbox is closed.

Julian, I think you're more familiar than me with this code, would you know what we're missing/doing wrong?

Flags: needinfo?(jdescottes)
Summary: When server watcher support is enabled, toolbox isn't receiving console messages after reopening → When server watcher support is enabled and user has DevTools extension, toolbox isn't receiving console messages after reopening
Summary: When server watcher support is enabled and user has DevTools extension, toolbox isn't receiving console messages after reopening → Target registered from webextension aren't unregistered when toolbox closes

(asking daisuke too, who worked on this area in the past)

note that I'm really not familiar with the extension code

Could we shutdown the context from browser/components/extensions/parent/ext-devtools.js#165-168, when we close the DevToolsPage, or does the context still need to be alive for some reason?

Flags: needinfo?(daisuke)

Let's also ask Luca :) I was wondering the same, why shutdown (which destroys the target) wouldn't be called when closing devtools.
https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/toolkit/components/extensions/ExtensionParent.jsm#687-713

(keeping the ni? because I wanted to test that myself, haven't had time yet)

Flags: needinfo?(lgreco)

I think I fixed it, and I'm about to push a patch to review for that.

Flags: needinfo?(lgreco)
Flags: needinfo?(jdescottes)
Flags: needinfo?(daisuke)

The problem has 2 folds.
The first one is that the shitdown function on DevToolsExtensionPageContextParent
wasn't called, which means the target was never destroyed. This is fixed by
calling context.shutdown instead of context.unload in closeProxyContext. This should
be fine as shutdown ends up calling unload in the end (ProxyContextParent.shutdown
directly calls ProxyContextParent.unload).

The second issue was that a single webextension could create 2 targets, and since
we only keep track of a single target, we would miss one. This is fixed by putting
the call to watchTargets in getCurrentDevTools in a promise, so subsequent calls
that might occur before the resulting promise isn't resolved don't end up calling
watchTargets a second time.

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80317e4673b6
Destroy target created by WebExtension when destroying the toolbox. r=rpl,daisuke,jdescottes.
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
Duplicate of this bug: 1495441
You need to log in before you can comment on or make changes to this bug.