1330123 - 1,100 instances of "NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012" emitted from layout/style/Loader.cpp during linux64 debug testing

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Eric Rahm [:erahm]]

> 1122 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file layout/style/Loader.cpp, line 2159

This warning [1] shows up in the following test suites:

>    171 - test-linux64/debug-mochitest-devtools-chrome-10 dt10
>    170 - test-linux64/debug-mochitest-devtools-chrome-e10s-10 dt10
>    130 - test-linux64/debug-mochitest-devtools-chrome-2 dt2
>    112 - test-linux64/debug-mochitest-devtools-chrome-e10s-6 dt6
>     95 - test-linux64/debug-mochitest-devtools-chrome-e10s-8 dt8
>     93 - test-linux64/debug-mochitest-devtools-chrome-5 dt5
>     58 - test-linux64/debug-mochitest-devtools-chrome-e10s-5 dt5
>     53 - test-linux64/debug-mochitest-devtools-chrome-3 dt3
>     25 - test-linux64/debug-mochitest-chrome-3 c3
>     21 - test-linux64/debug-mochitest-devtools-chrome-9 dt9
>     20 - test-linux64/debug-mochitest-devtools-chrome-4 dt4
>     19 - test-linux64/debug-mochitest-devtools-chrome-e10s-7 dt7
>     18 - test-linux64/debug-mochitest-devtools-chrome-e10s-1 dt1
>     17 - test-linux64/debug-mochitest-clipboard-e10s cl
>     17 - test-linux64/debug-mochitest-clipboard cl
>     15 - test-linux64/debug-mochitest-devtools-chrome-e10s-4 dt4
>     15 - test-linux64/debug-mochitest-chrome-2 c2
>     14 - test-linux64/debug-mochitest-devtools-chrome-6 dt6
>     13 - test-linux64/debug-mochitest-devtools-chrome-1 dt1
>     11 - test-linux64/debug-mochitest-devtools-chrome-e10s-9 dt9
>     11 - test-linux64/debug-mochitest-devtools-chrome-8 dt8
>     10 - test-linux64/debug-mochitest-devtools-chrome-e10s-2 dt2
>      7 - test-linux64/debug-mochitest-chrome-1 c1
>      2 - test-linux64/debug-mochitest-devtools-chrome-7 dt7
>      1 - test-linux64/debug-mochitest-a11y a11y
>      1 - test-linux64/debug-mochitest-browser-chrome-e10s-4 bc4
>      1 - test-linux64/debug-mochitest-browser-chrome-e10s-2 bc2
>      1 - test-linux64/debug-mochitest-browser-chrome-4 bc4
>      1 - test-linux64/debug-mochitest-browser-chrome-3 bc3

It shows up in 1089 tests. A few of the most prevalent:

>      4 -        devtools/client/framework/test/browser_toolbox_select_event.js
>      4 -        devtools/client/inspector/shared/test/browser_styleinspector_csslogic-content-stylesheets.js
>      3 -        devtools/client/shared/test/browser_telemetry_toolbox.js
>      3 -        devtools/client/webconsole/test/browser_webconsole_split_persist.js
>      3 - [e10s] devtools/client/webconsole/test/browser_webconsole_split_persist.js
>      3 - [e10s] devtools/client/shared/test/browser_telemetry_toolbox.js
>      2 - [e10s] devtools/client/framework/test/browser_toolbox_hosts.js
>      2 - [e10s] devtools/client/webconsole/test/browser_webconsole_split.js
>      2 -        devtools/client/inspector/test/browser_inspector_initialization.js
>      2 - [e10s] devtools/client/inspector/test/browser_inspector_sidebarstate.js

[1] https://hg.mozilla.org/mozilla-central/annotate/2977ca122452/layout/style/Loader.cpp#l2159

Comment 1

[Eric Rahm [:erahm]]

This looks similar to bug 1304197, the warning has become more prevalent (I would guess because we turned on e10s devtools tests).

Comment 2

[:Gijs (he/him)]

At least some of this is inspector.xhtml, which loads a non-existing CSS file as of bug 1309733 being fixed.

Can we make this warning log the filename if the containing document has a chrome:// or resource:// URL? That will really help track this kind of stuff down, and will also help us and add-on authors keep their stuff sane. :-)

Comment 3

[Eric Rahm [:erahm]]

(In reply to :Gijs from comment #2)
> At least some of this is inspector.xhtml, which loads a non-existing CSS
> file as of bug 1309733 being fixed.
> 
> Can we make this warning log the filename if the containing document has a
> chrome:// or resource:// URL? That will really help track this kind of stuff
> down, and will also help us and add-on authors keep their stuff sane. :-)

At least for 'chrome://' it looks like we're already printing it in the logs [1]. I'm not sure about 'resource://'.

[1] http://searchfox.org/mozilla-central/rev/8fa84ca6444e2e01fb405e078f6d2c8da0e55723/chrome/nsChromeProtocolHandler.cpp#142

Comment 4

[:Gijs (he/him)]

(In reply to Eric Rahm [:erahm] from comment #3)
> (In reply to :Gijs from comment #2)
> > At least some of this is inspector.xhtml, which loads a non-existing CSS
> > file as of bug 1309733 being fixed.
> > 
> > Can we make this warning log the filename if the containing document has a
> > chrome:// or resource:// URL? That will really help track this kind of stuff
> > down, and will also help us and add-on authors keep their stuff sane. :-)
> 
> At least for 'chrome://' it looks like we're already printing it in the logs
> [1]. I'm not sure about 'resource://'.
> 
> [1]
> http://searchfox.org/mozilla-central/rev/
> 8fa84ca6444e2e01fb405e078f6d2c8da0e55723/chrome/nsChromeProtocolHandler.
> cpp#142

The chrome:// thing only happens if the package or type name (so in chrome://browser/content/, that's 'browser' and 'content' respectively) isn't recognized. Not if the file doesn't exist (so e.g. chrome://browser/content/i-am-total-bogus.html will not warn in any way, but chrome://bogus/content/ or chrome://browser/bogus/ will).

Comment 5

[:Gijs (he/him)]

I'm hopeful that once bug 1333164 autolands we should be OK here. Can you recheck what's left once that's in?

And, is there a way to make this fail on infra for debug builds so we don't accidentally reintroduce this?

Comment 6

[Eric Rahm [:erahm]]

(In reply to :Gijs from comment #5)
> I'm hopeful that once bug 1333164 autolands we should be OK here. Can you
> recheck what's left once that's in?
> 
> And, is there a way to make this fail on infra for debug builds so we don't
> accidentally reintroduce this?

Sorry this took a while, we're down to 248 which is pretty low:

> 248 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file layout/style/Loader.cpp, line 2326

I don't think we have a good way of turning the tree red on warning regressions. It gets a little tricky as we'd have to run the analysis after all other tests run.

Comment 7

[Eric Rahm [:erahm]]

Lets close this for now and revisit if it spikes.