Bug 73348 – REGRESSION: Assertion in RenderBox::mapAbsoluteToLocalPoint when loading a page with a scrollable RenderLayer

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

This assertion started filing after http://trac.webkit.org/changeset/100819 which was for https://bugs.webkit.org/show_bug.cgi?id=70395

To reproduce: Using a Debug build, load a web page with a scrollable RenderLayer such as the attached test case. As far as I can tell, there is not a problem in release builds, and for some reason, this assertion does not fire when running the layout tests (though it does fire with many layout tests when they are loaded manually in the browser).

ASSERTION FAILED: !view() || !view()->layoutStateEnabled()
/Volumes/Big/Source/Labyrinth/OpenSource/Source/WebCore/rendering/RenderBox.cpp(1386) : virtual void WebCore::RenderBox::mapAbsoluteToLocalPoint(bool, bool, WebCore::TransformState &) const
1   0x10933cfc4 WebCore::RenderBox::mapAbsoluteToLocalPoint(bool, bool, WebCore::TransformState&) const
2   0x10940d005 WebCore::RenderObject::absoluteToLocal(WebCore::FloatPoint const&, bool, bool) const
3   0x10890e4b3 WebCore::FrameView::convertToRenderer(WebCore::RenderObject const*, WebCore::IntPoint const&) const
4   0x1093b887e WebCore::RenderLayer::convertFromContainingViewToScrollbar(WebCore::Scrollbar const*, WebCore::IntPoint const&) const
5   0x10958e548 WebCore::Scrollbar::convertFromContainingView(WebCore::IntPoint const&) const
6   0x109585118 -[WebScrollbarPainterControllerDelegate scrollerImpPair:convertContentPoint:toScrollerImp:]
7   0x7fff8e053b78 -[NSScrollerImpPair _updateOverlayScrollersStateWithReason:forceAtLeastKnobsVisible:]
8   0x109588fa1 WebCore::ScrollAnimatorMac::notifyPositionChanged()
9   0x109588a8a WebCore::ScrollAnimatorMac::immediateScrollToPoint(WebCore::FloatPoint const&)
10  0x109588a23 WebCore::ScrollAnimatorMac::scrollToOffsetWithoutAnimation(WebCore::FloatPoint const&)
11  0x10958280c WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(WebCore::FloatPoint const&)
12  0x1093b52c3 WebCore::RenderLayer::scrollToOffset(int, int, WebCore::RenderLayer::ScrollOffsetClamping)
13  0x1093ba639 WebCore::RenderLayer::updateScrollInfoAfterLayout()
14  0x1092d4321 WebCore::RenderBlock::updateScrollInfoAfterLayout()
15  0x1092d50d0 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
16  0x1092d4377 WebCore::RenderBlock::layout()
17  0x1092dd4ff WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
18  0x1092d72d3 WebCore::RenderBlock::layoutBlockChildren(bool, int&)
19  0x1092d4cb7 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
20  0x1092d4377 WebCore::RenderBlock::layout()
21  0x1092dd4ff WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
22  0x1092d72d3 WebCore::RenderBlock::layoutBlockChildren(bool, int&)
23  0x1092d4cb7 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
24  0x1092d4377 WebCore::RenderBlock::layout()
25  0x1092dd4ff WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
26  0x1092d72d3 WebCore::RenderBlock::layoutBlockChildren(bool, int&)
27  0x1092d4cb7 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
28  0x1092d4377 WebCore::RenderBlock::layout()
29  0x1094f2715 WebCore::RenderView::layout()
30  0x108906095 WebCore::FrameView::layout(bool)
31  0x10865a27b WebCore::Document::implicitClose()

[Beth Dakin]

Created attachment 117023 [details]
Test that asserts

[Beth Dakin]

I looked at this briefly in the debugger, and immediateScrollToPoint() is called with the point (0,0). It doesn't make sense to call notifyPositionChanged() in that case because, since the page it just loading and laying out for the first time, the position has not changed.

[Xiaomei Ji]

The reason to trigger notifyChange is explained at:
https://bugs.webkit.org/show_bug.cgi?id=70395#c2

We probably need a better way to distinguish the situations.
Sam is probably the person knowing the code the best.

[Xiaomei Ji]

Is the assertion triggered by just loading the attached test case in a debug build?
I tried it, but it does not tigger assertion, neither when I scroll/resize the page.

[Beth Dakin]

The assertion will only happen on Mac OS X Lion with overlay scrollbars since it involves the new overlay scrollbar code. Are you running Lion and using overlay scrollbars? In that configuration, for me, the assertion fires on load.

[Xiaomei Ji]

No wonder it does not crash for me in Mac and Chromium. I am running Snowleopard.

[Adam Roben (:aroben)]

Is this a dupe of bug 69187?

I'm seeing this all the time.

[Beth Dakin]

(In reply to comment #7)
> Is this a dupe of bug 69187?
> 
> I'm seeing this all the time.

It certainly seems so! I guess this was a crash we always had, but http://trac.webkit.org/changeset/100819 definitely seems to make it happen much more frequently.

[Simon Fraser (smfr)]

<rdar://problem/10518918>

[Simon Fraser (smfr)]

*** Bug 69187 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

It's wrong to be calling Scrollbar::convertFromContainingView() in the middle of layout, since coordinates are in flux. Maybe we should postpone calls to RenderBlock::updateScrollInfoAfterLayout() until after layout is complete.

[Beth Dakin]

*** Bug 73723 has been marked as a duplicate of this bug. ***

[Beth Dakin]

(In reply to comment #11)
> It's wrong to be calling Scrollbar::convertFromContainingView() in the middle of layout, since coordinates are in flux. Maybe we should postpone calls to RenderBlock::updateScrollInfoAfterLayout() until after layout is complete.

If you look at that stack trace, you can see that this work is happening as a result of code in updateScrollInfoAfterLayout(). the layout state doesn't seem to be popped until after that.

[Beth Dakin]

(In reply to comment #13)
> (In reply to comment #11)
> > It's wrong to be calling Scrollbar::convertFromContainingView() in the middle of layout, since coordinates are in flux. Maybe we should postpone calls to RenderBlock::updateScrollInfoAfterLayout() until after layout is complete.
> 
> If you look at that stack trace, you can see that this work is happening as a result of code in updateScrollInfoAfterLayout(). the layout state doesn't seem to be popped until after that.

Oh, I just misunderstood you. I see you are saying that updateScrollInfoAfterLayout() itself should maybe be postponed.

[Beth Dakin]

Created attachment 118296 [details]
Patch

[Beth Dakin]

Fixed with http://trac.webkit.org/changeset/102355

Filed https://bugs.webkit.org/show_bug.cgi?id=74111 to cover the not-regularly-reproducible occurrence of this assertion.