1402472 - stylo: Assertion failure: !aElement->HasFlag(ELEMENT_HANDLED_SNAPSHOT)

Status: RESOLVED FIXED

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

Description

[Jesse Schwartzentruber (:truber)]

Attachment: testcase.html

The attached testcase causes an assertion in m-c rev 20170922-14db7c0bcf9a

Assertion failure: !aElement->HasFlag(ELEMENT_HANDLED_SNAPSHOT), at /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:1043
#0: mozilla::ServoRestyleManager::SnapshotFor, at layout/base/ServoRestyleManager.cpp:1042
#1: mozilla::ServoRestyleManager::ContentStateChanged, at layout/base/ServoRestyleManager.cpp:1252
#2: mozilla::PresShell::ContentStateChanged, at layout/base/RestyleManagerInlines.h:51
#3: nsDocument::ContentStateChanged, at dom/base/nsDocument.cpp:5666
#4: mozilla::dom::Element::UpdateState, at dom/base/Element.cpp:272
#5: NotifyEditableStateChange, at dom/html/nsHTMLDocument.cpp:2629
#6: NotifyEditableStateChange, at dom/html/nsHTMLDocument.cpp:2631
#7: NotifyEditableStateChange, at dom/html/nsHTMLDocument.cpp:2631
#8: nsHTMLDocument::EditingStateChanged, at dom/html/nsHTMLDocument.cpp:2912
#9: nsHTMLDocument::SetDesignMode, at dom/html/nsHTMLDocument.cpp:2968
#10: nsHTMLDocument::SetDesignMode, at dom/html/nsHTMLDocument.cpp:2951
#11: mozilla::dom::HTMLDocumentBinding::set_designMode, at 6deb547566855812e17bc597f66751a8b0933de60f01e6509c3cb2cb3932d79f4ad1128a332976e79827fb9a066fe9feab2cdc99c59aeedf8472ea59d56b348c/dom/bindings/HTMLDocumentBinding.cpp:806
#12: mozilla::dom::GenericBindingSetter, at dom/bindings/BindingUtils.cpp:3016
#13: js::CallJSNative, at js/src/jscntxtinlines.h:293
#14: js::InternalCallOrConstruct, at js/src/vm/Interpreter.cpp:495
#15: InternalCall, at js/src/vm/Interpreter.cpp:540
#16: js::Call, at js/src/vm/Interpreter.cpp:559
#17: js::CallSetter, at js/src/vm/Interpreter.cpp:688
#18: SetExistingProperty, at js/src/vm/NativeObject.cpp:2755
#19: js::NativeSetProperty<(js::QualifiedBool)1u>, at js/src/vm/NativeObject.cpp:2791
#20: js::SetPropertyIgnoringNamedGetter, at js/src/vm/NativeObject.h:1607
#21: mozilla::dom::DOMProxyHandler::set, at dom/bindings/DOMJSProxyHandler.cpp:221
#22: js::Proxy::setInternal, at js/src/proxy/Proxy.cpp:390
#23: js::Proxy::set, at js/src/proxy/Proxy.cpp:400
#24: JSObject::nonNativeSetProperty, at js/src/jsobj.cpp:1031
#25: js::SetProperty, at js/src/vm/NativeObject.h:1606
#26: Interpret, at js/src/vm/Interpreter.cpp:269
#27: js::RunScript, at js/src/vm/Interpreter.cpp:435
#28: js::ExecuteKernel, at js/src/vm/Interpreter.cpp:724
#29: js::Execute, at js/src/vm/Interpreter.cpp:757
#30: ExecuteScript, at js/src/jsapi.cpp:4648
#31: ExecuteScript, at js/src/jsapi.cpp:4667
#32: nsJSUtils::ExecutionContext::CompileAndExec, at dom/base/nsJSUtils.cpp:265
#33: mozilla::dom::ScriptLoader::EvaluateScript, at dom/script/ScriptLoader.cpp:2244
#34: mozilla::dom::ScriptLoader::ProcessRequest, at dom/script/ScriptLoader.cpp:1884
#35: mozilla::dom::ScriptLoader::ProcessScriptElement, at dom/script/ScriptLoader.cpp:1585
#36: mozilla::dom::ScriptElement::MaybeProcessScript, at dom/script/ScriptElement.cpp:149
#37: nsIScriptElement::AttemptToExecute, at dom/script/nsIScriptElement.h:225
#38: nsHtml5TreeOpExecutor::RunScript, at parser/html/nsHtml5TreeOpExecutor.cpp:701
#39: nsHtml5TreeOpExecutor::RunFlushLoop, at parser/html/nsHtml5TreeOpExecutor.cpp:502
#40: nsHtml5ExecutorFlusher::Run, at parser/html/nsHtml5StreamParser.cpp:130
#41: mozilla::SchedulerGroup::Runnable::Run, at xpcom/threads/SchedulerGroup.cpp:396
#42: nsThread::ProcessNextEvent, at xpcom/threads/nsThread.cpp:1039
#43: NS_ProcessNextEvent, at xpcom/threads/nsThreadUtils.cpp:521
#44: mozilla::ipc::MessagePump::Run, at ipc/glue/MessagePump.cpp:97
#45: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326
#46: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319
#47: nsBaseAppShell::Run, at widget/nsBaseAppShell.cpp:158
#48: XRE_RunAppShell, at toolkit/xre/nsEmbedFunctions.cpp:880
#49: mozilla::ipc::MessagePumpForChildProcess::Run, at ipc/glue/MessagePump.cpp:269
#50: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326
#51: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319
#52: XRE_InitChildProcess, at toolkit/xre/nsEmbedFunctions.cpp:705
#53: content_process_main, at ipc/contentproc/plugin-container.cpp:63
#54: main, at browser/app/nsBrowserApp.cpp:285
#55: libc-2.26.so+0x20f6a
#56: MOZ_ReportAssertionFailure, at mfbt/Assertions.h:165

