1383845 - Stylo: Windows debug reftests / crashtests fail with "Assertion failure: cachedBinding == aProtoBinding"

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

In bug 1380053, we are working on enabling Stylo tests for more desktop platforms like Windows.  (You may want the patches in that bug to test this issue on try.)

Windows debug (both 32 and 64 bit) Stylo crashtest runs[1] and some Stylo vs. Gecko reftest runs[2] fail with the following assertion:

Assertion failure: cachedBinding == aProtoBinding, at z:/build/build/src/dom/xbl/nsXBLBinding.cpp:1043
#01: nsXBLPrototypeBinding::InitClass(nsString const &,JSContext *,JS::Handle<JSObject *>,JS::MutableHandle<JSObject *>,bool *) [dom/xbl/nsXBLPrototypeBinding.cpp:460]
#02: nsXBLProtoImpl::InitTargetObjects(nsXBLPrototypeBinding *,nsIContent *,JS::MutableHandle<JSObject *>,bool *) [dom/xbl/nsXBLProtoImpl.cpp:229]
#03: nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding *,nsXBLBinding *) [dom/xbl/nsXBLProtoImpl.cpp:70]
#04: nsXBLPrototypeBinding::InstallImplementation(nsXBLBinding *) [dom/xbl/nsXBLPrototypeBinding.cpp:317]
#05: nsXBLBinding::InstallImplementation() [dom/xbl/nsXBLBinding.cpp:587]
#06: nsXBLService::LoadBindings(nsIContent *,nsIURI *,nsIPrincipal *,nsXBLBinding * *,bool *) [dom/xbl/nsXBLService.cpp:536]
#07: nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState &,nsIContent *,nsContainerFrame *,nsIAtom *,int,bool,nsStyleContext *,unsigned int,nsTArray<nsIAnonymousContentCreator::ContentInfo> *,nsCSSFrameConstructor::FrameConstructionItemList &) [layout/base/nsCSSFrameConstructor.cpp:5869]
#08: nsCSSFrameConstructor::AddFCItemsForAnonymousContent(nsFrameConstructorState &,nsContainerFrame *,nsTArray<nsIAnonymousContentCreator::ContentInfo> &,nsCSSFrameConstructor::FrameConstructionItemList &,unsigned int) [layout/base/nsCSSFrameConstructor.cpp:11154]
#09: nsCSSFrameConstructor::BeginBuildingScrollFrame(nsFrameConstructorState &,nsIContent *,nsStyleContext *,nsContainerFrame *,nsIAtom *,bool,nsContainerFrame * &) [layout/base/nsCSSFrameConstructor.cpp:4697]
#10: nsCSSFrameConstructor::SetUpDocElementContainingBlock(nsIContent *) [layout/base/nsCSSFrameConstructor.cpp:2993]
#11: nsCSSFrameConstructor::ConstructDocElementFrame(mozilla::dom::Element *,nsILayoutHistoryState *) [layout/base/nsCSSFrameConstructor.cpp:2494]
#12: nsCSSFrameConstructor::ContentRangeInserted(nsIContent *,nsIContent *,nsIContent *,nsILayoutHistoryState *,bool,bool,TreeMatchContext *) [layout/base/nsCSSFrameConstructor.cpp:8047]
#13: nsCSSFrameConstructor::ContentInserted(nsIContent *,nsIContent *,nsILayoutHistoryState *,bool) [layout/base/nsCSSFrameConstructor.cpp:7939]
#14: mozilla::PresShell::Initialize(int,int) [layout/base/PresShell.cpp:1809]
#15: nsContentSink::StartLayout(bool) [dom/base/nsContentSink.cpp:1263]
#16: nsHtml5TreeOpExecutor::StartLayout(bool *) [parser/html/nsHtml5TreeOpExecutor.cpp:632]
#17: nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor *,nsIContent * *,bool *) [parser/html/nsHtml5TreeOperation.cpp:974]
#18: nsHtml5TreeOpExecutor::RunFlushLoop() [parser/html/nsHtml5TreeOpExecutor.cpp:464]
#19: nsHtml5ExecutorFlusher::Run() [parser/html/nsHtml5StreamParser.cpp:130]
#20: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1580]
#21: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:530]
#22: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:97]
#23: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:321]
#24: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:315]
#25: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:295]
#26: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:158]
#27: nsAppShell::Run() [widget/windows/nsAppShell.cpp:271]
#28: nsAppStartup::Run() [toolkit/components/startup/nsAppStartup.cpp:288]
#29: XREMain::XRE_mainRun() [toolkit/xre/nsAppRunner.cpp:4596]
#30: XREMain::XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/nsAppRunner.cpp:4779]
#31: XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/nsAppRunner.cpp:4874]
#32: mozilla::BootstrapImpl::XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/Bootstrap.cpp:45]
#33: do_main [browser/app/nsBrowserApp.cpp:237]
#34: NS_internal_main(int,char * *,char * *) [browser/app/nsBrowserApp.cpp:309]
#35: wmain [toolkit/xre/nsWindowsWMain.cpp:118]
#36: __scrt_common_main_seh [f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253]
#37: kernel32.dll + 0x53c45
#38: ntdll.dll + 0x637f5
#39: ntdll.dll + 0x637c8

