Closed Bug 1356712 Opened 3 years ago Closed 3 months ago

Assertion failure: !(f->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT) (anonymous ib-split block shouldn't have border/background)

Categories

(Core :: Web Painting, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: jkratzer, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, testcase, Whiteboard: [fuzzblocker], [wptsync upstream])

Attachments

(4 files, 3 obsolete files)

Attached file Testcase
Testcase found while fuzzing mozilla-central rev 20170414-1a1069b27f40.

Assertion failure: !(f->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT) (anonymous ib-split block shouldn't have border/background), at /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:539

ASAN:DEADLYSIGNAL
=================================================================
==14644==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f98f6ee980d bp 0x7ffc654483f0 sp 0x7ffc65448320 T0)
==14644==The signal is caused by a WRITE memory access.
==14644==Hint: address points to the zero page.
    #0 0x7f98f6ee980c in JoinBoxesForBlockAxisSlice(nsIFrame*, nsRect const&) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:530:5
    #1 0x7f98f6ee8324 in JoinBoxesForSlice(nsIFrame*, nsRect const&, InlineBoxOrder) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:564:10
    #2 0x7f98f6e628c6 in BoxDecorationRectForBorder(nsIFrame*, nsRect const&, mozilla::Sides, nsStyleBorder const*) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:584:14
    #3 0x7f98f6e72497 in nsCSSRendering::GetImageLayerClip(nsStyleImageLayers::Layer const&, nsIFrame*, nsStyleBorder const&, nsRect const&, nsRect const&, bool, int, nsCSSRendering::ImageLayerClipState*) /home/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:2180:5
    #4 0x7f98f6ed7682 in ComputeMaskGeometry(nsSVGIntegrationUtils::PaintFramesParams&) /home/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:8126:7
    #5 0x7f98f6ed8443 in nsDisplayMask::PaintAsLayer(nsDisplayListBuilder*, nsRenderingContext*, mozilla::layers::LayerManager*) /home/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:8376:3
    #6 0x7f98f6e5d67f in mozilla::PaintInactiveLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsDisplayItem*, gfxContext*, nsRenderingContext*) /home/worker/workspace/build/src/layout/painting/FrameLayerBuilder.cpp:3733:41
    #7 0x7f98f6e5cfdf in mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float, int) /home/worker/workspace/build/src/layout/painting/FrameLayerBuilder.cpp:6063:7
    #8 0x7f98f6e5ef01 in mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*) /home/worker/workspace/build/src/layout/painting/FrameLayerBuilder.cpp:6252:19
    #9 0x7f98f2cf3c8a in mozilla::layers::ClientPaintedLayer::PaintThebes(nsTArray<mozilla::layers::ReadbackProcessor::Update>*) /home/worker/workspace/build/src/gfx/layers/client/ClientPaintedLayer.cpp:86:5
    #10 0x7f98f2cf4550 in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) /home/worker/workspace/build/src/gfx/layers/client/ClientPaintedLayer.cpp:140:3
    #11 0x7f98f2d16410 in mozilla::layers::ClientContainerLayer::RenderLayer() /home/worker/workspace/build/src/gfx/layers/client/ClientContainerLayer.h:57:29
    #12 0x7f98f2d16410 in mozilla::layers::ClientContainerLayer::RenderLayer() /home/worker/workspace/build/src/gfx/layers/client/ClientContainerLayer.h:57:29
    #13 0x7f98f2cefe5e in mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) /home/worker/workspace/build/src/gfx/layers/client/ClientLayerManager.cpp:363:13
    #14 0x7f98f2cf081d in mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) /home/worker/workspace/build/src/gfx/layers/client/ClientLayerManager.cpp:421:3
    #15 0x7f98f6ead520 in nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) /home/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:2254:17
Flags: in-testsuite?
Goes back further than mozregression can bisect, unfortunately. Note that the testcase requires modification for builds predating bug 1300895 (s/column-width/-moz-column-width).
Has Regression Range: --- → no
Priority: -- → P3
We are hitting this frequently while fuzzing.
Flags: needinfo?(mreavy)
Priority: P3 → P1

Maire, Jessie, could you please help finding someone to fix the fuzz blocker?
It means that it is preventing us to fuzz some big part of the code because we hit this issue pretty quickly.

Flags: needinfo?(jbonisteel)

Mats, could you take a look at this? Thanks

Flags: needinfo?(jbonisteel) → needinfo?(mats)

A Pernosco session is available here: https://pernos.co/debug/rNt4Jwec6Q7lJUYh85_59w/index.html

It will expire in 7 days.

Looking at the pernosco link, I think the assertion is just invalid. We have mask inherited into the ib-split anonymous block, and the mask code does compute the box decoration rect. The ib-split sibling still doesn't have borders or backgrounds of course.

Oh, though I guess if it can be reached for those, the idea is that it should use the GetNextContinuationOrIBSplit* stuff?

While IB-split block wrappers cannot indeed have borders or backgrounds, we
inherit mask from the inline into them1, and the masking code can reach this
code too.

I think just using the block continuations rather than walking inlines is the
right thing to do here.

Right, I came to the same conclusion and I'm working on adding some WPTs for the actual layout too, so I should probably take this. https://treeherder.mozilla.org/#/jobs?repo=try&revision=911fe70c1dc415577a544a48d2bf12e3eb86dcad

That said, I'm not really sure there is a spec that defines this clearly...

Assignee: nobody → mats
Flags: needinfo?(mats)

The spec doesn't really address the ib-split case AFAICT: https://drafts.csswg.org/css-break-3/#joining-boxes
I'm guessing the assumption is that block-in-inline splitting is done per CSS2: https://www.w3.org/TR/CSS2/visuren.html#img-anon-block which seems to suggest the embedded block shouldn't be included in the continuation chain at all, i.e. something like this:

<p><span>...<div></div>...</span></p>

becomes:
<p>
  <p:anon-block>
    <span frag1>...</>
  </p:anon-block>
  <div></div>
  <p:anon-block>
    <span frag2>...</>
  </p:anon-block>
</p>

(I think that would violate our box tree invariants though. We'd need to link the two <p:anon-block>s as continuations.)

Yeah, that is probably a no-go. Having such a frame structure would also mean that there'd be no proper way to do opacity on the <span> apply to the <div>, which is required per spec (but Chrome/WebKit don't do it, IIRC).

(In reply to Mats Palmgren (:mats) from comment #10)

Right, I came to the same conclusion and I'm working on adding some WPTs for the actual layout too, so I should probably take this.

Thanks!

Attachment #9105485 - Attachment is obsolete: true

Yeah, the box tree that CSS2 suggests is impractical. I think we want the <div> boxes as children of some <span> box, which is what we have.

no proper way to do opacity on the <span> apply to the <div>, which is required per spec

Do you have a spec/issue link? That strongly suggests that mask-image, filters etc should also be applied on the <div>.

See all the discussion in bug 660682, but basically https://drafts.csswg.org/css-color/#transparency mentions element + descendants, not boxes.

We apply opacity also to stuff like fixed pos and such:

<!doctype html>
<div style="opacity: 0.5">
  <div style="position: fixed; height: 100px; width: 100px; background: green"></div>
</div>

Yeah, it seems Chrome has no problem applying these on fixed pos child boxes.

Chrome is at least consistent in NOT applying them to block-in-inline child boxes.
(Whereas Gecko do apply these consistently.)

Attachment #9105488 - Attachment is obsolete: true

The spec language at:
https://drafts.fxtf.org/css-masking-1/#the-mask-image

A computed value of other than none results in the creation of a stacking context [CSS21] the same way that CSS opacity [CSS3COLOR] does for values other than 1.

seems clear that all these properties should be handled the same as opacity, so they should be applied also for the block-in-inline case.

Which leaves my original question unanswered: how are these fragments supposed to be joined together?

I think our current layout is reasonable. Basically, treat the inline fragments as if the blocks don't exist, and the blocks (each block element separately though) as if the inlines don't exist. I'll file a spec issue though, because the spec needs to define this more precisely...

Attachment #9105498 - Attachment is obsolete: true
Flags: needinfo?(mreavy)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20219 for changes under testing/web-platform/tests
Whiteboard: [fuzzblocker] → [fuzzblocker], [wptsync upstream]
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Upstream PR merged by moz-wptsync-bot
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.