Bug 73348 - REGRESSION: Assertion in RenderBox::mapAbsoluteToLocalPoint when loading a page with a scrollable RenderLayer
Summary: REGRESSION: Assertion in RenderBox::mapAbsoluteToLocalPoint when loading a pa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.7
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar, Regression
: 69187 73723 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-29 12:34 PST by Beth Dakin
Modified: 2011-12-08 11:45 PST (History)
10 users (show)

See Also:


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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 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()
Comment 1 Beth Dakin 2011-11-29 12:34:41 PST
Created attachment 117023 [details]
Test that asserts
Comment 2 Beth Dakin 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.
Comment 3 Xiaomei Ji 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.
Comment 4 Xiaomei Ji 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.
Comment 5 Beth Dakin 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.
Comment 6 Xiaomei Ji 2011-12-01 14:42:52 PST
No wonder it does not crash for me in Mac and Chromium. I am running Snowleopard.
Comment 7 Adam Roben (:aroben) 2011-12-02 09:57:12 PST
Is this a dupe of bug 69187?

I'm seeing this all the time.
Comment 8 Beth Dakin 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.
Comment 9 Simon Fraser (smfr) 2011-12-02 11:08:09 PST
<rdar://problem/10518918>
Comment 10 Simon Fraser (smfr) 2011-12-02 11:09:04 PST
*** Bug 69187 has been marked as a duplicate of this bug. ***
Comment 11 Simon Fraser (smfr) 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.
Comment 12 Beth Dakin 2011-12-02 17:35:02 PST
*** Bug 73723 has been marked as a duplicate of this bug. ***
Comment 13 Beth Dakin 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.
Comment 14 Beth Dakin 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.
Comment 15 Beth Dakin 2011-12-07 18:00:54 PST
Created attachment 118296 [details]
Patch
Comment 16 Beth Dakin 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.