Bug 148690 – http/tests/navigation/anchor-frames-same-origin.html is flaky

• Status: REOPENED
• Component: Layout and Rendering

[Chris Dumez]

http/tests/navigation/anchor-frames-same-origin.html passes when run after http/tests/navigation/anchor-frames-cross-origin.html (case a) but fails when run on its own (case b).

The test uses the following frame structure:
- main Frame
-- child Frame (100x100px)
--- grand-child frame (8000x8000px)
----- anchor is at (0, 2000) px

When debugging it, I found that when we call FrameView::maintainScrollPositionAtAnchor(), we are in the following state:
- case a: Layout is done (renderView->needsLayout() returns false) so we call FrameView::scrollToAnchor() right away.
- case b: We need a layout, so we call layout() and only call FrameView::scrollToAnchor() after layout is done in performPostLayoutTasks().

scrollToAnchor() calls RenderLayer::scrollRectToVisible(rect) with the visible rect of the anchor we need to scroll to. This is the code that will actually scroll the view if needed and well as its parents. The logic looks like this:
===
                frameView.setScrollPosition(IntPoint(xOffset, yOffset));
                if (frameView.safeToPropagateScrollToParent()) {
                    parentLayer = ownerElement->renderer()->enclosingLayer();
                    // FIXME: This doesn't correctly convert the rect to
                    // absolute coordinates in the parent.
                    newRect.setX(rect.x() - frameView.scrollX() + frameView.x());
                    newRect.setY(rect.y() - frameView.scrollY() + frameView.y());
                    fprintf(stderr, "safeToPropagateScrollToParent, newRect: %d, %d, frameView.scrollY(): %d\n", newRect.x().toInt(), newRect.y().toInt(), frameView.scrollY());
                }
===

In case a, when we compute the parent's new Rect, we end up with 2346px as height because frameView.scrollY() returns 0 (as expected given that the content height is smaller than the frame height).
However, in case b, the new Rect has a height of 0 because frameView.scrollY() returns 2008.

For some reason, in case b, FrameView::maximumScrollPosition() returns [77, 2142] instead of [0, 0] as in case a.

Render tree in case a when scrollToAnchor() is called:
(R)elative/A(B)solute/Fi(X)ed/Stick(Y) positioned, (O)verflow clipping, (A)nonymous, (G)enerated, (F)loating, has(L)ayer, (C)omposited, (D)irty layout, Dirty (S)tyle
---G-L- --* RenderView  (0.00, 0.00) (8000.00, 8000.00) renderer->(0x11629fbe0)
-----L- --    HTML RenderBlock  (0.00, 0.00) (8000.00, 2034.00) renderer->(0x1163bdc60) node->(0x1163e52a0)
------- --      BODY RenderBody  (8.00, 8.00) (7984.00, 2018.00) renderer->(0x1163bdd10) node->(0x116335a80)
------- --        DIV RenderBlock  (0.00, 0.00) (7984.00, 2000.00) renderer->(0x1163bddc0) node->(0x116335ae0)
--AG--- --        RenderBlock  (0.00, 2000.00) (7984.00, 18.00) renderer->(0x1163bde70)
------- --          RootInlineBox  (0.00, 0.00) (265.88, 18.00) (0x1163f1b40) renderer->(0x1163bde70)
------- --            InlineTextBox  (0.00, 0.00) (261.88, 18.00) (0x116331160) run(0, 39) "This is an anchor point named "anchor1""
------- --            InlineTextBox  (261.88, 0.00) (4.00, 18.00) (0x1163311b8) run(0, 1) "."
------- --          A RenderInline renderer->(0x116343600) node->(0x1163ed5a0)
------- --            #text RenderText renderer->(0x1163a1dc0) node->(0x11634bea0) length->(39) "This is an anchor point named "anchor1""
------- --          #text RenderText renderer->(0x1163a1f20) node->(0x11634bee8) length->(4) ".\n\n\n"

====

