In a try push on the latest patch from Bug 1411707, I see assertion failures on some of the bc suites: GECKO(1835) | Assertion failure: !aReaction->IsUpgradeReaction() (Upgrade reaction should not be scheduled to backup queue), at /builds/worker/workspace/build/src/dom/base/CustomElementRegistry.cpp:1065 For example, from https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7442d4f9ff4026196709b7fe2b923acca865ec2&selectedJob=166895715.

This looks like the same thing from https://bugzilla.mozilla.org/show_bug.cgi?id=1404420#c40: > Upgrade reactions should not be scheduled to backup queue per spec, so we > will hit this assertion if we miss AutoCEReaction on the codepath which will > trigger enqueueing an upgrade reaction. > > In the crash case, the upgrade reaction is from > http://searchfox.org/mozilla-central/rev/ > 40e8eb46609dcb8780764774ec550afff1eed3a5/dom/xbl/nsXBLService.cpp#506. We > probably could add AutoCEReaction (Before nsAutoScriptBlocker) if the > enqueueing upgrade reaction is expected.

I can reproduce this locally with the patch applied using: `./mach mochitest browser/base/content/test/alerts/browser_notification_open_settings.js`

This appears to happen when the Custom Element (a stringbundle, in this case) is created inside of XBL anonymous content. Removing this tag causes the assertion to go away: https://searchfox.org/mozilla-central/rev/588d8120aa11738657da93e09a03378bcd1ba8ec/toolkit/content/widgets/filefield.xml#17

The patch fixes most of the crashes but is now failing accessibility and some other tests with "Assertion failure: !mMightHaveUnreportedJSException": https://treeherder.mozilla.org/#/jobs?repo=try&revision=1fa82e6810b012e9bcce9cb9b61dd4a8c93da650&selectedJob=167121012

Looks like treeherder doesn't have access to the logs atm.

...since the stack trace for those assertion failures would be interesting to know.

The patch didn't seem to compile. Made a tiny tweak and pushed to try again. remote: View the pushlog for these changes here: remote: https://hg.mozilla.org/try/pushloghtml?changeset=7c003ddb835fc0a2b13a584efd1ac07f3b305d19 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c003ddb835fc0a2b13a584efd1ac07f3b305d19 remote: recorded changegroup in replication log in 0.073s

That tryserver build doesn't show the a11y issue. Perhaps the tree was in bad state when you pushed the patch. But I retriggered a11y tests.

Or, hmm, should I have included some other stuff with the try push?

