https://tbpl.mozilla.org/?tree=Try&rev=3b9368919def (or https://tbpl.mozilla.org/?tree=Try&rev=66f4e2177c4f without the unrelated crasher that was recently backed out of aurora) - everything except cpp/jit/xpcshell, which don't open windows, will assert on debug "Failed to get script global: 'NS_SUCCEEDED(rv) && newInnerGlobal && newInnerWindow->GetWrapperPreserveColor() == newInnerGlobal'" and then crash [@ nsGlobalWindow::DefineArgumentsProperty(nsIArray*)]. Since both DefineArgumentsProperty and the only RELEASE_BUILD ifdef in dom/base/ are bholley's, I'm kind of looking in that direction, though the appearance of GetWrapperPreserveColor in both the assertion and in DefineArgumentsProperty is suspicious too. Merge is July 21st, so you've got three weeks starting tomorrow.

Looks like the stuff added in bug 1010577 fails, and once that happens, setting up the wrapper for the window doesn't work. . 19:36:17 INFO - [2011] ###!!! ASSERTION: This is not supposed to fail!: 'Error', file /builds/slave/try-lx-d-000000000000000000000/build/js/xpconnect/src/nsXPConnect.cpp, line 303 19:36:17 INFO - UnexpectedFailure<tag_nsresult> [js/xpconnect/src/nsXPConnect.cpp:305] 19:36:17 INFO - nsXPConnect::GetWrappedNativePrototype(JSContext*, JSObject*, nsIClassInfo*, nsIXPConnectJSObjectHolder**) [js/xpconnect/src/nsXPConnect.cpp:864] 19:36:17 INFO - GetXPCProto [dom/base/nsDOMClassInfo.cpp:2390] 19:36:17 INFO - nsWindowSH::GlobalResolve(nsGlobalWindow*, JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>) [dom/base/nsDOMClassInfo.cpp:2832] 19:36:17 INFO - nsGlobalWindow::DoNewResolve(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>) [dom/base/nsGlobalWindow.cpp:4287] 19:36:17 INFO - mozilla::dom::WindowBinding::_newResolve [obj-firefox/dom/bindings/WindowBinding.cpp:12846] 19:36:17 INFO - CallResolveOp [js/src/jsobj.cpp:4022] 19:36:17 INFO - LookupPropertyInline<(js::AllowGC)1u> [js/src/jsobj.cpp:4111] 19:36:17 INFO - js::FindClassObject(js::ExclusiveContext*, JS::MutableHandle<JSObject*>, js::Class const*) [js/src/jsobj.cpp:4246] 19:36:17 INFO - js::FindClassPrototype(js::ExclusiveContext*, JS::MutableHandle<JSObject*>, js::Class const*) [js/src/jsobj.cpp:5560] 19:36:17 INFO - js::FindProto(js::ExclusiveContext*, js::Class const*, JS::MutableHandle<JSObject*>) [js/src/jsobjinlines.h:822] 19:36:17 INFO - js::NewObjectWithClassProtoCommon(js::ExclusiveContext*, js::Class const*, JSObject*, JSObject*, js::gc::AllocKind, js::NewObjectKind) [js/src/jsobj.cpp:1568] 19:36:17 INFO - JS_NewObject(JSContext*, JSClass const*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) [js/src/jsapi.cpp:2586] 19:36:17 INFO - nsWindowSH::GlobalResolve(nsGlobalWindow*, JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>) [dom/base/nsDOMClassInfo.cpp:2652] 19:36:17 INFO - nsGlobalWindow::DoNewResolve(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>) [dom/base/nsGlobalWindow.cpp:4287] 19:36:17 INFO - mozilla::dom::WindowBinding::_newResolve [obj-firefox/dom/bindings/WindowBinding.cpp:12846] 19:36:17 INFO - CallResolveOp [js/src/jsobj.cpp:4022] 19:36:17 INFO - LookupOwnPropertyInline<(js::AllowGC)1u> [js/src/jsobj.cpp:4111] 19:36:17 INFO - js::DefineNativeProperty(js::ExclusiveContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>), unsigned int) [js/src/jsobj.cpp:4146] 19:36:17 INFO - JSObject::defineGeneric(js::ExclusiveContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>), unsigned int) [js/src/jsobj.cpp:3607] 19:36:17 INFO - DefinePropertyById [js/src/jsapi.cpp:2976] 19:36:17 INFO - DefineProperty [obj-firefox/dist/include/js/RootingAPI.h:811] 19:36:17 INFO - JS_DefineProperties(JSContext*, JS::Handle<JSObject*>, JSPropertySpec const*) [js/src/jsapi.cpp:3391] 19:36:17 INFO - bool mozilla::dom::DefinePrefable<JSPropertySpec const>(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Prefable<JSPropertySpec const> const*) [dom/bindings/BindingUtils.cpp:314] 19:36:17 INFO - mozilla::dom::DefineProperties(JSContext*, JS::Handle<JSObject*>, mozilla::dom::NativeProperties const*, mozilla::dom::NativeProperties const*) [dom/bindings/BindingUtils.cpp:624] 19:36:17 INFO - mozilla::dom::WindowBinding::Wrap(JSContext*, nsGlobalWindow*, nsWrapperCache*, JS::CompartmentOptions&, JSPrincipals*, bool) [obj-firefox/dom/bindings/WindowBinding.cpp:13047] 19:36:17 INFO - nsGlobalWindow::SetNewDocument(nsIDocument*, nsISupports*, bool) [dom/base/nsGlobalWindow.cpp:2275]

