1753381 - Safeguard in walker::children hit when reloading while selecting an iframe element

Status: NEW

(DevTools :: Inspector, defect, P2)

Description

[Julian Descottes [:jdescottes]]

The STRs are slightly intermittent and must depend on how fast the iframe loads

STRs:

• open http://janodvarko.cz/tests/fission/case10/index.html
• open devtools > inspector
• select the #document in the iframe
• reload

This can lead to a console.warn: "connectToFrame returned an unexpected target". It usually takes 2 reloads for me to be triggered.

Without the safeguard, this would result in an infinite loop, so the goal of this bug is to investigate the root cause of this issue and to fix it.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Thanks for filing this Julian.
I can't reproduce it (I tried with some network simulation too, without any luck), but this seems to be an issue with the "reselect previously selected node" feature. I thought we took care of slow loading iframe (which was intended to be covered in https://searchfox.org/mozilla-release/source/devtools/client/inspector/test/browser_inspector_reload_nested_iframe.js)
Could you dig a bit more and see why this happen? (wrong browsingContextId on the iframe nodeFront form, or something else) ?

Comment 2

[Julian Descottes [:jdescottes]]

Oh I think my STRs are wrong, the "document" never gets backed up as a node to reselect. It happens when you select the iframe element.

Basically we have a race in WalkerFront::connectToFrame. We attempt to get the target for a browsing context id for which we didn't receive the target yet.

So we end up in

    // If we could not find a window global target for the provided id, the
    // window global might not be the topmost one of a given process (isProcessRoot == true).
    // For now we only create targets for the top window global of each process,
    // so we recursively check the parent browsing context ids
    // until we find a valid target.
    const parentBrowsingContextID = await this.getParentBrowsingContextID(id);
    if (parentBrowsingContextID && parentBrowsingContextID !== id) {
      return this.getWindowGlobalTarget(parentBrowsingContextID);
    }

We assume that we have no target for the provided browsing context (makes no sense for the content toolbox + EFT, right?), and we fallback to the parent target, which is wrong.

For the record, I added some logging and we attempt to call connectToFrame for the frame's browsing context id before the server created the target. The JSWindowActor attempts to instantiate a target for this BrowsingContextId earlier, but attempts bail on _validateWindowGlobal (I think)