Closed Bug 1425959 Opened 2 years ago Closed 2 years ago

Assertion failure: providerFrame && providerFrame->GetParent()-> IsFrameOfType(nsIFrame::eBlockFrame) (How could we get a ::first-line parent style without having a ::first-line provider frame?), at /src/layout/base/ServoRestyleManager.cpp:1618

Categories

(Core :: Layout, defect)

57 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: tsmith, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file testcase.html
Assertion failure: providerFrame && providerFrame->GetParent()-> IsFrameOfType(nsIFrame::eBlockFrame) (How could we get a ::first-line parent style without having a ::first-line provider frame?), at /src/layout/base/ServoRestyleManager.cpp:1618

#0 mozilla::ServoRestyleManager::DoReparentStyleContext(nsIFrame*, mozilla::ServoStyleSet&) /src/layout/base/ServoRestyleManager.cpp:1560:5
#1 mozilla::ServoRestyleManager::ReparentStyleContext(nsIFrame*) /src/layout/base/ServoRestyleManager.cpp:1521:3
#2 nsCSSFrameConstructor::CheckForFirstLineInsertion(nsIFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:11316:23
#3 nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind, TreeMatchContext*) /src/layout/base/nsCSSFrameConstructor.cpp:8254:5
#4 mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /src/layout/base/RestyleManager.cpp:1419:27
#5 mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1161:9
#6 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4185:41
#7 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1891:18
#8 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1862:12
#9 nsRefreshDriver::FinishedWaitingForTransaction() /src/layout/base/nsRefreshDriver.cpp:2154:5
#10 mozilla::layers::ClientLayerManager::DidComposite(unsigned long, mozilla::TimeStamp const&, mozilla::TimeStamp const&) /src/gfx/layers/client/ClientLayerManager.cpp:520:32
#11 mozilla::layers::CompositorBridgeChild::RecvDidComposite(unsigned long const&, unsigned long const&, mozilla::TimeStamp const&, mozilla::TimeStamp const&) /src/gfx/layers/ipc/CompositorBridgeChild.cpp:542:8
#12 mozilla::layers::PCompositorBridgeChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PCompositorBridgeChild.cpp:1441:20
#13 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2110:25
#14 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2040:17
#15 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1886:5
#16 mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1919:15
#17 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#18 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:508:10
#19 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#20 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#21 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#22 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27
#23 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
#24 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4709:22
#25 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4871:8
#26 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4963:21
#27 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:231:22
#28 main /src/browser/app/nsBrowserApp.cpp:304:16
#29 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#30 _start (firefox+0x41f3d4)
Flags: in-testsuite?
Huh, so this is because the <button> style has the ALLOW_BLOCK_STYLES bit in the frame constructor, based on the assumption that it always create a block wrapper for its kids, yet we have code from bug 984869 which doesn't create a block frame, but a grid container frame...
Yeah, I think we need to enforce that the parent actually is a block frame too.
I'll take a look...
Assignee: nobody → mats
Comment on attachment 8937758 [details] [diff] [review]
Only allow ::first-letter/line children when the parent frame is a block frame

Review of attachment 8937758 [details] [diff] [review]:
-----------------------------------------------------------------

Heh, I had pretty much pushed the same patch to try a bit ago :)

::: layout/base/crashtests/1425959.html
@@ +8,5 @@
> +}
> +</script>
> +<body onload=go()>
> +<button id="a">
> +<video id="b">

You can probably make this something that isn't a <video> so the test-case is a bit simpler, but no big deal if it takes much effort.

::: layout/base/nsCSSFrameConstructor.cpp
@@ +10998,5 @@
>  
>    // XXXbz ideally, this would do all the pushing of various
>    // containing blocks as needed, so callers don't have to do it...
>  
> +  const bool allowFirstPseudos = aAllowBlockStyles &&

Can you leave a comment saying that we need to check that the frame is actually a block because we may create non-block wrappers for <button>, or something along those lines?
Attachment #8937758 - Flags: review?(emilio) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e28372a90098
Only allow ::first-letter/line children when the parent frame is a block frame.  r=emilio
(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> You can probably make this something that isn't a <video> so the test-case
> is a bit simpler, but no big deal if it takes much effort.

Sure, the assertion also occurs with <span id="b">.

> Can you leave a comment saying that we need to check that the frame is
> actually a block because we may create non-block wrappers for <button>, or
> something along those lines?

OK, I added a comment and clarified that FCDATA_ALLOW_BLOCK_STYLES
doesn't imply that the parent frame is an actual block (it probably
used to, but I think it's less error prone if it doesn't now that we
allow display:grid/flexbox in a bunch of more places).
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/e28372a90098
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
mozregression says this first started asserting when bug 1388877 landed, but I'm guessing this isn't severe enough to warrant uplift consideration, especially this late in the cycle.
Blocks: 1388877
Version: 59 Branch → 57 Branch
You need to log in before you can comment on or make changes to this bug.