Closed Bug 1477847 Opened 2 years ago Closed 2 years ago

Assertion failure: itemClip.GetRoundedRectCount() == 0, at src/layout/painting/FrameLayerBuilder.cpp:5026

Categories

(Core :: Web Painting, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: tsmith, Assigned: botond)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase)

Attachments

(6 files)

Attached file testcase.html
Reduced with m-c:
BuildID=20180723154916
SourceStamp=ff3fab43d24dfdaa8971d92cc4caaf4dc9f54dba

Assertion failure: itemClip.GetRoundedRectCount() == 0, at src/layout/painting/FrameLayerBuilder.cpp:5026

#0 mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) src/layout/painting/FrameLayerBuilder.cpp:5162:9
#1 mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) src/layout/painting/FrameLayerBuilder.cpp:6343:9
#2 nsDisplayFilter::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) src/layout/painting/nsDisplayList.cpp:9571:5
#3 mozilla::FrameLayerBuilder::AddPaintedDisplayItem(mozilla::PaintedLayerData*, mozilla::AssignedDisplayItem&, mozilla::ContainerState&, mozilla::layers::Layer*) src/layout/painting/FrameLayerBuilder.cpp:5439:20
#4 void mozilla::ContainerState::FinishPaintedLayerData<mozilla::PaintedLayerDataNode::PopAllPaintedLayerData()::$_0>(mozilla::PaintedLayerData&, mozilla::PaintedLayerDataNode::PopAllPaintedLayerData()::$_0) src/layout/painting/FrameLayerBuilder.cpp:3618:20
#5 mozilla::PaintedLayerDataNode::PopAllPaintedLayerData() src/layout/painting/FrameLayerBuilder.cpp:3250:23
#6 mozilla::PaintedLayerDataNode::Finish(bool) src/layout/painting/FrameLayerBuilder.cpp:3215:3
#7 mozilla::PaintedLayerDataNode::FinishAllChildren(bool) src/layout/painting/FrameLayerBuilder.cpp:3204:19
#8 mozilla::PaintedLayerDataNode::Finish(bool) src/layout/painting/FrameLayerBuilder.cpp:3213:3
#9 mozilla::PaintedLayerDataTree::Finish() src/layout/painting/FrameLayerBuilder.cpp:3275:12
#10 mozilla::ContainerState::Finish(unsigned int*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, nsDisplayList*) src/layout/painting/FrameLayerBuilder.cpp:5971:25
#11 mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) src/layout/painting/FrameLayerBuilder.cpp:6350:9
#12 nsDisplayList::BuildLayers(nsDisplayListBuilder*, mozilla::layers::LayerManager*, unsigned int, bool) src/layout/painting/nsDisplayList.cpp:2525:9
#13 nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) src/layout/painting/nsDisplayList.cpp:2736:20
#14 nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) src/layout/base/nsLayoutUtils.cpp:3843:12
#15 mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) src/layout/base/PresShell.cpp:6322:5
#16 nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) src/view/nsViewManager.cpp:480:19
#17 nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) src/view/nsViewManager.cpp:412:33
#18 nsViewManager::ProcessPendingUpdates() src/view/nsViewManager.cpp:1102:5
#19 nsRefreshDriver::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2042:11
#20 mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:299:7
#21 mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:317:5
#22 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:671:16
#23 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NormalPriorityNotify() src/layout/base/nsRefreshDriver.cpp:596:9
#24 mozilla::detail::RunnableMethodImpl<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver*, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1219:13
#25 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1166:14
#26 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#27 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#28 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#29 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#30 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
#31 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:938:22
#32 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#33 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#34 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#35 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:764:34
#36 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#37 main src/browser/app/nsBrowserApp.cpp:287:18
#38 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#39 _start (firefox+0x423d04)
Flags: in-testsuite?
Assignee: nobody → mikokm
Status: NEW → ASSIGNED
Blocks: 1462672
It seems that this was not related to transforms flattening after all.

mozregression gives the range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b9c75d921ae0ee6336d727f0ba29117c4e3d3e0f&tochange=a86ac3dafdf5b4fc5e874dce2be0c1ab87a2ec72 for failing this assertion. The most likely change to cause this is bug 1382534.

Previous builds fail another assertion: ###!!! ASSERTION: Bounds computation mismatch: 'mContainerBounds.IsEqualInterior(mAccumulatedChildBounds).

Botond, would you mind taking a look at this?
Assignee: mikokm → nobody
Blocks: 1382534
No longer blocks: 1462672
Status: ASSIGNED → NEW
Flags: needinfo?(botond)
Sorry, Miko, I didn't get a chance to look at this before going on PTO. It's one of the first things I plan to look at when I'm back.
Assignee: nobody → botond
Priority: -- → P2
(In reply to Botond Ballo [:botond] [on PTO, back Aug 27] from comment #2)
> Sorry, Miko, I didn't get a chance to look at this before going on PTO. It's
> one of the first things I plan to look at when I'm back.

Matt, Botond indicates in his bugzilla handle that he is on PTO until Aug 27, that is one week before the merge to beta, I'd prefer avoiding uplifts to beta if we can plan differently. Is there somebody in the team that could take this bug while Botond is on PTO and land it during the 63 nightly cycle? Thanks
Flags: needinfo?(matt.woodrow)
This is an assertion failure from a fuzzer testcase, we're not aware of any real-world content being negatively affected by it.

Given that, I don't think we need to rush to get it uplifted, so I'm happy to wait for Botond to be back.

Still want it fixed though :)
Flags: needinfo?(matt.woodrow)
marking as fix-optional given that we are in Beta + low impact.
The recent spike in failures was caused by miss classification because of Bug 1495055. It should return to normal.
QA Contact: matt.woodrow
QA Contact: matt.woodrow
Hmm, I can't reproduce this in a recent m-c debug build. Perhaps it was fixed by something else along the way?
I tried to use mozregression to find what might have fixed this, like so:

  mozregression --find-fix -B debug --bad 20180723154916

