J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Reporter
	Description • 7 years ago

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

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Reporter
	Comment 1 • 7 years ago

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

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 7 years ago

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

	Ting-Yu Lin [:TYLin] (UTC-8)
	Comment 3 • 7 years ago

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.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Reporter
	Comment 4 • 7 years ago

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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 5 • 7 years ago

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).

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 6 • 7 years ago

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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 7 • 7 years ago

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

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 8 • 7 years ago

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.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 9 • 7 years ago

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.

	Bobby Holley (:bholley)
	Comment 10 • 7 years ago

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

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 11 • 7 years ago

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.

	Ting-Yu Lin [:TYLin] (UTC-8)
	Comment 13 • 7 years ago
mozreview-review

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

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

	Pulsebot
	Comment 14 • 7 years ago

Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d344fb6fcf6d
Enable tests disabled for bug 1383845. r=TYLin

	Carsten Book [:Tomcat]
	Comment 15 • 7 years ago
bugherder

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