Defer scroll status determination until scrolling has occurred

Defer scroll status determination until scrolling has occurred

As of M38, the first touchmove in a sequence is no longer special; it
can be preventDefault'ed while allowing future scrolling. One side
effect of this is that GestureScrollBegin may be forwarded to the
compositor even if the page consumes the remainder of the touch
sequence. This in turn caused the scheduler to enter smoothness priority
mode, interfering with optimal touch event dispatch.

As a workaround, tweak the semantics of |LTHI::IsCurrentlyScrolling()|
to indicate both that there is an actively scrolling layer and that
scrolling has occurred, renaming to |IsActivelyScrolling()|. Also stop
throttling touch event acks on the renderer main thread as there is no
current evidence that this feature improves touch interaction, and it
may unintentionally bump the scheduler out of smoothness mode.

BUG=426621
Committed: https://crrev.com/16dde6e3648b8d67675846bd74c8a99b732269fd
Cr-Commit-Position: refs/heads/master@{#301001}

[jdduke (slow), 2014-10-23 19:12:04]

jdduke@chromium.org changed reviewers:
+ brianderson@chromium.org

[jdduke (slow), 2014-10-23 19:12:04]

Thoughts?

[brianderson, 2014-10-23 20:55:51]

https://codereview.chromium.org/676853002/diff/1/cc/trees/layer_tree_host_imp...
File cc/trees/layer_tree_host_impl.cc (left):

https://codereview.chromium.org/676853002/diff/1/cc/trees/layer_tree_host_imp...
cc/trees/layer_tree_host_impl.cc:2411: client_->RenewTreePriority();
Why is this removed?

Is it because we'll only have a scrolling layer at this point, but not have
actually scrolled?

https://codereview.chromium.org/676853002/diff/1/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/676853002/diff/1/content/renderer/render_widg...
content/renderer/render_widget.cc:1082: // Unconsumed touchmove acks should
never be throttled as they're required to
Update comment? Removing throttling of TouchMove sgtm, especially since we don't
expect really high frequency touch inputs like we can get for mouse moves.

[jdduke (slow), 2014-10-23 22:12:49]

jdduke@chromium.org changed reviewers:
+ boliu@chromium.org

[jdduke (slow), 2014-10-23 22:12:50]

boliu@: Sorry for the noise, but would you mind ensuring the latest version
works (with the renewed priority on ScrollBegin)? Thanks.

https://codereview.chromium.org/676853002/diff/1/cc/trees/layer_tree_host_imp...
File cc/trees/layer_tree_host_impl.cc (left):

https://codereview.chromium.org/676853002/diff/1/cc/trees/layer_tree_host_imp...
cc/trees/layer_tree_host_impl.cc:2411: client_->RenewTreePriority();
On 2014/10/23 20:55:51, brianderson wrote:
> Why is this removed?
> 
> Is it because we'll only have a scrolling layer at this point, but not have
> actually scrolled?

Right, unless there's another side effect of this method that might cause a
different renewal priority? I guess it couldn't hurt to keep it? I'll add it
back.

https://codereview.chromium.org/676853002/diff/1/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/676853002/diff/1/content/renderer/render_widg...
content/renderer/render_widget.cc:1082: // Unconsumed touchmove acks should
never be throttled as they're required to
On 2014/10/23 20:55:51, brianderson wrote:
> Update comment? Removing throttling of TouchMove sgtm, especially since we
don't
> expect really high frequency touch inputs like we can get for mouse moves.

Done.

[boliu, 2014-10-23 22:27:15]

PS2 fix still works

[jdduke (slow), 2014-10-23 22:36:46]

jdduke@chromium.org changed reviewers:
+ jamesr@chromium.org

[jdduke (slow), 2014-10-23 22:36:47]

jamesr@: Owner review for content/renderer? Thanks.

[jamesr, 2014-10-23 22:37:38]

lgtm

[brianderson, 2014-10-23 22:39:42]

https://codereview.chromium.org/676853002/diff/20001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/676853002/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl_unittest.cc:522:
EXPECT_FALSE(host_impl_->IsActivelyScrolling());
Is this the expectation that would fail without your patch? If so, lgtm.

[jdduke (slow), 2014-10-23 22:45:14]

https://codereview.chromium.org/676853002/diff/20001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/676853002/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl_unittest.cc:522:
EXPECT_FALSE(host_impl_->IsActivelyScrolling());
On 2014/10/23 22:39:42, brianderson wrote:
> Is this the expectation that would fail without your patch? If so, lgtm.

Yup, exactly.

[jdduke (slow), 2014-10-23 22:48:26]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-10-23 23:01:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/676853002/20001

[commit-bot: I haz the power, 2014-10-24 00:23:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-24 00:23:40]

Patchset 2 (id:??) landed as
https://crrev.com/16dde6e3648b8d67675846bd74c8a99b732269fd
Cr-Commit-Position: refs/heads/master@{#301001}