1496194 - Assertion failure: min <= max, at /builds/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:7599

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Jesse Schwartzentruber (:truber)]

Attachment: testcase.html

The attached testcase causes an assertion failure in m-c 20181003-dcba2a476ccf
with WebRender enabled. Linux, Intel UHD Graphics 620.

Assertion failure: min <= max, at /builds/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:7599
#0: DistanceToRange
        at layout/painting/nsDisplayList.cpp:7599
#1: nsDisplayStickyPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&, mozilla::layers::StackingContextHelper const&, mozilla::layers::WebRenderLayerManager*, nsDisplayListBuilder*)
        at layout/painting/nsDisplayList.cpp:7728
#2: mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList*, nsDisplayItem*, nsDisplayListBuilder*, mozilla::layers::StackingContextHelper const&, mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&)
        at gfx/layers/wr/WebRenderCommandBuilder.cpp:1447
#3: nsDisplayWrapList::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&, mozilla::layers::StackingContextHelper const&, mozilla::layers::WebRenderLayerManager*, nsDisplayListBuilder*)
        at layout/painting/nsDisplayList.cpp:6259
#4: nsDisplayStickyPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&, mozilla::layers::StackingContextHelper const&, mozilla::layers::WebRenderLayerManager*, nsDisplayListBuilder*)
        at layout/painting/nsDisplayList.cpp:7773
#5: mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList(nsDisplayList*, nsDisplayItem*, nsDisplayListBuilder*, mozilla::layers::StackingContextHelper const&, mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&)
        at gfx/layers/wr/WebRenderCommandBuilder.cpp:1447
#6: mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands(mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&, nsDisplayList*, nsDisplayListBuilder*, mozilla::layers::WebRenderScrollData&, mozilla::wr::TypedSize2D<float, mozilla::wr::LayoutPixel>&, nsTArray<mozilla::wr::WrFilterOp> const&)
        at gfx/layers/wr/WebRenderCommandBuilder.cpp:1311
#7: mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer(nsDisplayList*, nsDisplayListBuilder*, nsTArray<mozilla::wr::WrFilterOp> const&, mozilla::layers::WebRenderBackgroundData*)
        at gfx/layers/wr/WebRenderLayerManager.cpp:297
#8: nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int)
        at layout/painting/nsDisplayList.cpp:2725
#9: nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags)
        at layout/base/nsLayoutUtils.cpp:3834
#10: mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int)
        at layout/base/PresShell.cpp:6351
#11: nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*)
        at view/nsViewManager.cpp:480
#12: nsViewManager::ProcessPendingUpdatesForView(nsView*, bool)
        at view/nsViewManager.cpp:412
#13: nsViewManager::ProcessPendingUpdates()
        at view/nsViewManager.cpp:1102
#14: nsRefreshDriver::Tick(mozilla::TimeStamp)
        at layout/base/nsRefreshDriver.cpp:2046
#15: mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&)
        at layout/base/nsRefreshDriver.cpp:325
#16: mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp)
        at layout/base/nsRefreshDriver.cpp:318
#17: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp)
        at layout/base/nsRefreshDriver.cpp:756
#18: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp)
        at layout/base/nsRefreshDriver.cpp:572
#19: mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&)
        at layout/ipc/VsyncChild.cpp:78
#20: mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&)
        at ipc/ipdl/PVsyncChild.cpp:167
#21: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)
        at ipc/glue/MessageChannel.cpp:2248
#22: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)
        at ipc/glue/MessageChannel.cpp:2175
#23: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)
        at ipc/glue/MessageChannel.cpp:2012
#24: mozilla::ipc::MessageChannel::MessageTask::Run()
        at ipc/glue/MessageChannel.cpp:2045
#25: nsThread::ProcessNextEvent(bool, bool*)
        at xpcom/threads/nsThread.cpp:1231
#26: NS_ProcessNextEvent(nsIThread*, bool)
        at xpcom/threads/nsThreadUtils.cpp:530
#27: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)
        at ipc/glue/MessagePump.cpp:125
#28: MessageLoop::RunInternal()
        at ipc/chromium/src/base/message_loop.cc:325
