scheduler: Don't block expensive loading tasks during main thread gestures

scheduler: Don't block expensive loading tasks during main thread gestures

Previously we were using the presence of main thread input gestures
(e.g., a mouse drag) as a hint that expensive loading and timer tasks
are non-essential. This patch relaxes the policy to allow loading tasks
in this use case to avoid starving loading work for the entire duration
of the gesture, e.g., for loading new content in an infinite scrolling
scenario.

BUG=548606
Committed: https://crrev.com/70eceecb80f82f7deea985549b52b97149ef9005
Cr-Commit-Position: refs/heads/master@{#357122}

[Sami, 2015-10-30 14:46:36]

Description was changed from

==========
scheduler: Don't block expensive loading tasks during main thread gestures

Previously we were using the presence of main thread input gestures
(e.g., a mouse drag) as a hint that expensive loading and timer tasks
are non-essential. This patch relaxes the policy to allow loading tasks
in this use case to avoid starving loading work for the entire duration
of the gesture, e.g., for infinite scroll implementations.

BUG=548606
==========

to

==========
scheduler: Don't block expensive loading tasks during main thread gestures

Previously we were using the presence of main thread input gestures
(e.g., a mouse drag) as a hint that expensive loading and timer tasks
are non-essential. This patch relaxes the policy to allow loading tasks
in this use case to avoid starving loading work for the entire duration
of the gesture, e.g., for loading new content in an infinite scrolling
scenario.

BUG=548606
==========

[Sami, 2015-10-30 14:46:36]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org

[alex clarke (OOO till 29th), 2015-10-30 14:55:23]

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
File components/scheduler/renderer/renderer_scheduler_impl_unittest.cc (right):

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
components/scheduler/renderer/renderer_scheduler_impl_unittest.cc:2155:
TestCompositorPolicy_ExpensiveLoadingTasksNotBlockedDuringMainThreadGestures) {
Please shorten this.

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
components/scheduler/renderer/renderer_scheduler_impl_unittest.cc:2169:
EXPECT_THAT(run_order,
Please add:

 EXPECT_TRUE(LoadingTasksSeemExpensive());
 EXPECT_FALSE(TimerTasksSeemExpensive());

[Sami, 2015-10-30 15:01:20]

Thanks, both addressed.

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
File components/scheduler/renderer/renderer_scheduler_impl_unittest.cc (right):

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
components/scheduler/renderer/renderer_scheduler_impl_unittest.cc:2155:
TestCompositorPolicy_ExpensiveLoadingTasksNotBlockedDuringMainThreadGestures) {
On 2015/10/30 14:55:23, alexclarke1 wrote:
> Please shorten this.

Done.

https://codereview.chromium.org/1420773003/diff/1/components/scheduler/render...
components/scheduler/renderer/renderer_scheduler_impl_unittest.cc:2169:
EXPECT_THAT(run_order,
On 2015/10/30 14:55:23, alexclarke1 wrote:
> Please add:
> 
>  EXPECT_TRUE(LoadingTasksSeemExpensive());
>  EXPECT_FALSE(TimerTasksSeemExpensive());

Done.

[alex clarke (OOO till 29th), 2015-10-30 15:10:39]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2015-10-30 15:10:39]

lgtm

[commit-bot: I haz the power, 2015-10-30 15:11:15]

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

[commit-bot: I haz the power, 2015-10-30 16:44:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-30 16:45:17]

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

[alex clarke (OOO till 29th), 2015-10-31 15:49:25]

This patch seems to have caused a big regression on 

ChromiumPerf/android-nexus7v2/smoothness.scrolling_tough_ad_cases /
first_gesture_scroll_update_latency

https://chromeperf.appspot.com/report?sid=c0a062750ab8431568733d9b766d025ca79...

This is unexpected, should we revert?

[picksi, 2015-11-02 12:34:49]

Are you certain that this CL was the cause? Even if we don't revert it would be
good to understand why this has made the tough_ad_cases worse, when presumably,
we would expect them to get better?

[Sami, 2015-11-02 12:50:22]

On 2015/11/02 12:34:49, picksi wrote:
> Are you certain that this CL was the cause? Even if we don't revert it would
be
> good to understand why this has made the tough_ad_cases worse, when
presumably,
> we would expect them to get better?

Yes, the issue was that we were missing handling for touch prediction in the
main thread use case. We've got a patch that fixes that and adds another use
case for synchronized scrolling to make it a little more obvious.