(the argument for --bad being the build ID from comment 0). However, I could not trigger the assertion even on the supposed "bad" build.

Tyson, what platform were you testing this on? Can you still repro it with a recent nightly or m-c build? Can you repro it on the "bad" build with the above mozregression command?
Flags: needinfo?(botond) → needinfo?(twsmith)
(In reply to Botond Ballo [:botond] from comment #10)

> Tyson, what platform were you testing this on? Can you still repro it with a
> recent nightly or m-c build? Can you repro it on the "bad" build with the
> above mozregression command?

I can reproduce this locally on my Ubuntu 16.04 machine with a debug build:
Version=65.0a1
BuildID=20181025160949
SourceStamp=37d240a1d498bd1662e0f9d6053ee75ccdb90786
Flags: needinfo?(twsmith)
Huh. I wonder what is different between our setups.

Could you post the about:support of the build in which you're reproing this please?
Flags: needinfo?(twsmith)
Attached file about-support.txt
No sure where the problem could be. Can you reproduce it if you launch the build on it's own and then open the test case?
Flags: needinfo?(twsmith)
Ok, I could repro this by doing Ctrl+mousewheel to change the browser zoom a few times.
The mask item which triggers the assertion is 0x7f68c0bf9220 in the "after" display list.

It initially ("before optimization") has an empty clip, but a clip chain that includes a clip with a rounded rect, associated with the ASR for the root scroll frame.

However, it eventually ("after optimization") picks up that clip, via a call to FuseClipChainUpTo().

I don't really understand the expectations around FuseClipChainUpTo(). Markus, any insights are appreciated.
Actually... something else is going on here.

Whether we get the problematic display list or not, seems to depend _not_ on bug 1382534, but on whether the scrollable subframe on the page happens to be active. If it's active, we get the assertion failure even without bug 1382534.
(In reply to Botond Ballo [:botond] from comment #18)
> Actually... something else is going on here.
> 
> Whether we get the problematic display list or not, seems to depend _not_ on
> bug 1382534, but on whether the scrollable subframe on the page happens to
> be active. If it's active, we get the assertion failure even without bug
> 1382534.

I should clarify that I made this determination by backing out the "core" part of bug 1382534 (the change to the ASR of mask items) locally. Bug 1382534 also included other, "peripheral" changes. It's possible that the regression is actually caused by one of the peripheral changes.
Would be nice to bisect locally to find out which but 1-year-old code is difficult to coax into building. Currently getting rust errors like:

 0:35.57 error: use of deprecated item 'std::ascii::AsciiExt': use inherent methods instead
 0:35.57   --> /home/botond/dev/mozilla/refactoring/servo/components/style/gecko_string_cache/mod.rs:16:5
 0:35.57    |
 0:35.57 16 | use std::ascii::AsciiExt;
 0:35.57    |     ^^^^^^^^^^^^^^^^^^^^
 0:35.58    |
You can:

 * figure out which rust version we were using, from the taskcluster/ files and downgrade to that, or alternatively:
 * just remove the #[deny(warnings)] in servo/components/style/lib.rs, or alternatively:
 * build with RUSTFLAGS="--cap-lints allow".

Hope it helps :)
Thanks for the suggestions :)

I ended up looking up the Rust version here [1] and downgrading to it - so far, so good.

[1] https://wiki.mozilla.org/Rust_Update_Policy_for_Firefox
(In reply to Botond Ballo [:botond] from comment #23)
> The culprit is https://hg.mozilla.org/mozilla-central/rev/98355059c8d1.

This tells us that the issue is with layerizing the mask itself, rather than with the clip we compute for the mask.

That makes this more of a latent issue uncovered by bug 1382534, than a regression caused by it.

Here's an attempt at summarizing the issue, based on a discussion with Markus:
 
  * The assertion is telling us that FrameLayerBuilder doesn't implement
    intersecting a rounded-rect mask with an image mask.

  * WebRender does support this, so there's not much motivation to
    implement this functionality in FrameLayerBuilder.

  * Normally, performing such an intersection doesn't come up in the
    first place, becaue the rounded-rect clip ends up inside or
    outside the mask item, rather than on it. However, in this case
    it comes up because the mask is inside a filter which creates a
    BasicLayerManager.

  * A simple solution is therefore to avoid layerizing a mask if
    we're inside a BasicLayerManager.
(In reply to Botond Ballo [:botond] from comment #14)
> Ok, I could repro this by doing Ctrl+mousewheel to change the browser zoom a
> few times.

The reason this helped repro the issue is that the scroll frame for the <div id="a"> has a zero scroll range at some zoom levels. Adding some heights to the testcase to ensure the scroll frame always has a nonzero scroll range makes the assertion happen reliably on a page load, which allows me to turn the test case into a crashtest.
Depends on D10414
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b52fd8300b0
Do not layerize mask items if we are inside a BasicLayerManager already. r=mstange
https://hg.mozilla.org/integration/autoland/rev/3f06cfca9259
Add a crashtest. r=mstange
https://hg.mozilla.org/mozilla-central/rev/0b52fd8300b0
https://hg.mozilla.org/mozilla-central/rev/3f06cfca9259
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
I won't be uplifting this fix.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.