1177565 - ~15,000 instances of 'NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111' emitted from docshell/base/nsDocShell.cpp during linux64 debug testing

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Eric Rahm [:erahm]]

> 14754 [NNNNN] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file docshell/base/nsDocShell.cpp, line 4607

0x80040111 is NS_ERROR_NOT_AVAILABLE. This warning [1] is rather widespread across tests, it is most frequent during mochitest-browser-chrome-2, a few more verbose tests:
> 97 warnings from test browser/components/sessionstore/test/browser_354894_perwindowpb.js
> 76 warnings from test browser/components/sessionstore/test/browser_522545.js
> 48 warnings from test browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js

It looks like this can happen if |mIsBeingDestroyed| is true [2] or if |do_GetInterface(mTreeOwner)| fails [3]. Given the frequency of this warning it seems like we should just remove it (or possibly move it to |EnsureScriptEnvironment| where we can get a better idea of what is actually failing).

[1] https://hg.mozilla.org/mozilla-central/annotate/02f640a72dcd/docshell/base/nsDocShell.cpp#l4607
[2] https://hg.mozilla.org/mozilla-central/annotate/0b2f5e8b7be5/docshell/base/nsDocShell.cpp#l12780
[3] https://hg.mozilla.org/mozilla-central/annotate/0b2f5e8b7be5/docshell/base/nsDocShell.cpp#l12795

Comment 1

[Eric Rahm [:erahm]]

Attachment: Stop warning if EnsureScriptEnvironment returns NS_ERROR_NOT_AVAILABLE

Note: This is the most verbose warning during linux64 debug testing.

Comment 2

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

Comment on attachment 8626364 [details] [diff] [review]
Stop warning if EnsureScriptEnvironment returns NS_ERROR_NOT_AVAILABLE

I don't think it is useful to warn ever.
So just 

return NS_SUCCEEDED(EnsureScriptEnvironment()) ? mScriptGlobal : nullptr;

Or even
EnsureScriptEnvironment();
return mScriptGlobal;

But I think I'd prefer the first one.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Olli Pettay [:smaug] from comment #2)
> Comment on attachment 8626364 [details] [diff] [review]
> Stop warning if EnsureScriptEnvironment returns NS_ERROR_NOT_AVAILABLE
> 
> I don't think it is useful to warn ever.
> So just 
> 
> return NS_SUCCEEDED(EnsureScriptEnvironment()) ? mScriptGlobal : nullptr;
> 
> Or even
> EnsureScriptEnvironment();
> return mScriptGlobal;
> 
> But I think I'd prefer the first one.

Thanks for the quick review! I'll update to use the tristate.

Comment 4

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63543a32a11d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63543a32a11d