The attached testcase causes an assertion in m-c rev 20170920-469eb992a9d1 Assertion failure: nsLayoutUtils::FirstContinuationOrIBSplitSibling(parent) == this (This should only be used for children!), at /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:10234 #0: nsIFrame::UpdateStyleOfChildAnonBox, at layout/generic/nsFrame.cpp:10236 #1: mozilla::ServoRestyleState::ProcessMaybeNestedWrapperRestyle, at layout/base/ServoRestyleManager.cpp:227 #2: mozilla::ServoRestyleState::ProcessWrapperRestyles, at layout/base/ServoRestyleManager.cpp:169 #3: mozilla::ServoRestyleManager::ProcessPostTraversal, at layout/base/ServoRestyleManager.cpp:938 #4: mozilla::ServoRestyleManager::DoProcessPendingRestyles, at layout/base/ServoRestyleManager.cpp:1121 #5: mozilla::PresShell::DoFlushPendingNotifications, at layout/base/PresShell.cpp:4170 #6: mozilla::PresShell::DoFlushPendingNotifications, at layout/base/PresShell.cpp:4043 #7: nsDocument::FlushPendingNotifications, at dom/base/nsDocument.cpp:8345 #8: nsDocLoader::DocLoaderIsEmpty, at uriloader/base/nsDocLoader.cpp:703 #9: nsDocLoader::OnStopRequest, at uriloader/base/nsDocLoader.cpp:632 #10: mozilla::net::nsLoadGroup::RemoveRequest, at netwerk/base/nsLoadGroup.cpp:629 #11: nsDocument::DoUnblockOnload, at dom/base/nsDocument.cpp:9173 #12: nsDocument::UnblockOnload, at dom/base/nsDocument.cpp:9095 #13: nsDocument::DispatchContentLoadedEvents, at dom/base/nsDocument.cpp:5599 #14: mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0u>::Run, at xpcom/threads/nsThreadUtils.h:1142 #15: mozilla::SchedulerGroup::Runnable::Run, at xpcom/threads/SchedulerGroup.cpp:396 #16: nsThread::ProcessNextEvent, at xpcom/threads/nsThread.cpp:1039 #17: NS_ProcessNextEvent, at xpcom/threads/nsThreadUtils.cpp:521 #18: mozilla::ipc::MessagePump::Run, at ipc/glue/MessagePump.cpp:97 #19: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326 #20: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319 #21: nsBaseAppShell::Run, at widget/nsBaseAppShell.cpp:158 #22: XRE_RunAppShell, at toolkit/xre/nsEmbedFunctions.cpp:880 #23: mozilla::ipc::MessagePumpForChildProcess::Run, at ipc/glue/MessagePump.cpp:269 #24: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326 #25: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319 #26: XRE_InitChildProcess, at toolkit/xre/nsEmbedFunctions.cpp:705 #27: content_process_main, at ipc/contentproc/plugin-container.cpp:63 #28: main, at browser/app/nsBrowserApp.cpp:285 #29: libc-2.26.so+0x20f6a #30: MOZ_ReportAssertionFailure, at mfbt/Assertions.h:165

aChildFrame in this function is a table frame, and thus the parent here ends up being a table wrapper frame. "this" points to the parent of the table wrapper frame (which is a block frame).

Requesting tracking on all outstanding p2 stylo bugs.

Oh, hmm, I was wrong. parent at that moment does point to the block frame which is the parent of the table wrapper frame. The problem here is that, the block frame "parent" points to is not the first continuation. I'm not sure what is the correct fix here. I guess either the assertion is invalid, or "parentForRestyle" in ServoRestyleState::ProcessMaybeNestedWrapperRestyle should be assigned to nsLayoutUtils::FirstContinuationOrIBSplitSibling(parent) rather than parent at ServoRestyleManager.cpp:219.

I think we should loosen the assert at the beginning of nsIFrame::UpdateStyleOfChildAnonBox. Using the first continuation for parentForRestyle isn't right, because of first-line bits. I'll post a patch with some comments.

> Using the first continuation for parentForRestyle isn't right, because of first-line bits. No, that's not true. Let me poke at this some more.

MozReview-Commit-ID: KhfvBuCeoex

MozReview-Commit-ID: KhfvBuCeoex

Comment on attachment 8911337 [details] [diff] [review] Make sure to always call UpdateStyleOfChildAnonBox with the first continuation as "this" Review of attachment 8911337 [details] [diff] [review]: ----------------------------------------------------------------- This looks like a reasonable fix, but emilio should be a better reviewer here given he reviewed bug 1390389 (where you incorrectly marked r=me). I'm confused that, wouldn't this make a unrelated frame inherit something which has taken :first-line into account? Or :first-line fixup always happens after this?

Comment on attachment 8911337 [details] [diff] [review] Make sure to always call UpdateStyleOfChildAnonBox with the first continuation as "this" Review of attachment 8911337 [details] [diff] [review]: ----------------------------------------------------------------- This looks ok, given it happens before the first line reparenting stuff. r=me

> Or :first-line fixup always happens after this? Exactly. At this point, the ::first-line styles are not in play yet.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b97d4e53e02 Make sure to always call UpdateStyleOfChildAnonBox with the first continuation as "this". r=emilio

Comment on attachment 8911337 [details] [diff] [review] Make sure to always call UpdateStyleOfChildAnonBox with the first continuation as "this" Should probably backport this to 57. Approval Request Comment [Feature/Bug causing the regression]: Stylo. [User impact if declined]: No immediate impact, but interferes with fuzzing. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: In my opinion, no. [Why is the change risky/not risky?]: It just uses a different frame with the same style, to satisfy an assertion. [String changes made/needed]: None.

Comment on attachment 8911337 [details] [diff] [review] Make sure to always call UpdateStyleOfChildAnonBox with the first continuation as "this" Fix some assert issue, taking it. Should be in 57b4