1598364 - The browser toolbox inspector breaks when the browser window reloads

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Alexandre Poirot [:ochameau]]

STR:

• Open the Omniscient Browser Toolbox
• Open the inspector
• Open the split console
• execute location.reload()

AR:
many exception and the markup view is broken. The markup is outdate and selecting any element doesn't update the sidebar.

ER:
the markup view is updated and works against the new browser.xhtml document.

Comment 1

[Alexandre Poirot [:ochameau]]

This isn't trivial and it looks like it has always been somewhat broken.

It relates to this code:
https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/devtools/server/actors/targets/browsing-context.js#1547-1548

    const handler = getDocShellChromeEventHandler(docShell);
    handler.addEventListener("DOMWindowCreated", this._onWindowCreated, true);

The inspector looks more broken than the others because it doesn't receive any "new root"
https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/devtools/client/inspector/inspector.js#431
this.walker.on("new-root", this.onNewRoot); which allows to re-populate the markup-view
That's because we don't emit window-ready:
https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/devtools/server/actors/inspector/walker.js#329
targetActor.on("window-ready", this.onFrameLoad);
And the window-ready are a translationg of DOMWindowCreated events:
https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/devtools/server/actors/targets/browsing-context.js#1628
this._targetActor._windowReady(window, { isBFCache });

So. The issue here is that, in the context of the browser toolbox and the ParentProcessTargetActor, getDocShellChromeEventHandler(docShell) return the current browser.xhtml window object. It watches correctly all children documents/windows of this current browser window, but not the other windows, nor the next window in case of reload or navigation.
We would need to reattach the DOMWindowCreated listener on the next window on reload and navigation, as well as fire a window-ready in case of reload/navigation of the top level window.
Unfortunately, the docShell isn't destroyed, otherwise this code would have fixed this issue:
https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/devtools/server/actors/targets/browsing-context.js#792-801

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1598364 - Re-register DOMWindowCreated listeners when reloading browser.xhtml.

In the context of the browser toolbox, the ParentProcessTarget Actor watches
all top level window's docshells. But as it registers DOMWindowCreated/pageshow/pagehide
listeners against the docshell's window (because there is no parent chromeEventListener),
it has to ensure re-registering these listeners on navigation.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/014b9e47ea00
Re-register DOMWindowCreated listeners when reloading browser.xhtml. r=jdescottes

Comment 4

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/014b9e47ea00