#29: MessageLoop::Run()
        at ipc/chromium/src/base/message_loop.cc:318
#30: nsBaseAppShell::Run()
        at widget/nsBaseAppShell.cpp:158
#31: XRE_RunAppShell()
        at toolkit/xre/nsEmbedFunctions.cpp:939
#32: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)
        at ipc/glue/MessagePump.cpp:269
#33: MessageLoop::RunInternal()
        at ipc/chromium/src/base/message_loop.cc:325
#34: MessageLoop::Run()
        at ipc/chromium/src/base/message_loop.cc:318
#35: XRE_InitChildProcess(int, char**, XREChildData const*)
        at toolkit/xre/nsEmbedFunctions.cpp:765
#36: content_process_main(mozilla::Bootstrap*, int, char**)
        at ipc/contentproc/plugin-container.cpp:50
#37: main
        at browser/app/nsBrowserApp.cpp:287
#39: MOZ_ReportAssertionFailure
        at mfbt/Assertions.h:164

Comment 1

[Jeff Muizelaar [:jrmuizel]]

@kats is probably a decent person to look at this, if he has time.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

This intersection didn't quite seem to produce the expected result:

https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/layout/generic/StickyScrollContainer.cpp#322

outer                   : {left = -71940,  top = -536870911, right = 81780,  bottom = 536870912}
inner (before intersect): {left =  89700,  top = -536870911, right = 162120, bottom = 536870912}
inner (after intersect) : {left =  89700,  top = -536870911, right = 89700,  bottom = -536870911}

I would have expected inner.left/right would be set to outer.right.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Andrew Osmond [:aosmond] from comment #2)
> This intersection didn't quite seem to produce the expected result:
> 
> https://searchfox.org/mozilla-central/rev/
> adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/layout/generic/
> StickyScrollContainer.cpp#322
> 
> outer                   : {left = -71940,  top = -536870911, right = 81780, 
> bottom = 536870912}
> inner (before intersect): {left =  89700,  top = -536870911, right = 162120,
> bottom = 536870912}
> inner (after intersect) : {left =  89700,  top = -536870911, right = 89700, 
> bottom = -536870911}
> 
> I would have expected inner.left/right would be set to outer.right.

That is to say, intersection is behaving as documented, but for how the rect is actually used in nsDisplayStickPosition, the desired result in this empty rect case would probably be:

{left =  81780,  top = -536870911, right = 81780, bottom = 536870912}

(Or we need to special case the empty rect.)

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Your analysis seems correct (just based on my understanding of how the rects are used, I didn't actually try the testcase).

I can look at this after I'm done with bug 1495962 unless somebody else wants it. I'll assign it to myself for now but feel free to steal if you want to work on it.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

This took me on a bit of an adventure in RectAbsolute.h but I have something that works locally. Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=040644fc2f059bf6afcfe168e889824b0f2a11e6

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1496194 - Fix the MoveBy(Point) implementation in RectAbsolute. r?aosmond

RectAbsolute contained a MoveBy(Point) function where Point was not a
template parameter but the actual gfx::Point type. This seems to be an
unintentional copy/paste error. This patch fixes it to be properly
templated.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1496194 - Add a RectAbsolute implementation for MoveInsideAndClamp. r?aosmond

Depends on D15135

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1496194 - Ensure the inner sticky rect stays inside the outer sticky rect. r?aosmond

Depends on D15136

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3bad720a367
Fix the MoveBy(Point) implementation in RectAbsolute. r=botond
https://hg.mozilla.org/integration/autoland/rev/08f51212951e
Add a RectAbsolute implementation for MoveInsideAndClamp. r=botond
https://hg.mozilla.org/integration/autoland/rev/f27f6731ee74
Ensure the inner sticky rect stays inside the outer sticky rect. r=botond

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a3bad720a367
https://hg.mozilla.org/mozilla-central/rev/08f51212951e
https://hg.mozilla.org/mozilla-central/rev/f27f6731ee74

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Not important enough to warrant uplifting since it's a debug assertion failure on a pathological test case