In bug 979481, I'm changing the SafeJSContext such that pushing it resets its compartment to null, rather than to the compartment of the dummy SafeJSContext global. There were two oranges - one of them the Settings API, the other one in Workers, via browser/devtools/shared/test/browser_telemetry_button_tilt.js. The basic issue is that the TILT_CRAFTER chrome worker is parented to a main thread global that is not an nsIDOMWindow, so mWindow and mScriptContext are null. So we end up using the SafeJSContext. In WorkerRunnable.cpp, we try to find a default compartment object to enter, using GetWrapper(). But this can return null if the wrapper has been GCed: http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerRunnable.cpp#300 Before bug 979481, the SafeJSContext defaults to the SafeJSContext compartment, so we silently do the wrong thing. Afterwards, its defaults compartment is null, so we assert. It seems like we should replace (or augment) mWindow with some sort of |nsIGlobalObject* mMainThreadGlobal|, which would always be non-null. Kyle and Ben didn't have an immediate consensus on what this should look like, so I'm punting this to Kyle and taking myself out of the loop.

This is the stack where we crash: https://pastebin.mozilla.org/4480803

I think we should land this for now. We're seeing this pattern where we want to be able to recreate a wrapper at will from C++ appear a lot. I think we need a more generic solution here in nsWrapperCache or something.

Comment on attachment 8386457 [details] [diff] [review] Patch Review of attachment 8386457 [details] [diff] [review]: ----------------------------------------------------------------- This should be ok for beta as long as these are addressed: ::: dom/workers/WorkerPrivate.cpp @@ +2142,5 @@ > > AssertIsOnParentThread(); > > + JSObject* wrapper = > + WorkerBinding::Wrap(aCx, aScope, ParentAsWorkerPrivate()); Can you call TryPreserveWrapper if this returns null? It's probably safer to nullcheck right? @@ +2143,5 @@ > AssertIsOnParentThread(); > > + JSObject* wrapper = > + WorkerBinding::Wrap(aCx, aScope, ParentAsWorkerPrivate()); > + TryPreserveWrapper(wrapper); If I'm reading the comments right this should be MOZ_ALWAYS_TRUE(Try...)

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/a944b1d3110b (along with bug 619487and bug 968836) for m-bc bustage on at least OSX: https://tbpl.mozilla.org/php/getParsedLog.php?id=35695094&tree=Mozilla-Inbound

Also, looks like this had a rooting hazard: https://tbpl.mozilla.org/?tree=Try&rev=55d3b0ada798

I had to add a Rooted to make the analysis happy.

If it's not too late I'd like to slip this into beta to avoid shipping a release where web workers may not work if the GC runs at exactly the right time. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 928312 User impact if declined: If the website uses web workers in odd ways, they may not work properly. Testing completed (on m-c, etc.): On m-c, trivial patch. Risk to taking this patch (and alternatives if risky): Very low risk. This essentially restores the behavior we have had in Gecko 27 and lower. String or IDL/UUID changes made by this patch: N/A

Tracking since this is a regression from 27 and would cause website breakage in ways we can't tell the impact of at this late stage - will approve the uplifts in a sec.

https://hg.mozilla.org/releases/mozilla-aurora/rev/e8883bce299f https://hg.mozilla.org/releases/mozilla-beta/rev/bef53d213a54