cc: Allow SendBeginMainFrame in idle state for synchronous compositor.

cc: Allow SendBeginMainFrame in idle state for synchronous compositor.

It's possible for main thread to be updated at half the vsync
frequency for input related cases when using the synchronous
compositor. Unlike raf each input event triggers it's own
SetNeedsCommit which gets posted as a task which runs after BeginFrame
and OnDraw. We don't send BeginMainFrame if the scheduler is in it's
idle state which is always true for the synchronous compositor because
we don't use a deadline.

BUG=506832
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/02bc6439d63f509492c9d2429318972c663236ea
Cr-Commit-Position: refs/heads/master@{#338614}

[brianderson, 2015-07-07 19:41:26]

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

[brianderson, 2015-07-07 19:41:26]

Approach looks good.

[sunnyps, 2015-07-07 19:43:56]

On 2015/07/07 19:41:26, brianderson wrote:
> Approach looks good.

I'm also investigating resetting the funnels every time we call
SetNeedsBeginFrames(false).

[mithro-old, 2015-07-08 03:26:33]

mithro@mithis.com changed reviewers:
+ mithro@mithis.com

[mithro-old, 2015-07-08 03:26:33]

I see no problem with the *idea* proposed here and think it would work well *in
theory*. However, I believe there are still some code paths in LTHI and friends
which might cause issues here. The main problem is that code is extremely
careless with its usage of frame time in the impl side verse the main thread
side (which I've been trying to fix for a while now).

I do think the scheduler code will need a way to differentiate between a
BeginMainFrame being sent during an idle period versus inside a BeginImplFrame.
That way you can do the following;

 def SendBeginMainFrame(inside_impl_frame):
  if inside_impl_frame:
    args_to_send = self.begin_frame_args_.Current()
  else:
    # When sending a BeginMainFrame inside an idle period, 
    # we use the last BeginFrameArgs we have received.
    args_to_send = self.begin_frame_args_.Last()
 
  ...

This makes it clear that we are *choosing* the right begin_frame_args here,
rather than doing it implicitly and provides a strong sanity check.

Does that make sense?

Tim 'mithro' Ansell

https://codereview.chromium.org/1218663007/diff/1/cc/scheduler/scheduler_stat...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/1218663007/diff/1/cc/scheduler/scheduler_stat...
cc/scheduler/scheduler_state_machine.cc:33: prepare_tiles_funnel_(1),
Why do these values change?

[sunnyps, 2015-07-13 23:56:11]

PTAL

No changes need to be made to the BeginFrameTracker - the frame args chosen are
always the last one.

https://codereview.chromium.org/1218663007/diff/1/cc/scheduler/scheduler_stat...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/1218663007/diff/1/cc/scheduler/scheduler_stat...
cc/scheduler/scheduler_state_machine.cc:33: prepare_tiles_funnel_(1),
On 2015/07/08 03:26:33, mithro wrote:
> Why do these values change?

Only the value for send_begin_main_frame_funnel_ needs to change. The basic idea
is this: for webview we want to allow SendBeginMainFrame in the
BEGIN_IMPL_FRAME_STATE_IDLE state in between frames. However we do not want to
allow SendBeginMainFrame before we've received the first BeginFrame in a "run"
of BeginFrames. To do that we set the funnel to true initially and also when we
end a run of frames and to false on each new frame (as before).

[brianderson, 2015-07-14 01:27:21]

lgtm

[sunnyps, 2015-07-14 01:32:08]

The CQ bit was checked by sunnyps@chromium.org

[commit-bot: I haz the power, 2015-07-14 01:33:06]

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

[commit-bot: I haz the power, 2015-07-14 01:37:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-14 01:39:11]

Patchset 3 (id:??) landed as
https://crrev.com/02bc6439d63f509492c9d2429318972c663236ea
Cr-Commit-Position: refs/heads/master@{#338614}