This seems to show up while running the following tests:

* docshell/base/crashtests/914521.html
* dom/base/crashtests/610571-1.html
* dom/base/crashtests/828903.html
* dom/base/crashtests/structured_clone_container_throws.html
* dom/xslt/crashtests/603844.html
* layout/reftests/bugs/306630-1.html (stylo vs. gecko mode)

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74dab813af8b25642919c747cd0cef9b3f25500e&filter-searchStr=windows%20stylo%20debug%20crash
[2]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74dab813af8b25642919c747cd0cef9b3f25500e&filter-searchStr=windows%20stylo%20debug%20rs5

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Maybe :TYLin has some ideas here from past work on XBL and Stylo.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Sounds like we're trying to set up both stylo and non-stylo versions of the same prototype binding?

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I have no idea on top of my head why this assertion happens only on Windows. I'll need to find a windows machine to reproduce this locally.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Just so it's clear here, the tests that cause this issue are currently skipped in the reftest lists so that we can start running Windows in general.  Search for this bug number to find the skipped tests, since you'll want to re-enable them when testing a fix.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

I cannot reproduce this crash locally with dom/base/crashtests/610571-1.html and dom/base/crashtests/828903.html (via opening them directly, because I somehow cannot run reftest/crashtest locally because of bug 1386502).

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

So, I tested locally (with sandbox disabled via MOZ_DISABLE_CONTENT_SANDBOX=1 to workaround bug 1386502), and I still cannot reproduce this crash with any of the four tests marked with this bug number even with running the whole test set in dom/base/crashtests.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

... but today I reproduced this with an a11y test accessible/tests/mochitest/events/test_docload.html

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

So at least in accessible/tests/mochitest/events/test_docload.html, it seems the issue is that, cachedBinding uses Servo backend, while aProtoBinding uses Gecko backend, which causes this inconsistency. And in this case, the binding in question is the resizer.

For test_docload.html, this is probably because of the "about:" and "about:buildconfig" pages in iframes, which may still using Gecko backend. This is inevitable for now given we don't really want to enable stylo on about pages at the moment (bug 1376805), although I guess "about:" and "about:buildconfig" may actually be fine to use stylo.

All reftests and crashtests which have this bug annotated involves some kind of iframe (or frame) with either data URI or operation in empty documents, which, I suspect, should have been fixed in bug 1384162.

I have no idea why this happens on Windows only, though. It seems to me this really should be a problem across all platforms.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

There is a try push with all annotation related to this bug removed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd436b67bf5241b0ba46d27900d24ba890454af3

We'll see what this would look like.

Comment 10

[Bobby Holley (:bholley)]

See also my patches in bug 1389300, which try to be more consistent about our style backend in various places.

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

So, it looks like everything is fine.

We may need to handle this for accessible/tests/mochitest/events/test_docload.html somehow, but that can probably be in a separate bug.

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1383845 - Enable tests disabled for bug 1383845.

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

Comment 13

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8898089 [details]
Bug 1383845 - Enable tests disabled for bug 1383845.

https://reviewboard.mozilla.org/r/169420/#review174752

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d344fb6fcf6d
Enable tests disabled for bug 1383845. r=TYLin

Comment 15

[Carsten Book [:Tomcat]]

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