Don't rely on values computed as a side effect of paint.

Don't rely on values computed as a side effect of paint.

Contrary to the assertions on the bug, the current code is correct. As
the user changes the selection, there will naturally be a visual
affordance to indicate the selection changes. By the time we scroll,
RL::hasBlockSelectionGapsBounds will be current and correct.

That said, the hope is that in the future paint will have no side
effects at all, nor do we want layout tests to require repaints during
their execution. So even if this is technically correct it's undesirable
on a number of levels.

This cl replaces the use of hasBlockSelectionGapsBounds with non-paint
related code (in fact, that function could be removed entirely).

R=abarth@chromium.org
BUG=353827
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169752

[Ian Vollick, 2014-03-21 13:37:50]

Adam, PTAL. For simplicity, I'm computing the selection rect dynamically (once
per scroll update), but if this is undesirable, I'd be happy to look into
caching.

[abarth-chromium, 2014-03-21 15:45:54]

https://codereview.chromium.org/208203002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayerScrollableArea.cpp (right):

https://codereview.chromium.org/208203002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayerScrollableArea.cpp:399: &&
layer()->renderer()->selectionRectForRepaint(0, true).isEmpty();
Why not just call selectionRect() ?  That supplies these values for you.

[abarth-chromium, 2014-03-21 15:46:42]

Otherwise, LGTM

[Ian Vollick, 2014-03-21 16:58:43]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-03-21 16:58:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/208203002/20001

[commit-bot: I haz the power, 2014-03-21 17:00:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-21 17:00:29]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[Ian Vollick, 2014-03-21 17:01:50]

https://codereview.chromium.org/208203002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderLayerScrollableArea.cpp (right):

https://codereview.chromium.org/208203002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderLayerScrollableArea.cpp:399: &&
layer()->renderer()->selectionRectForRepaint(0, true).isEmpty();
On 2014/03/21 15:45:54, abarth wrote:
> Why not just call selectionRect() ?  That supplies these values for you.

Thanks, that's way better. Done.

[Ian Vollick, 2014-03-21 17:07:17]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-03-21 17:07:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/208203002/20001

[commit-bot: I haz the power, 2014-03-21 18:01:00]

Change committed as 169752

[Ian Vollick, 2014-03-23 16:57:31]

A revert of this CL has been created in
https://codereview.chromium.org/209543002/ by vollick@chromium.org.

The reason for reverting is: Looks like this is causing ASSERTs due to calling
layout-dependent code at the wrong time:
https://code.google.com/p/chromium/issues/detail?id=355277.