I can't reproduce this in my beta build, nor do I see the assertion from comment 1 in the failing tinderbox logs... :(

> nor do I see the assertion Er, it helps to look at a debug log. Now I do see it, and I guess the issue is not beta per se but aurora with RELEASE_BUILD bits.

OK, so what's happening here is that an nsGlobalChromeWindow. We go to define webidl properties on it, since on 32 Window is on WebIDL bindings. This includes defining the ChromeOnly ones (unlike 31, where quickstubs only defined the normal props). And _that_ includes "controllers", so we land in nsWindowSH::GlobalResolve while still creating the window's JS object, with id == IDX_CONTROLLERS. We check for !nsContentUtils::IsSystemPrincipal(aWin->GetPrincipal()), which tests true because aWin->GetPrincipal() is null. It's null because we're still in nsGlobalWindow::SetNewDocument and we haven't gotten far enough along yet to have set mDoc on the new inner we're creating. Bobby, can we just replace this check by a subject principal check or an object principal check on "obj" (presumably the same thing in practice, since we're in the compartment of "obj")?

(In reply to Boris Zbarsky [:bz] from comment #3) > I guess the issue is not beta per se but aurora with RELEASE_BUILD bits. I'll switch to filing them with explict STR, instead of "go look at my try push's queue and parent, just look at it!"

Nah, it was just me not reading very carefully. Or rather forgetting the context by the time I came back to this bug and just reading comment 1.

An alternative is to move the code to CreateGlobalOptions<nsGlobalWindow>::PostCreateGlobal. Not a very elegant solution, but just throwing it out there.

(In reply to Boris Zbarsky [:bz] from comment #4) > Bobby, can we just replace this check by a subject principal check or an > object principal check on "obj" (presumably the same thing in practice, > since we're in the compartment of "obj")? Yeah, let's use |obj|, with a comment.

Philor, can you test this patch on try with the appropriate magic?

Comment on attachment 8449030 [details] [diff] [review] Grab the principal from the object rather than the window when resolving the controller shim. v1 r=me

Don't forget to request approval-aurora.

Comment on attachment 8449030 [details] [diff] [review] Grab the principal from the object rather than the window when resolving the controller shim. v1 See comment 0. We need to take this to avoid breaking all of CI in two weeks. Approval Request Comment [Feature/regressing bug #]: bug 1010577 [User impact if declined]: Beta is all orange after the merge [Describe test coverage new/current, TBPL]: No explicit test coverage [Risks and why]: Very low risk [String/UUID change made/needed]: None