1177578 - ~11,000 instances of 'No docshells for remote frames!' emitted from dom/base/nsFrameLoader.cpp during linux64 debug tests

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Eric Rahm [:erahm]]

This is currently the #3 most verbose warning [1] during linux64 debug testing. It appears primarily in e10s tests:
> erahm-25043:2015-24-06 ericrahm$ grep -c 'No docshells for remote frames!' *.txt | grep -v ':0$' | sort -t ':' -k 2 -rn
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-3-bm54-tests1-linux64-build7.txt:4526
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-1-bm114-tests1-linux64-build18.txt:2328
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-2-bm51-tests1-linux64-build3.txt:1942
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-2-bm114-tests1-linux64-build31.txt:615
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-1-bm121-tests1-linux64-build2.txt:537
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-5-bm51-tests1-linux64-build7.txt:315
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-3-bm68-tests1-linux64-build7.txt:196
> mozilla-central_ubuntu64_vm-debug_test-mochitest-browser-chrome-3-bm67-tests1-linux64-build10.txt:134
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-4-bm118-tests1-linux64-build5.txt:103
> mozilla-central_ubuntu64_vm-debug_test-mochitest-other-bm123-tests1-linux64-build3.txt:38
> mozilla-central_ubuntu64_vm-debug_test-mochitest-browser-chrome-2-bm121-tests1-linux64-build8.txt:8
> mozilla-central_ubuntu64_vm-debug_test-mochitest-2-bm117-tests1-linux64-build7.txt:7
> mozilla-central_ubuntu64_vm-debug_test-reftest-e10s-2-bm117-tests1-linux64-build5.txt:3
> mozilla-central_ubuntu64_vm-debug_test-reftest-e10s-1-bm114-tests1-linux64-build10.txt:3

It seems like we should just remove the warning as this code path is exercised on e10s.

[1] https://hg.mozilla.org/mozilla-central/annotate/0b2f5e8b7be5/dom/base/nsFrameLoader.cpp#l459

Comment 1

[Boris Zbarsky [:bzbarsky]]

Who's asking for the docshell in this case and why?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

This particular warning is IMO useful, since it usually hints that something isn't really e10s-fied properly yet.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Boris Zbarsky [:bz] from comment #1)
> Who's asking for the docshell in this case and why?

Well 11,000 things apparently. I can look into a few of the top tests.

Comment 4

[Eric Rahm [:erahm]]

Attachment: Example stacks

It looks like some sort of XUL bindings:

> (gdb) call DumpJSStack()
> 0 get_docShell() ["chrome://global/content/bindings/browser.xml":258]
>     this = [object XULElement]
> 1 browser_XBL_Constructor() ["chrome://global/content/bindings/browser.xml":779]
>     this = [object XULElement]
> 2 addTab(aURI = "about:blank", aReferrerURI = undefined) ["chrome://browser/content/tabbrowser.xml":1821]
>     this = [object XULElement]

> (gdb) call DumpJSStack()
> 0 get_docShell() ["chrome://global/content/bindings/browser.xml":258]
>     this = [object XULElement]
> 1 updateCurrentBrowser() ["chrome://browser/content/tabbrowser.xml":1174]
>     this = [object XULElement]
> 2 onselect(event = [object Event]) ["chrome://browser/content/browser.xul":1]
>     this = [object XULElement]

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hmm.  Bill, should those xbl bits get changed, or should the frame loader get changed?

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I don't think this warning is helping many people. Frontend devs use opt builds and I run with warnings disabled.

However, if Olli feels strongly about this we could do something to avoid asking for the docShell. We  use its absence as an indicator for whether e10s is on right now.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Olli, see comment 6?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Bill McCloskey (:billm) from comment #6)
> I don't think this warning is helping many people. Frontend devs use opt
> builds and I run with warnings disabled.
Whaaat? 


> However, if Olli feels strongly about this we could do something to avoid
> asking for the docShell. We  use its absence as an indicator for whether
> e10s is on right now.
oh, its use has changed. Then let's just remove the warning.

Comment 9

[Eric Rahm [:erahm]]

Attachment: Remove 'No docshells for remote frames' warning in nsFrameLoader::GetDocShell

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8627729 [details] [diff] [review]
Remove 'No docshells for remote frames' warning in nsFrameLoader::GetDocShell

Thanks

Comment 11

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b9d80a126e5

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6b9d80a126e5