Force a commit at the end of the top controls animation

Schedule commit when top controls animation updates the scroll offset

Force a commit if the top controls animate the scroll offset. This prevents main
and impl scroll offsets from remaining out-of-sync, which can cause problems
with subsequent touch interaction.

BUG=402907, 364340
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289217

[jdduke (slow), 2014-08-12 18:53:54]

aelias@: Does this look reasonable? Or should we keep finer track of whether the
offset actually did change due to the animation, and only do the commit then?

[aelias_OOO_until_Jul13, 2014-08-12 19:01:09]

I think you can just put the SetNeedsCommit and RenewTreePriority calls on every
scroll increment in the if block above.  We don't need to wait until the
animation finishes; LTHI::ScrollBy also does it on every frame.

[aelias_OOO_until_Jul13, 2014-08-12 19:02:08]

Could you also add a unit test for this?

[jdduke (slow), 2014-08-12 20:23:18]

On 2014/08/12 19:02:08, aelias wrote:
> Could you also add a unit test for this?

Yup, done.

[aelias_OOO_until_Jul13, 2014-08-12 20:38:09]

https://codereview.chromium.org/461083002/diff/40001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/461083002/diff/40001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:3041: SetNeedsAnimate();
Now that I look at this, this SetNeedsAnimate should be before the early-return
(or the early-return should be replaced by a && condition in the if statement). 
The early return was added in https://chromiumcodereview.appspot.com/14715002
when SetNeedsAnimate didn't exist.  Indeed I see the animation sometimes snaps
when at the top of the page.  Could you also fix this while you're at it and add
a test?

[jdduke (slow), 2014-08-12 22:48:50]

https://codereview.chromium.org/461083002/diff/40001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/461083002/diff/40001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:3041: SetNeedsAnimate();
On 2014/08/12 20:38:09, aelias wrote:
> Now that I look at this, this SetNeedsAnimate should be before the
early-return
> (or the early-return should be replaced by a && condition in the if
statement). 
> The early return was added in https://chromiumcodereview.appspot.com/14715002
> when SetNeedsAnimate didn't exist.  Indeed I see the animation sometimes snaps
> when at the top of the page.  Could you also fix this while you're at it and
add
> a test?

Done.

[aelias_OOO_until_Jul13, 2014-08-12 22:51:22]

lgtm

[jdduke (slow), 2014-08-12 23:49:42]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-08-13 00:18:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/461083002/60001

[commit-bot: I haz the power, 2014-08-13 01:38:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-13 08:51:26]

Change committed as 289217