Comment 1

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: 6cdbab15f8de44b3995125f468a525d2aceb9d0d
INFO: First bad revision: 1a8d86baaabc1af9d2359a0f078a9ee50aec9351
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6cdbab15f8de44b3995125f468a525d2aceb9d0d&tochange=1a8d86baaabc1af9d2359a0f078a9ee50aec9351

Bug 1389347 I presume.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I got this.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1402472: Make sure to not run any style invalidation in an unstyled children only traversal.

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

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8911438 [details]
Bug 1402472: Make sure to not run any style invalidation in an unstyled children only traversal.

https://reviewboard.mozilla.org/r/182908/#review188118

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Servo bits landed in https://github.com/servo/servo/pull/18602

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d22408a702ec
Crashtest. r=me

Comment 7

[Bobby Holley (:bholley)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5)
> Servo bits landed in https://github.com/servo/servo/pull/18602

(which landed as https://hg.mozilla.org/integration/autoland/rev/888ca78cd6ec )

Comment 8

[Phil Ringnalda (:philor)]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8911438 [details]
Bug 1402472: Make sure to not run any style invalidation in an unstyled children only traversal.

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: Potentially wrong styling in some edge cases.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: 
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not really
[Why is the change risky/not risky?]: three-liner patch that avoids running some invalidation until it's actually needed.
[String changes made/needed]: none

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8911438 [details]
Bug 1402472: Make sure to not run any style invalidation in an unstyled children only traversal.

Improve stylo, taking it
Should be in 57b3

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1512bd018892
https://hg.mozilla.org/releases/mozilla-beta/rev/29da5f3b170d

Comment 13

[Andrei Vaida [:avaida]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #10)
> [Is this code covered by automated tests?]: Yes
> [Has the fix been verified in Nightly?]: 
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Emilio's assessment on manual testing needs and the fact that this fix has automated coverage.