Render tree in case b when scrollToAnchor is called:
(R)elative/A(B)solute/Fi(X)ed/Stick(Y) positioned, (O)verflow clipping, (A)nonymous, (G)enerated, (F)loating, has(L)ayer, (C)omposited, (D)irty layout, Dirty (S)tyle
---G-L- --* RenderView  (0.00, 0.00) (0.00, 0.00) renderer->(0x1152ecbe0)
-----L- --    HTML RenderBlock  (0.00, 0.00) (0.00, 2142.00) renderer->(0x1153c6840) node->(0x1153ac4e0)
------- --      BODY RenderBody  (8.00, 8.00) (0.00, 2126.00) renderer->(0x1153c68f0) node->(0x1153ac5a0)
------- --        DIV RenderBlock  (0.00, 0.00) (0.00, 2000.00) renderer->(0x1153c69a0) node->(0x1153ac600)
--AG--- --        RenderBlock  (0.00, 2000.00) (0.00, 126.00) renderer->(0x1153c6a50)
------- --          RootInlineBox  (0.00, 0.00) (28.45, 18.00) (0x1153f1aa0) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 0.00) (28.45, 18.00) (0x1153460b0) run(0, 4) "This"
------- --          RootInlineBox  (0.00, 18.00) (10.67, 18.00) (0x1153f1b40) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 18.00) (10.67, 18.00) (0x115346108) run(5, 7) "is"
------- --          RootInlineBox  (0.00, 36.00) (15.10, 18.00) (0x1153f1be0) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 36.00) (15.10, 18.00) (0x115346160) run(8, 10) "an"
------- --          RootInlineBox  (0.00, 54.00) (43.53, 18.00) (0x1153f1c80) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 54.00) (43.53, 18.00) (0x1153461b8) run(11, 17) "anchor"
------- --          RootInlineBox  (0.00, 72.00) (32.89, 18.00) (0x1153f1d20) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 72.00) (32.89, 18.00) (0x115346210) run(18, 23) "point"
------- --          RootInlineBox  (0.00, 90.00) (42.65, 18.00) (0x1153f1dc0) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 90.00) (42.65, 18.00) (0x115346268) run(24, 29) "named"
------- --          RootInlineBox  (0.00, 108.00) (68.59, 18.00) (0x1153f1e60) renderer->(0x1153c6a50)
------- --            InlineTextBox  (0.00, 108.00) (64.59, 18.00) (0x1153462c0) run(30, 39) ""anchor1""
------- --            InlineTextBox  (64.59, 108.00) (4.00, 18.00) (0x115346318) run(0, 1) "."
------- --          A RenderInline renderer->(0x11534c060) node->(0x1153ede88)
------- --            #text RenderText renderer->(0x115380f20) node->(0x115355dc8) length->(39) "This is an anchor point named "anchor1""
------- --          #text RenderText renderer->(0x115380f78) node->(0x115355e10) length->(4) ".\n\n\n"

[Chris Dumez]

It looks like it fails when run on its own on WebKit2 as well.

[Chris Dumez]

Passes:
Tools/Scripts/run-webkit-tests --release http/tests/navigation/anchor-frames-cross-origin.html  http/tests/navigation/anchor-frames-same-origin.html

Fails:
Tools/Scripts/run-webkit-tests --release http/tests/navigation/anchor-frames-same-origin.html

[Chris Dumez]

Created attachment 260408 [details]
Temporary workaround

[Chris Dumez]

Comment on attachment 260408 [details]
Temporary workaround

Zalan, what do you think? Can we land this in the mean time?

[Chris Dumez]

Committed r189250: <http://trac.webkit.org/changeset/189250>

[Chris Dumez]

Reopening as it is a temporary workaround.

[Ahmad Saleem]

We still have this workaround - https://searchfox.org/wubkat/rev/df1e46cebe93c020392645e94f67469843cd8063/LayoutTests/http/tests/navigation/resources/grandchild-with-anchor.html#8