1301189 - Unmark gray assertion failure in nsJSUtils::ModuleEvaluation

Status: RESOLVED FIXED

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

Description

[Andrew McCreight [:mccr8]]

The patch in bug 1110928 triggers a black-gray invariant assertion failure, only on Android 4.3 API15+ debug. I assume this is some quirk that it always fails there.

The stack looks like this:
js::CompartmentChecker::check [jscntxtinlines.h:5001e8f1da97 : 71 + 0x2]
JS::ModuleEvaluation [jscntxtinlines.h:5001e8f1da97 : 83 + 0x3]
nsJSUtils::ModuleEvaluation [nsJSUtils.cpp:5001e8f1da97 : 329 + 0x7]
nsScriptLoader::EvaluateScript [nsScriptLoader.cpp:5001e8f1da97 : 2151 + 0x5]
nsScriptLoader::ProcessRequest [nsScriptLoader.cpp:5001e8f1da97 : 1953 + 0x7]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5001e8f1da97&selectedJob=27105527

Comment 1

[Andrew McCreight [:mccr8]]

I think the getters on nsModuleScript should simply do the expose.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> I think the getters on nsModuleScript should simply do the expose.

Yes, that is correct.  They should.

Comment 3

[Andrew McCreight [:mccr8]]

That fixed my issue:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=306f6996c495

I suppose I should do a real try run, too.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1301189 - Use ExposeObjectToActiveJS in nsModuleScript.

These GC things can be passed to the JS engine, so we should make sure
they are not marked gray.

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

Comment 5

[Andrew McCreight [:mccr8]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aed27f8fa967&selectedJob=27181575

Comment 6

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

Comment on attachment 8789881 [details]
Bug 1301189 - Use ExposeObjectToActiveJS in nsModuleScript.

https://reviewboard.mozilla.org/r/77934/#review76366

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dbeaca20e104
Use ExposeObjectToActiveJS in nsModuleScript. r=smaug

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/dbeaca20e104

Comment 9

[Andrew McCreight [:mccr8]]

Comment on attachment 8789881 [details]
Bug 1301189 - Use ExposeObjectToActiveJS in nsModuleScript.

Approval Request Comment
[Feature/regressing bug #]: bug 1240072
[User impact if declined]: possible crashes
[Describe test coverage new/current, TreeHerder]: I think this code has tests
[Risks and why]: low
[String/UUID change made/needed]: none

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8789881 [details]
Bug 1301189 - Use ExposeObjectToActiveJS in nsModuleScript.

Crash fix, has been on Nightly for a week, Aurora50+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ade8492b5680