RESOLVED FIXED 73348
REGRESSION: Assertion in RenderBox::mapAbsoluteToLocalPoint when loading a page with a scrollable RenderLayer 
https://bugs.webkit.org/show_bug.cgi?id=73348
Summary REGRESSION: Assertion in RenderBox::mapAbsoluteToLocalPoint when loading a pa...
Beth Dakin
Reported 2011-11-29 12:34:07 PST
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()
Attachments
Test that asserts (1.16 KB, text/html)
2011-11-29 12:34 PST, Beth Dakin 		no flags
Details
Patch (3.48 KB, patch)
2011-12-07 18:00 PST, Beth Dakin 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Beth Dakin
Comment 1 2011-11-29 12:34:41 PST
Created attachment 117023 [details] Test that asserts
Beth Dakin
Comment 2 2011-11-29 12:39:36 PST
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
Comment 3 2011-11-29 13:23:51 PST
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
Comment 4 2011-12-01 12:55:52 PST
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
Comment 5 2011-12-01 14:11:23 PST
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
Comment 6 2011-12-01 14:42:52 PST
No wonder it does not crash for me in Mac and Chromium. I am running Snowleopard.
Adam Roben (:aroben)
Comment 7 2011-12-02 09:57:12 PST
Is this a dupe of bug 69187? I'm seeing this all the time.
Beth Dakin
Comment 8 2011-12-02 10:47:13 PST
(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)
Comment 9 2011-12-02 11:08:09 PST
<rdar://problem/10518918>
Simon Fraser (smfr)
Comment 10 2011-12-02 11:09:04 PST
*** Bug 69187 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 11 2011-12-02 11:10:28 PST
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
Comment 12 2011-12-02 17:35:02 PST
*** Bug 73723 has been marked as a duplicate of this bug. ***
Beth Dakin
Comment 13 2011-12-05 14:19:59 PST
(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
Comment 14 2011-12-05 14:23:07 PST
(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
Comment 15 2011-12-07 18:00:54 PST
Created attachment 118296 [details] Patch
Beth Dakin
Comment 16 2011-12-08 11:45:06 PST
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.
Note You need to log in before you can comment on or make changes to this bug.