cc: Don't swap throttle BeginMainFrame within deadline

cc: Don't swap throttle BeginMainFrame within deadline

This will better reflect behavior prior to the
OutputSurface+Scheduler refactor and will
prevent us from skipping BeginMainFrames that
the scheduler only meant to defer.

BUG=381346
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276130

[brianderson, 2014-06-06 00:41:38]

PTAL. The test fails without this patch.

[simonhong, 2014-06-06 11:12:00]

Just a question.
How about triggering the BeginMainFrame in the deadline regardless of
HasRequestedSwapThisFrame() when needs_commit is true?
If we can trigger BeginMainFrame earlier,  I think this can also improve
throughput.

https://codereview.chromium.org/311353003/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine_unittest.cc (right):

https://codereview.chromium.org/311353003/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine_unittest.cc:1760: // Finish the previous
commit.and initiate another one.
nit:  there is a point "commit.and"

[brianderson, 2014-06-06 16:47:02]

On 2014/06/06 11:12:00, simonhong wrote:
> Just a question.
> How about triggering the BeginMainFrame in the deadline regardless of
> HasRequestedSwapThisFrame() when needs_commit is true?

Are you suggesting a different way to get to the same goal? The swap throttling
method behaves like we did before the OutputSurface+Scheduler refactor.

> If we can trigger BeginMainFrame earlier,  I think this can also improve
> throughput.

Yes! I wanted to get rid of the swap throttling of BeginMainFrames all together,
but there is some test flakiness I need to figure out.

> 
>
https://codereview.chromium.org/311353003/diff/1/cc/scheduler/scheduler_state...
> File cc/scheduler/scheduler_state_machine_unittest.cc (right):
> 
>
https://codereview.chromium.org/311353003/diff/1/cc/scheduler/scheduler_state...
> cc/scheduler/scheduler_state_machine_unittest.cc:1760: // Finish the previous
> commit.and initiate another one.
> nit:  there is a point "commit.and"

Will fix.

[Sami, 2014-06-06 18:10:35]

Patch itself lgtm, but let's figure out why miletus@ saw jank on the main
thread[1] before landing.

On 2014/06/06 16:47:02, brianderson wrote:
> On 2014/06/06 11:12:00, simonhong wrote:
> > Just a question.
> > How about triggering the BeginMainFrame in the deadline regardless of
> > HasRequestedSwapThisFrame() when needs_commit is true?
> 
> Are you suggesting a different way to get to the same goal? The swap
throttling
> method behaves like we did before the OutputSurface+Scheduler refactor.

Going back to the old behavior for now sounds like a good idea. Maybe later we
can figure out a more uniform way to throttle using swaps since now it is a
little ad-hoc. I'm also thinking whether we should reconsider some of the
shortcuts we take in smoothness mode since they add quite a bit of complexity.
 
> > If we can trigger BeginMainFrame earlier,  I think this can also improve
> > throughput.
> 
> Yes! I wanted to get rid of the swap throttling of BeginMainFrames all
together,
> but there is some test flakiness I need to figure out.

Definitely worth trying, although the risk is that the BeginMainFrame might
starve the browser compositor of CPU of we start running both more concurrently.

[1] http://code.google.com/p/chromium/issues/detail?id=381346

[brianderson, 2014-06-09 18:58:13]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2014-06-09 18:59:58]

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

[miletus1, 2014-06-09 19:02:36]

On 2014/06/09 18:59:58, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> 
https://chromium-status.appspot.com/cq/brianderson@chromium.org/311353003/20001

I thought we want to first resolve the issues posted at
https://code.google.com/p/chromium/issues/detail?id=381346   first ?

[brianderson, 2014-06-09 19:29:35]

On 2014/06/09 19:02:36, miletus1 wrote:
> On 2014/06/09 18:59:58, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> > 
>
https://chromium-status.appspot.com/cq/brianderson@chromium.org/311353003/20001
> 
> I thought we want to first resolve the issues posted at
> https://code.google.com/p/chromium/issues/detail?id=381346   first ?

This and https://codereview.chromium.org/311263016/ are needed to get us back to
old behavior, which I really want to get to even if it doesn't fix all of issue
381346.

https://codereview.chromium.org/311263016/ will make us less aggressive. This
patch will make us more aggressive. So I would like to land both patches at the
same time so we don't have a huge regression in some cases from only landing the
other patch.

[commit-bot: I haz the power, 2014-06-10 08:56:26]

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

[commit-bot: I haz the power, 2014-06-10 11:50:58]

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

[commit-bot: I haz the power, 2014-06-10 11:50:59]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[brianderson, 2014-06-10 15:52:41]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2014-06-10 15:54:53]

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

[commit-bot: I haz the power, 2014-06-10 17:12:24]

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

[commit-bot: I haz the power, 2014-06-10 20:47:37]

Change committed as 276130

[brianderson, 2014-06-11 21:38:41]

New issue here: https://codereview.chromium.org/325303002