1317347 - TypeError: this.docShell is null when opening about:newtab

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Alexandre Poirot [:ochameau]]

I don't think that's specific to about:newtab, but sometime you can see tons of these exception in the browser console:
console.error: 
  Handler function threw an exception: TypeError: this.docShell is null
Stack: get_contentWindow@chrome://global/content/bindings/browser.xml:466:1
get_outerWindowID@chrome://global/content/bindings/browser.xml:435:11
matchRequest@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/network-monitor.js:87:1
NetworkMonitor.prototype._onRequestHeader@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/network-monitor.js:1148:10
NetworkMonitor.prototype.observeActivity<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/network-monitor.js:1012:7
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
Line: 466, column: 1

Here is a possible STR (note it is racy or it depends on same caches from about:newtab or something)
* open data:text/html,foo
* open webconsole
* open about:newtab in a new tab
* over website previews

> you should see many of these exceptions

If you don't, try closing about:newtab tab, it sometimes forces this exception.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1317347 - Fix exception in network monitor for requests coming from iframes added to the hidden window.

Review commit: https://reviewboard.mozilla.org/r/92768/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92768/

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8810439 [details]
Bug 1317347 - Fix exception in network monitor for requests coming from iframes added to the hidden window.

https://reviewboard.mozilla.org/r/92768/#review92738

::: devtools/shared/webconsole/network-monitor.js:89
(Diff revision 1)
>  
>    if (filters.outerWindowID) {
>      let topFrame = NetworkHelper.getTopFrameForRequest(channel);
> -    if (topFrame && topFrame.outerWindowID &&
> +    // Bug XXX: Check for docShell as outerWindowID getter may throw on frames
> +    // added to the hidden window.
> +    if (topFrame && topFrame.docShell && topFrame.outerWindowID &&

Doesn't this break the network monitor for every remote browser (where `docShell` is unavailable from the parent)?

Comment 3

[Alexandre Poirot [:ochameau]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2)
> Doesn't this break the network monitor for every remote browser (where
> `docShell` is unavailable from the parent)?

Yes most likely, good catch!
Do you think a try catch is reasonable? It may hide other breakage... But I can't think about any other way?
topFrame is still rooted to a document (topFrame.parentNode and topFrame.ownerDocument are not null).
I haven't found any other check that would help knowing this frame should be ignored.

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8810439 [details]
Bug 1317347 - Fix exception in network monitor for requests coming from iframes added to the hidden window.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/92768/diff/1-2/

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8810439 [details]
Bug 1317347 - Fix exception in network monitor for requests coming from iframes added to the hidden window.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/92768/diff/2-3/

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8810439 [details]
Bug 1317347 - Fix exception in network monitor for requests coming from iframes added to the hidden window.

https://reviewboard.mozilla.org/r/92768/#review93376

Thanks, this version seems safe to me!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/163281db1563
Fix exception in network monitor for requests coming from iframes added to the hidden window. r=jryans

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/163281db1563