(In reply to Olli Pettay [:smaug] from comment #10) > Or, hmm, should I have included some other stuff with the try push? Yes there needs to be a Custom Element registered that gets used inside of XBL anonymous content. The original try push had one, but it's now bitrotted. I'll push up a patch that does it.

(In reply to Brian Grinstead [:bgrins] from comment #11) > (In reply to Olli Pettay [:smaug] from comment #10) > > Or, hmm, should I have included some other stuff with the try push? > > Yes there needs to be a Custom Element registered that gets used inside of > XBL anonymous content. The original try push had one, but it's now > bitrotted. I'll push up a patch that does it. If you apply the rev from this try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bf324e5a694ad5f5ee42b8b18d0e2edc657697b (specifically https://hg.mozilla.org/try/rev/b68e1a5a3c1e6d13a32718f4cec1a6f775067bba), then do `./mach test browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js` you should hit the original error: GECKO(37825) Assertion failure: !aReaction->IsUpgradeReaction() (Upgrade reaction should not be scheduled to backup queue), at /builds/worker/workspace/build/src/dom/base/CustomElementRegistry.cpp:1188 Then with your unbitrotted version of the patch from this bug, I think that particular test should be fixed but other tests will hit the new `Assertion failure: !mMightHaveUnreportedJSException` error.

Pushed up the folded together version from the try push in Comment 8

Here's a try push showing the failure with the patches applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3624f29f48aa20396f96b18c493591785b431fed&selectedJob=175155532

Copy/pasting the stack since we lost the last try push: [task 2018-04-23T18:37:41.993Z] 18:37:41 INFO - GECKO(1041) | Assertion failure: !mMightHaveUnreportedJSException, at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ErrorResult.h:487 [task 2018-04-23T18:38:08.845Z] 18:38:08 INFO - GECKO(1041) | #01: mozilla::binding_danger::TErrorResult<mozilla::binding_danger::AssertAndSuppressCleanupPolicy>::AssertReportedOrSuppressed [dom/bindings/ErrorResult.h:487] [task 2018-04-23T18:38:08.847Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.849Z] 18:38:08 INFO - GECKO(1041) | #02: mozilla::binding_danger::TErrorResult<mozilla::binding_danger::AssertAndSuppressCleanupPolicy>::~TErrorResult [dom/bindings/ErrorResult.h:143] [task 2018-04-23T18:38:08.850Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.852Z] 18:38:08 INFO - GECKO(1041) | #03: mozilla::dom::CustomElementReactionsStack::InvokeReactions [dom/bindings/ErrorResult.h:555] [task 2018-04-23T18:38:08.853Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.855Z] 18:38:08 INFO - GECKO(1041) | #04: mozilla::dom::CustomElementReactionsStack::PopAndInvokeElementQueue [dom/base/CustomElementRegistry.cpp:1136] [task 2018-04-23T18:38:08.855Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.857Z] 18:38:08 INFO - GECKO(1041) | #05: mozilla::Maybe<mozilla::dom::AutoCEReaction>::reset [mfbt/Maybe.h:535] [task 2018-04-23T18:38:08.857Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.862Z] 18:38:08 INFO - GECKO(1041) | #06: nsXBLService::LoadBindings [dom/xbl/nsXBLService.cpp:581] [task 2018-04-23T18:38:08.862Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.864Z] 18:38:08 INFO - GECKO(1041) | #07: nsCSSFrameConstructor::AddFrameConstructionItemsInternal [layout/base/nsCSSFrameConstructor.cpp:5677] [task 2018-04-23T18:38:08.864Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.866Z] 18:38:08 INFO - GECKO(1041) | #08: nsCSSFrameConstructor::DoAddFrameConstructionItems [layout/base/nsCSSFrameConstructor.cpp:5550] [task 2018-04-23T18:38:08.868Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.869Z] 18:38:08 INFO - GECKO(1041) | #09: nsCSSFrameConstructor::AddFrameConstructionItems [layout/base/nsCSSFrameConstructor.cpp:5563] [task 2018-04-23T18:38:08.871Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.873Z] 18:38:08 INFO - GECKO(1041) | #10: nsCSSFrameConstructor::ProcessChildren [layout/base/nsCSSFrameConstructor.cpp:10283] [task 2018-04-23T18:38:08.873Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.876Z] 18:38:08 INFO - GECKO(1041) | #11: nsCSSFrameConstructor::ConstructFrameFromItemInternal [layout/base/nsCSSFrameConstructor.cpp:4037] [task 2018-04-23T18:38:08.877Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.879Z] 18:38:08 INFO - GECKO(1041) | #12: nsCSSFrameConstructor::ConstructFramesFromItem [layout/base/nsCSSFrameConstructor.cpp:6067] [task 2018-04-23T18:38:08.880Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.882Z] 18:38:08 INFO - GECKO(1041) | #13: nsCSSFrameConstructor::ConstructFramesFromItemList [layout/base/nsCSSFrameConstructor.cpp:10104] [task 2018-04-23T18:38:08.882Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.884Z] 18:38:08 INFO - GECKO(1041) | #14: nsCSSFrameConstructor::ProcessChildren [layout/base/nsCSSFrameConstructor.cpp:10304] [task 2018-04-23T18:38:08.885Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.887Z] 18:38:08 INFO - GECKO(1041) | #15: nsCSSFrameConstructor::ConstructFrameFromItemInternal [layout/base/nsCSSFrameConstructor.cpp:4037] [task 2018-04-23T18:38:08.887Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.889Z] 18:38:08 INFO - GECKO(1041) | #16: nsCSSFrameConstructor::ConstructFramesFromItem [layout/base/nsCSSFrameConstructor.cpp:6067] [task 2018-04-23T18:38:08.889Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.892Z] 18:38:08 INFO - GECKO(1041) | #17: nsCSSFrameConstructor::ConstructFramesFromItemList [layout/base/nsCSSFrameConstructor.cpp:10104] [task 2018-04-23T18:38:08.892Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.894Z] 18:38:08 INFO - GECKO(1041) | #18: nsCSSFrameConstructor::ProcessChildren [layout/base/nsCSSFrameConstructor.cpp:10304] [task 2018-04-23T18:38:08.895Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.897Z] 18:38:08 INFO - GECKO(1041) | #19: nsCSSFrameConstructor::ConstructDocElementFrame [layout/base/nsCSSFrameConstructor.cpp:2675] [task 2018-04-23T18:38:08.898Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.900Z] 18:38:08 INFO - GECKO(1041) | #20: nsCSSFrameConstructor::ContentRangeInserted [layout/base/nsCSSFrameConstructor.cpp:7470] [task 2018-04-23T18:38:08.901Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.902Z] 18:38:08 INFO - GECKO(1041) | #21: nsCSSFrameConstructor::ContentInserted [layout/base/nsCSSFrameConstructor.cpp:7366] [task 2018-04-23T18:38:08.903Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.905Z] 18:38:08 INFO - GECKO(1041) | #22: mozilla::PresShell::Initialize [layout/base/PresShell.cpp:1822] [task 2018-04-23T18:38:08.906Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.908Z] 18:38:08 INFO - GECKO(1041) | #23: mozilla::dom::XULDocument::StartLayout [dom/xul/XULDocument.cpp:1617] [task 2018-04-23T18:38:08.909Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.911Z] 18:38:08 INFO - GECKO(1041) | #24: mozilla::dom::XULDocument::DoneWalking [dom/xul/XULDocument.cpp:2793] [task 2018-04-23T18:38:08.912Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.913Z] 18:38:08 INFO - GECKO(1041) | #25: mozilla::dom::XULDocument::ResumeWalk [dom/xul/XULDocument.cpp:2719] [task 2018-04-23T18:38:08.914Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.916Z] 18:38:08 INFO - GECKO(1041) | #26: mozilla::dom::XULDocument::OnScriptCompileComplete [dom/xul/XULDocument.cpp:3233] [task 2018-04-23T18:38:08.916Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.917Z] 18:38:08 INFO - GECKO(1041) | #27: mozilla::dom::XULDocument::OnStreamComplete [dom/xul/XULDocument.cpp:3148] [task 2018-04-23T18:38:08.917Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.919Z] 18:38:08 INFO - GECKO(1041) | #28: mozilla::net::nsStreamLoader::OnStopRequest [netwerk/base/nsStreamLoader.cpp:110] [task 2018-04-23T18:38:08.919Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.920Z] 18:38:08 INFO - GECKO(1041) | #29: nsBaseChannel::OnStopRequest [netwerk/base/nsBaseChannel.cpp:878] [task 2018-04-23T18:38:08.922Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.923Z] 18:38:08 INFO - GECKO(1041) | #30: nsInputStreamPump::OnStateStop [netwerk/base/nsInputStreamPump.cpp:707] [task 2018-04-23T18:38:08.923Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.925Z] 18:38:08 INFO - GECKO(1041) | #31: nsInputStreamPump::OnInputStreamReady [netwerk/base/nsInputStreamPump.cpp:444] [task 2018-04-23T18:38:08.926Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.927Z] 18:38:08 INFO - GECKO(1041) | #32: nsInputStreamReadyEvent::Run [xpcom/io/nsStreamUtils.cpp:102] [task 2018-04-23T18:38:08.928Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.929Z] 18:38:08 INFO - GECKO(1041) | #33: nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1100] [task 2018-04-23T18:38:08.930Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.932Z] 18:38:08 INFO - GECKO(1041) | #34: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:519] [task 2018-04-23T18:38:08.933Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.934Z] 18:38:08 INFO - GECKO(1041) | #35: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:98] [task 2018-04-23T18:38:08.934Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.936Z] 18:38:08 INFO - GECKO(1041) | #36: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327] [task 2018-04-23T18:38:08.936Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.938Z] 18:38:08 INFO - GECKO(1041) | #37: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:298] [task 2018-04-23T18:38:08.939Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.941Z] 18:38:08 INFO - GECKO(1041) | #38: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:159] [task 2018-04-23T18:38:08.942Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.943Z] 18:38:08 INFO - GECKO(1041) | #39: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:291] [task 2018-04-23T18:38:08.944Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.945Z] 18:38:08 INFO - GECKO(1041) | #40: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4835] [task 2018-04-23T18:38:08.947Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.947Z] 18:38:08 INFO - GECKO(1041) | #41: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4979] [task 2018-04-23T18:38:08.947Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.948Z] 18:38:08 INFO - GECKO(1041) | #42: XRE_main [toolkit/xre/nsAppRunner.cpp:5072] [task 2018-04-23T18:38:08.948Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.984Z] 18:38:08 INFO - GECKO(1041) | #43: do_main [browser/app/nsBrowserApp.cpp:231] [task 2018-04-23T18:38:08.984Z] 18:38:08 INFO - [task 2018-04-23T18:38:08.986Z] 18:38:08 INFO - GECKO(1041) | #44: main [browser/app/nsBrowserApp.cpp:306]

Ah, that is interesting and worrisome stack. We can't invoke reactions in that unsafe place. And the assertion happens because of https://searchfox.org/mozilla-central/rev/fcd5cb3515d0e06592bba42e378f1b9518497e3d/dom/base/CustomElementRegistry.cpp#1209

I wonder... perhaps we could just disable the assertion on system principal documents. The assertion was added because per spec upgrade shouldn't in practice ever use backup queue, but the spec doesn't know about XBL. Need to still fix https://searchfox.org/mozilla-central/rev/fcd5cb3515d0e06592bba42e378f1b9518497e3d/dom/base/CustomElementRegistry.cpp#1209 though.

At least (debug) browser starts with this :) But anyway, this might not be too bad. remote: View your changes here: remote: https://hg.mozilla.org/try/rev/7a6fbbf3b1e50d91c056ed55e07a44602399ffd4 remote: https://hg.mozilla.org/try/rev/f702dbc8138e3895f5519bfb670fc6d135784772 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f702dbc8138e3895f5519bfb670fc6d135784772 remote: recorded changegroup in replication log in 0.114s

(In reply to Olli Pettay [:smaug] from comment #19) > Created attachment 8970603 [details] [diff] [review] > ce_in_xbl.diff > > At least (debug) browser starts with this :) > > But anyway, this might not be too bad. > > > remote: View your changes here: > remote: > https://hg.mozilla.org/try/rev/7a6fbbf3b1e50d91c056ed55e07a44602399ffd4 > remote: > https://hg.mozilla.org/try/rev/f702dbc8138e3895f5519bfb670fc6d135784772 > remote: > remote: Follow the progress of your build on Treeherder: > remote: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=f702dbc8138e3895f5519bfb670fc6d135784772 > remote: recorded changegroup in replication log in 0.114s a11y and bc tests look good on that push!

Comment on attachment 8970603 [details] [diff] [review] ce_in_xbl.diff mrbkap, this makes us sort of fallback to backupqueue even for upgrades in case custom element is inside xbl. So when we're instantiating XBL, we need to queue upgrades. Relaxed the assertions only for chrome code, since XBL with CE should run only there and for the web upgrades shouldn't happen using backup queue (only other reactions). It is possible we will need to tweak this while converting more XBL stuff to custom elements, but if we could live with this rather small change, I'd prefer that. Backup queue scheduling should ensure constructor runs at safe time during microtask checkpoint.

Comment on attachment 8970603 [details] [diff] [review] ce_in_xbl.diff Review of attachment 8970603 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/CustomElementRegistry.cpp @@ +1238,5 @@ > > auto& reactions = elementData->mReactionQueue; > for (uint32_t j = 0; j < reactions.Length(); ++j) { > // Transfer the ownership of the entry due to reentrant invocation of > // this funciton. The entry will be removed when bug 1379573 is landed. While reading code and bugs, I noticed that this bug is now WONTFIX. Might as well remove the comment referring to it.

Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6825c9cd5bd5 let custom element upgrades to use backup queue inside chrome/XBL, r=mrbkap