The attached testcase causes an assertion in m-c rev a6a1f5c1d971 with stylo enabled by pref. Assertion failure: ExpectedOwnerForChild(*mOwner) == aParent.mOwner, at /home/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:109 #0: mozilla::ServoRestyleState::AssertOwner (layout/base/ServoRestyleManager.cpp:105) #1: mozilla::ServoRestyleState::ProcessMaybeNestedWrapperRestyle (mfbt/Maybe.h:459) #2: mozilla::ServoRestyleState::ProcessWrapperRestyles (layout/base/ServoRestyleManager.cpp:168) #3: mozilla::ServoRestyleManager::ProcessPostTraversal (layout/base/ServoRestyleManager.cpp:889) #4: mozilla::ServoRestyleManager::ProcessPostTraversal (layout/base/ServoRestyleManager.cpp:874) #5: mozilla::ServoRestyleManager::DoProcessPendingRestyles (layout/base/ServoRestyleManager.cpp:1095) #6: mozilla::PresShell::DoFlushPendingNotifications (layout/base/PresShell.cpp:4200)

I'm looking into it.

Can't replicate, though I was able to spot it in a slightly older Nightly. Something has resolved it, apparently.

Chances are, the first hunk of https://hg.mozilla.org/mozilla-central/rev/ca0fcfb8cec2 fixed this, but getting the first continuation correctly when getting the parent of an anon box. In this case, the parent of the anon table wrapping the <tr>. And we have multiple continuations here because of the bidi bits.

Here's another that repros in m-c rev 20170905-3ecda4678c49.

Yes, this testcase reproduces the error for me. I'll investigate.

Here's a bit more information on what's going on: The assert is firing on a ServoRestyleState with an mOwner of an nsBlockFrame which is an anonymous box. The parent of this frame is an nsColumnSetFrame, and that is what is being returned as the ExpectedOwnerForChild. The aParent ServoRestyleState mOwner however is an nsHTMLScrollFrame. My understanding of this is poor. I'm taking myself off the bug and needinfo-ing Xidorn (author of the fix for Bug 1390389, likely related).

So with testcase2.html we have a scrollable columnset. When the assert fires, we have: mOwner == :-moz-column-content block inside the columnset (0x7fa2f34f3450) aParent.mOwner == scrollframe for the scrollable columnset (0x7fa2f34f2ca0) ExpectedOwnerForChild(*mOwner) == :-moz-scrolled-content columnset (0x7fa2f34f3500) Relevant frametree dump bit: HTMLScroll(div)(3)@7fa2f34f2ca0 {0,480,76800,3480} [state=0080020000000010] [content=7fa2f33df820] [sc=7fa2cde85f68]< ColumnSet(div)(3)@7fa2f34f3500 {0,0,76080,0} vis-overflow=0,0,76080,2760 scr-overflow=0,0,76080,2760 [state=0080000000400220] [content=7fa2f33df820] [sc=7fa2cde87288:-moz-scrolled-content]< Block(div)(3)@7fa2f34f3450 {0,0,3,0} [state=0000100008d00000] [content=7fa2f33df820] [sc=7fa2e7a994a8:-moz-column-content]< line 7fa2f34f3808: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x108] {0,0,0,0} < TableWrapper(div)(3)@7fa2f34f3590 {0,0,0,0} [state=0000000000000200] [content=7fa2f33df820] [sc=7fa2e7a9a8e8:-moz-table-wrapper]< Table(div)(3)@7fa2f34f3628 {0,0,0,0} [state=0080000000000000] [content=7fa2f33df820] [sc=7fa2e7a9a7c8:-moz-table]<> This is all happening from mozilla::ServoRestyleState::ProcessMaybeNestedWrapperRestyle where aParent is the scrollframe. This all sort of makes sense: we have a table-caption kid inside the scrollable columnset. So when we process the columnset's DOM kids we queue up the anonymous table wrapping the table caption as a thing that needs recomputation of its style. We look up the "parent" for that anonymous table in ProcessMaybeNestedWrapperRestyle and find the column-content block. This does not match aParent, so we try to construct a ServoRestyleState for the column-content block. But that has the columnset as its ExpectedOwnerForChild, not the scrollframe. I guess we could try to build up a stack of ServoRestyleStates here somehow. Or somehow loosen the assertion to allow this case... It seems like it shouldn't actually matter thatn we never created a ServoRestyleState for the columnset itself in this case, right?

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #7) > I guess we could try to build up a stack of ServoRestyleStates here somehow. > Or somehow loosen the assertion to allow this case... It seems like it > shouldn't actually matter thatn we never created a ServoRestyleState for the > columnset itself in this case, right? Yeah, I don't think it matters at all in this case, so relaxing the assertion sounds ok.

Emilio, can you relax the assertion then?

Apparently I'm neither the author nor the reviewer of the fix for bug 1390389.

Oh, wait, I was canceling ni? me... Set again for emilio...

Boris said yesterday that he could look into relaxing the assertion. I'd appreciate it since he's looked at the test-case more closely, and has all the anon-box wrapper restyle stuff that I only skimmed.

fyi, bughunter reproduced on Windows and Linux using <http://www.indonesia-tourism.com/forum/showthread.php?528-Kadriah-Palace-Istana-Kadriah-West-Kalimantan-Indonesia>. I reproduced on Linux with a build from this morning.

Comment on attachment 8914445 [details] [diff] [review] Relax the ExpectedOwnerForChild assert in the ServoRestyleState constructor Review of attachment 8914445 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/ServoRestyleManager.cpp @@ +120,5 @@ > + // the root of the tree. We also allow aParent.mOwner to be somewhere up our > + // expected owner chain not our immediate owner, which allows us creating long > + // chains of ServoRestyleStates in some cases where it's just not worth it. > +#ifdef DEBUG > + if (aParent.mOwner) { Can we assert that if it isn't the first, there is an anon box involved? Don't worry too much if you think it's not worth it. ::: layout/base/crashtests/1391736.html @@ +10,5 @@ > + document.documentElement.appendChild(a) > + a.style.overflow = "scroll" > + a.style.columnWidth = "calc(-15px)" > + b.style.display = "table-caption" > + requestIdleCallback(() => { I think you need to use reftest-wait here, what guarantees that this callback runs?

Did that; could use another once-over

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ce3e3c2f1598 Relax the ExpectedOwnerForChild assert in the ServoRestyleState constructor. r=emilio

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

Comment on attachment 8914450 [details] [diff] [review] Relax the ExpectedOwnerForChild assert in the ServoRestyleState constructor Approval Request Comment [Feature/Bug causing the regression]: Stylo [User impact if declined]: None, this is debug-only code. But this is interfering 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]: No. [List of other uplifts needed for the feature/fix]: None. This patch is on top of the patch for bug 1402476 just in terms of simpler merging. [Is the change risky?]: No. [Why is the change risky/not risky?]: Debug-only code. [String changes made/needed]: None.

Comment on attachment 8914450 [details] [diff] [review] Relax the ExpectedOwnerForChild assert in the ServoRestyleState constructor Stylo related, with automated test (awesome!), Beta57+