cc: Be less aggressive about scheduling for scroll handlers

cc: Be less aggressive about scheduling for scroll handlers

A previous patch changed the scheduling policy so that scroll handlers
get more of a chance of completing in the same frame as the scroll
update ("best effort sync scrolling"). It turns out this went a little
too far by fully disabling prefer smoothness mode in the presense of
scroll handlers. For instance, this caused us to start synchronously
waiting for asynchronous texture upload completion, leading to skipped
frames.

This patch adjusts the logic so that scroll handlers are still given an
opportunity to complete within the same frame (i.e., the deadline isn't
triggered early), but the tree activation policy is to prefer existing
content over new content.

We also rename the smoothness mode in the scheduler to "impl latency
takes priority" to make its purpose clearer.

BUG=404267
Committed: https://crrev.com/733d7abae5f8665da809ac743d1f18f9230f7bf4
Cr-Commit-Position: refs/heads/master@{#293369}

[Sami, 2014-08-29 18:56:28]

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

[Sami, 2014-08-29 18:56:44]

Brian, do you think this deserves its own mode in the scheduler?

[brianderson, 2014-09-03 20:01:58]

lgtm once smoothness_takes_priority doesn't mean two different things.

https://codereview.chromium.org/522903002/diff/1/cc/trees/thread_proxy.cc
File cc/trees/thread_proxy.cc (right):

https://codereview.chromium.org/522903002/diff/1/cc/trees/thread_proxy.cc#new...
cc/trees/thread_proxy.cc:1362: impl().scheduler->SetSmoothnessTakesPriority(
Should we rename the concept in the scheduler since it isn't exactly the same
thing anymore?

Or maybe add scroll_affects_scroll_handler to the scheduler, along side
smoothness_takes_priority?

[brianderson, 2014-09-03 20:04:53]

On 2014/08/29 18:56:44, Sami wrote:
> Brian, do you think this deserves its own mode in the scheduler?

By mode, do you mean name? Or do you mean a platform-specific flag?

[chromium-reviews, 2014-09-03 20:07:02]

Right, I meant a different name that makes it clear that it is a different
thing. Suggestions welcome :)
On 3 Sep 2014 21:04, <brianderson@chromium.org> wrote:

> On 2014/08/29 18:56:44, Sami wrote:
>
>> Brian, do you think this deserves its own mode in the scheduler?
>>
>
> By mode, do you mean name? Or do you mean a platform-specific flag?
>
> https://codereview.chromium.org/522903002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[brianderson, 2014-09-03 20:36:24]

I double checked all the places the Scheduler uses smoothness_takes_priority and
we'll want to take into account scroll_affects_scroll_handler in every instance;
so it doesn't make sense to separate the concepts in the scheduler yet.

Let's rename it to impl_latency_takes_priority?

[Sami, 2014-09-04 17:42:04]

On 2014/09/03 20:36:24, brianderson wrote:
> I double checked all the places the Scheduler uses smoothness_takes_priority
and
> we'll want to take into account scroll_affects_scroll_handler in every
instance;
> so it doesn't make sense to separate the concepts in the scheduler yet.
> 
> Let's rename it to impl_latency_takes_priority?

That's a great name, thanks!

Could you do another pass and cq if everything's straight?

[brianderson, 2014-09-04 17:54:22]

lgtm!

[Sami, 2014-09-04 18:02:17]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-09-04 18:04:47]

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

[commit-bot: I haz the power, 2014-09-04 23:13:28]

Committed patchset #2 (id:20001) as 13831dba869fcbb2d8a76c4fb231b5507afca71e

[commit-bot: I haz the power, 2014-09-10 03:33:47]

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