1664438 - Requests made by protocol.js fronts are not purged

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P2)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

From Julian:

Seems to be triggered when navigating while the toolbox is not fully ready.

STRs:

1. open https://www.wikipedia.org/
2. Open toolbox (inspector or debugger seem to reproduce the bug)
3. Navigate to mozilla.org
4. Navigate back and forth between the two (with back/forward button) until the current devtools panel breaks: empty source pane for the debugger, empty markup view for the inspector. This requires a specific timing, not too quick, not too fast (but quite easy to repro IMO)
5. At that point, the panel will remain broken, and if you trigger the DevTools shortcut, a new Toolbox will open on top of the existing one.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I was able to easily reproduce even after Bug 1663636 landed.

Honza

Comment 2

[Julian Descottes [:jdescottes]]

Sometimes when triggered while using the inspector, a specific error might be logged:

[nsIDOMWindowUtils.getRootBounds] from: server0.conn1.child2/inspectorActor3 (resource://devtools/shared/layout/utils.js:599:0)

Once this issue occurs, you can't reopen DevTools for this tab, they will always be blank (opening DevTools on a new tab is fine however)

Comment 3

[Alexandre Poirot [:ochameau]]

Whaa that was quite a quest to figure out this one. Something is wrong around TargetMixin.destroy and protocol.js request not being "purged", leading to pending target that is never properly destroyed.

While bug 1644397 would probably help mitigate these issues, which all related to client side target switching,
we can probably fix a few things which may be benifitial anyway.

Comment 4

[Alexandre Poirot [:ochameau]]

In the TargetList, when we switch 2 times between 2 distinct processes, and so doing 2 target switching in a row, we end up stuck waiting for the first target to be destroyed, when moving to the second target:
https://searchfox.org/mozilla-central/source/devtools/shared/resources/target-list.js#543-544

    // Wait for the target to be destroyed so that TargetFactory clears its memoized target for this tab
    await targetFront.once("target-destroyed");

This first target never emits target-destroyed because we are stuck on this:
https://searchfox.org/mozilla-central/rev/0c97a6410ff018c22e65a0cbe4e5f2ca4581b22e/devtools/client/fronts/targets/target-mixin.js#651-655

        // If a Front with an async initialize method is still being instantiated,
        // we should wait for completion before trying to destroy it.
        if (front instanceof Promise) {
          front = await front;
        }

We are waiting for the inspector async initialization here. But there is nothing really particular to the inspector here.
This Promise never resolves, and that leads us somewhere else, far from target world, deep down in DevToolsClient!

We do receive a "forwardCancelling" event, which is meant to cancel all pending requests to a given target/process that has been destroyed:
https://searchfox.org/mozilla-central/source/devtools/client/devtools-client.js#476-486

    if (
      this.mainRoot &&
      packet.from == this.mainRoot.actorID &&
      packet.type == "forwardingCancelled"
    ) {
      this.purgeRequests(packet.prefix);
      return;
    }

This RDP event is fired correctly, but its implemention is incomplete. It only cancel old fashion requests, the ones using DevToolsClient.request, but it ignores all protocol.js!!!
If we improve this method to also cover protocol.js, the front promise is rejected in TargetMixin and we can eventually emit target-destroyed.

Comment 5

[Alexandre Poirot [:ochameau]]

But, even after fixing all this, there is some more rarer failure happening, already mentioned in comment 2:

console.error: "Error while calling actor 'inspector's method 'getHighlighter'" "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMWindowUtils.getRootBounds]"
console.error: "getWindowDimensions@resource://devtools/shared/layout/utils.js:599:39\nAutoRefreshHighlighter@resource://devtools/server/actors/highlighters/auto-refresh.js:81:25\nBoxModelHighlighter@resource://devtools/server/actors/highlighters/box-model.js:101:5\n_createHighlighter@resource://devtools/server/actors/highlighters.js:144:25\ninitializeInstance@resource://devtools/server/actors/highlighters.js:137:12\ngetHighlighter/this._highlighterPromise<@resource://devtools/server/actors/inspector/inspector.js:208:25\n"
JavaScript error: resource://devtools/shared/protocol/Front.js, line 335: Error: Protocol error (NS_ERROR_UNEXPECTED): Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMWindowUtils.getRootBounds] from: server0.conn0.child4162/inspectorActor3 (resource://devtools/shared/layout/utils.js:599:0)

We should probably followup on this.

Comment 6

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70d2ef063ff8e7385b44a7d733e420d888646307

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1664438 - Destroy pending protocol.js requests when a remote connector closes.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1664438 - Ignore errors when destroying target scoped fronts.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3c3c04383a5
Destroy pending protocol.js requests when a remote connector closes. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/63b929aedf5f
Ignore errors when destroying target scoped fronts. r=jdescottes

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/a3c3c04383a5
https://hg.mozilla.org/mozilla-central/rev/63b929aedf5f