Bug 78038 – [Chromium] Assertion failure minX <= maxX in Region.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Julien Chaffraix]

The ASSERT is in Region.cpp - line 214:

ASSERT(minX <= maxX);

It seems to be hit frequently by our bots on test following test:

compositing/iframes/invisible-nested-iframe-show.html
compositing/iframes/layout-on-compositing-change.html

I don't know if it's a Chromium only bug.

[James Robinson]

Can we get a full stack or link to failing runs?

[Anders Carlsson]

It would also be helpful if we could figure out which Region operations lead up to this invalid region being created.

[Dana Jansens]

I managed to get this running invisible-nested-iframe-show.html enough times (7 in a row seems to be a magic number):

[1522:1522:530403859202:ERROR:process_util_posix.cc(142)] Received signal 11
	base::debug::StackTrace::StackTrace() [0x7fe70a800c06]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x7fe70a85e55d]
	0x7fe6ffd07af0
	WebCore::Region::Shape::bounds() [0x7fe70660409f]
	WebCore::Region::subtract() [0x7fe706604595]
	WebCore::TiledLayerChromium::updateBounds() [0x7fe70663b354]

When I print out the two rects going into Regions in updateBounds() I see the following:
old: 0 0 new: 800 600
old: 880 1879 new: 0 0
old: 0 0 new: 15 600
old: 0 0 new: 800 600
old: 0 0 new: 800 600
old: 0 0 new: 800 600
old: 0 0 new: 210 210
old: 0 0 new: 250 170
old: 0 0 new: 250 230
old: 0 0 new: 250 230
old: 0 0 new: 210 210
old: 0 0 new: 260 193
old: 0 0 new: -15 15

-15 isn't a good size width. contentBounds() is wrong here in this case.

[Dana Jansens]

Just in case it's of interest and others have difficulty reproing, here is the full backtrace.

Summary: Region seems okay, though asserting sooner on negative widths might be useful.

[3526:3526:531217440850:ERROR:process_util_posix.cc(142)] Received signal 11
	base::debug::StackTrace::StackTrace() [0x7f6fd8a6bc06]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x7f6fd8ac955d]
	0x7f6fcdf71af0
	WebCore::Region::Shape::bounds() [0x7f6fd486f30f]
	WebCore::Region::subtract() [0x7f6fd486f805]
	WebCore::TiledLayerChromium::updateBounds() [0x7f6fd48a662f]
	WebCore::TiledLayerChromium::invalidateRect() [0x7f6fd48a75c7]
	WebCore::TiledLayerChromium::setNeedsDisplayRect() [0x7f6fd48a753d]
	WebCore::LayerChromium::setNeedsDisplay() [0x7f6fd3dfb03c]
	WebCore::LayerChromium::setBounds() [0x7f6fd4887302]
	WebCore::GraphicsLayerChromium::updateLayerSize() [0x7f6fd4883d58]
	WebCore::GraphicsLayerChromium::setSize() [0x7f6fd4882e24]
	WebCore::positionScrollbarLayer() [0x7f6fd47df99a]
	WebCore::ScrollView::positionScrollbarLayers() [0x7f6fd47dfc00]
	WebCore::RenderLayerCompositor::updateOverflowControlsLayers() [0x7f6fd424d7ef]
	WebCore::RenderLayerCompositor::frameViewDidChangeSize() [0x7f6fd424ae0d]
	WebCore::RenderLayerCompositor::ensureRootLayer() [0x7f6fd424dd68]
	WebCore::RenderLayerCompositor::enableCompositingMode() [0x7f6fd4247bc5]
	WebCore::RenderLayerCompositor::updateBacking() [0x7f6fd42485e4]
	WebCore::RenderLayerCompositor::computeCompositingRequirements() [0x7f6fd424a416]
	WebCore::RenderLayerCompositor::computeCompositingRequirements() [0x7f6fd4249f63]
	WebCore::RenderLayerCompositor::computeCompositingRequirements() [0x7f6fd424a00a]
	WebCore::RenderLayerCompositor::updateCompositingLayers() [0x7f6fd42483a9]
	WebCore::FrameView::updateCompositingLayers() [0x7f6fd4e2753a]
	WebCore::FrameView::layout() [0x7f6fd4e28c5f]
	WebCore::FrameView::layoutTimerFired() [0x7f6fd4e2bb99]
	WebCore::Timer<>::fired() [0x7f6fd4e35728]
	WebCore::ThreadTimers::sharedTimerFiredInternal() [0x7f6fd47ede30]
	WebCore::ThreadTimers::sharedTimerFired() [0x7f6fd47edd67]
	webkit_glue::WebKitPlatformSupportImpl::DoTimeout() [0x7f6fd8eae30c]
	base::BaseTimer<>::TimerTask::Run() [0x7f6fd8eaeb7f]
	base::internal::RunnableAdapter<>::Run() [0x7f6fd8b07f61]
	base::internal::InvokeHelper<>::MakeItSo() [0x7f6fd8b07ee5]
	base::internal::Invoker<>::Run() [0x7f6fd8b07e91]
	base::Callback<>::Run() [0x7f6fd8a64267]
	MessageLoop::RunTask() [0x7f6fd8aa0710]
	MessageLoop::DeferOrRunPendingTask() [0x7f6fd8aa0827]
	MessageLoop::DoWork() [0x7f6fd8aa1049]
	base::MessagePumpGlib::RunWithDispatcher() [0x7f6fd8a47738]
	base::MessagePumpGlib::Run() [0x7f6fd8a47b18]
	MessageLoop::RunInternal() [0x7f6fd8aa03cb]
	MessageLoop::RunHandler() [0x7f6fd8aa027e]
	MessageLoop::Run() [0x7f6fd8a9fbb3]
	webkit_support::RunMessageLoop() [0x57299c]
	TestShell::waitTestFinished() [0x53826e]
	TestShell::runFileTest() [0x5308f9]
	runTest() [0x4fa214]
	main [0x4facd6]
	0x7f6fcdf5cc4d
	0x4e6569

[Julien Chaffraix]

FYI, the traces on the bot are the same as in comment #4.

[Dana Jansens]

When I run ntyimes in a debugger I end up seeing this assertion as well. In this case the size in RenderLayerBacking::m_compositedBounds is bad:

(gdb) print m_compositedBounds  
$7 = {m_location = {m_x = 0, m_y = 0}, m_size = {m_width = -180, m_height = 930}}

And this comes all the way in through the GraphicsLayer to cause this ASSERT.

[James Robinson]

We should clamp the rects somewhere or revert the change to pipe these rects into regions, this is causing an unacceptable number of ASSERT failures on the bots and with interactive debugging.

[Dana Jansens]

Would it be wrong to clamp them in GraphicsLayerChromium?
What do you think about the fact this hides the problem in WebCore for others to discover somehow?

[James Robinson]

(In reply to comment #8)
> Would it be wrong to clamp them in GraphicsLayerChromium?

I don't know.  If in doubt, we should do something conservative (i.e. doesn't break rendering).

> What do you think about the fact this hides the problem in WebCore for others to discover somehow?

I don't see how making tests crash and debugging impossible helps anyone.

[Dana Jansens]

Created attachment 126374 [details]
Patch

[James Robinson]

Comment on attachment 126374 [details]
Patch

I think checking for size.isEmpty() and setting size to (0, 0) would be slightly preferable - I'm not sure I want to "grow" a -50,50 layer to 0,50, or if that'll have any effect further down the pipe.

[Dana Jansens]

Created attachment 126377 [details]
Patch

[Dana Jansens]

Comment on attachment 126377 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=126377&action=review

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:179
> +    // avoid assertions in the compositor.

ill fix that redundant comment before landing

[James Robinson]

Comment on attachment 126377 [details]
Patch

OK thanks, I think this will work for now. Can you file a bug to investigate if we need to do anything more on the RenderLayerBacking or earlier phases? I have some theories but haven't investigated them on these particular test cases.

[Dana Jansens]

http://code.google.com/p/chromium/issues/detail?id=113534

[Fady Samuel]

Created attachment 126389 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 126389 [details]
Patch for landing

Clearing flags on attachment: 126389

Committed r107360: <http://trac.webkit.org/changeset/107360>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adrienne Walker]

Committed r110719: <http://trac.webkit.org/changeset/110719>