cc: Unify use of DidSwapBuffers() and did_request_swap

cc: Unify use of DidSwapBuffers() and did_request_swap

Use OutputSurface::DidSwapBuffers() as the source of whether
we swapped or not.

R=brianderson@chromium.org
BUG=363414
TEST=cc_unittests

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

[simonhong, 2014-04-23 13:02:57]

PTAL!

[brianderson, 2014-04-24 01:54:15]

I like the idea of using the callback as the one true source of whether we
swapped or not.

Can you also replace the instances of did_request_swap in the
SchedulerStateMachine so that it relies on DidSwapBuffers to update
last_frame_number_swap_performed_ instead?

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
File cc/scheduler/draw_swap_readback_result.h (left):

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
cc/scheduler/draw_swap_readback_result.h:21: DrawSwapReadbackResult()
Let's get rid of DrawSwapReadbackResult and just replace it with DrawResult.

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc
File cc/scheduler/scheduler.cc (right):

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc#ne...
cc/scheduler/scheduler.cc:103:
DCHECK(!IsInsideAction(SchedulerStateMachine::ACTION_DRAW_AND_READBACK));
Can you move this DCHECK into the state machine? It should be easier to check
there.

[simonhong, 2014-04-25 01:13:37]

Moving last_frame_number_swap_performed_ setting time to DidSwapBuffers() cause
many tests failure.
I need to check the reason.

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
File cc/scheduler/draw_swap_readback_result.h (left):

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
cc/scheduler/draw_swap_readback_result.h:21: DrawSwapReadbackResult()
On 2014/04/24 01:54:15, brianderson wrote:
> Let's get rid of DrawSwapReadbackResult and just replace it with DrawResult.

Done.

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc
File cc/scheduler/scheduler.cc (right):

https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc#ne...
cc/scheduler/scheduler.cc:103:
DCHECK(!IsInsideAction(SchedulerStateMachine::ACTION_DRAW_AND_READBACK));
On 2014/04/24 01:54:15, brianderson wrote:
> Can you move this DCHECK into the state machine? It should be easier to check
> there.

I tried but i think checking this condition in state machine is not easier than
here.
When state_machine::DidSwapBuffers() is called, all states were already
transited to next state.
Using IsInsideAction() in state machine can do but its not much difference.

[simonhong, 2014-04-28 07:13:51]

On 2014/04/25 01:13:37, simonhong wrote:
> Moving last_frame_number_swap_performed_ setting time to DidSwapBuffers()
cause
> many tests failure.
> I need to check the reason.
> 
>
https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
> File cc/scheduler/draw_swap_readback_result.h (left):
> 
>
https://codereview.chromium.org/246753008/diff/1/cc/scheduler/draw_swap_readb...
> cc/scheduler/draw_swap_readback_result.h:21: DrawSwapReadbackResult()
> On 2014/04/24 01:54:15, brianderson wrote:
> > Let's get rid of DrawSwapReadbackResult and just replace it with DrawResult.
> 
> Done.
> 
> https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc
> File cc/scheduler/scheduler.cc (right):
> 
>
https://codereview.chromium.org/246753008/diff/1/cc/scheduler/scheduler.cc#ne...
> cc/scheduler/scheduler.cc:103:
> DCHECK(!IsInsideAction(SchedulerStateMachine::ACTION_DRAW_AND_READBACK));
> On 2014/04/24 01:54:15, brianderson wrote:
> > Can you move this DCHECK into the state machine? It should be easier to
check
> > there.
> 
> I tried but i think checking this condition in state machine is not easier
than
> here.
> When state_machine::DidSwapBuffers() is called, all states were already
> transited to next state.
> Using IsInsideAction() in state machine can do but its not much difference.

I introduced SchedulerStateMachine::HasSentDrawRequestThisFrame().
This prevents infinite draw request in ShouldDraw().
Please take another look.

[Sami, 2014-05-01 10:21:43]

Thanks Simon, I think this is a good cleanup.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/draw_swap_r...
File cc/scheduler/draw_swap_readback_result.h (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/draw_swap_r...
cc/scheduler/draw_swap_readback_result.h:5: #ifndef
CC_SCHEDULER_DRAW_SWAP_READBACK_RESULT_H_
Mind renaming this file to draw_result.h too?

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.cc
File cc/scheduler/scheduler.cc (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.c...
cc/scheduler/scheduler.cc:190: // Swap should not occur by readback operation.
s/by/inside/?

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.c...
cc/scheduler/scheduler.cc:605: void Scheduler::DrawAndSwapForced() {
Looks like this and the next function could be removed in favor of just calling
the client directly.

[brianderson, 2014-05-02 04:08:58]

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:768:
last_frame_number_draw_request_sent_ = current_frame_number_;
Hmmm. This would mean that a readback, which didn't swap, could prevent the
replacement commit from drawing until the next BeginFrame.

Maybe we do need to keep separate concepts in the scheduler of swap_requested
vs. swap_performed, at least for this method. Sorry I didn't realize this when I
reviewed the patch last time.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:878: if (HasSwappedThisFrame())
Probably want to use HasSentDrawRequestThisFrame() instead since we'll want to
stay active even if a swap had an early out or failed.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:983: return
(active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&
Please keep this HasSwappedThisFrame(). I didn't think of this before, but ff a
swap early outs, we would be able to transition from high latency mode to low
latency mode without even skipping a BeginMainFrame because this would allow us
to swap the pending commit immediately.

Yay! =)

[simonhong, 2014-05-08 01:14:57]

Dear brian and sami,
Sorry for late response. (I just come back from vacation :))
Please check again.
Many thanks!

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/draw_swap_r...
File cc/scheduler/draw_swap_readback_result.h (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/draw_swap_r...
cc/scheduler/draw_swap_readback_result.h:5: #ifndef
CC_SCHEDULER_DRAW_SWAP_READBACK_RESULT_H_
On 2014/05/01 10:21:44, Sami wrote:
> Mind renaming this file to draw_result.h too?

Renamed to draw_result.h.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.cc
File cc/scheduler/scheduler.cc (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.c...
cc/scheduler/scheduler.cc:190: // Swap should not occur by readback operation.
On 2014/05/01 10:21:44, Sami wrote:
> s/by/inside/?

Done.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler.c...
cc/scheduler/scheduler.cc:605: void Scheduler::DrawAndSwapForced() {
On 2014/05/01 10:21:44, Sami wrote:
> Looks like this and the next function could be removed in favor of just
calling
> the client directly.

Yep. Both are removed.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:768:
last_frame_number_draw_request_sent_ = current_frame_number_;
On 2014/05/02 04:08:58, brianderson wrote:
> Hmmm. This would mean that a readback, which didn't swap, could prevent the
> replacement commit from drawing until the next BeginFrame.
> 
> Maybe we do need to keep separate concepts in the scheduler of swap_requested
> vs. swap_performed, at least for this method. Sorry I didn't realize this when
I
> reviewed the patch last time.

Yep. I created last_frame_number_swap_request_sent_ and checks it in the
ShouldDraw().

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:878: if (HasSwappedThisFrame())
On 2014/05/02 04:08:58, brianderson wrote:
> Probably want to use HasSentDrawRequestThisFrame() instead since we'll want to
> stay active even if a swap had an early out or failed.

Done.

https://codereview.chromium.org/246753008/diff/40001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:983: return
(active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&
On 2014/05/02 04:08:58, brianderson wrote:
> Please keep this HasSwappedThisFrame(). I didn't think of this before, but ff
a
> swap early outs, we would be able to transition from high latency mode to low
> latency mode without even skipping a BeginMainFrame because this would allow
us
> to swap the pending commit immediately.
> 
> Yay! =)

Done.

[brianderson, 2014-05-08 02:24:13]

Hope you had a good vacation Simon. Welcome back!

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (left):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:631: 
Accident?

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:302: bool
SchedulerStateMachine::HasSentSwapRequestThisFrame() const {
HasRequestedSwapThisFrame?

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:306: bool
SchedulerStateMachine::HasSentDrawRequestThisFrame() const {
HasRequestedDrawThisFrame?

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:389: if (HasSentDrawRequestThisFrame()
&& HasSentSwapRequestThisFrame())
HasSentSwapRequestThisFrame should be good enough - is it not?

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:1026: return HasSwappedThisFrame() &&
!HasSentBeginMainFrameThisFrame();
Why was the active_tree_needs_first_draw_ condition removed?

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.h:274: bool
HasScheduledManageTilesThisFrame() const;
Is this used anywhere?

[simonhong, 2014-05-08 03:13:30]

All done!
Thanks for kind review!

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (left):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:631: 
On 2014/05/08 02:24:13, brianderson wrote:
> Accident?

Oops!
blank line restored.

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:302: bool
SchedulerStateMachine::HasSentSwapRequestThisFrame() const {
On 2014/05/08 02:24:13, brianderson wrote:
> HasRequestedSwapThisFrame?

Done.

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:306: bool
SchedulerStateMachine::HasSentDrawRequestThisFrame() const {
On 2014/05/08 02:24:13, brianderson wrote:
> HasRequestedDrawThisFrame?

Done.

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:389: if (HasSentDrawRequestThisFrame()
&& HasSentSwapRequestThisFrame())
On 2014/05/08 02:24:13, brianderson wrote:
> HasSentSwapRequestThisFrame should be good enough - is it not?

Yep, you're right.
Using it alone is enough.

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:1026: return HasSwappedThisFrame() &&
!HasSentBeginMainFrameThisFrame();
On 2014/05/08 02:24:13, brianderson wrote:
> Why was the active_tree_needs_first_draw_ condition removed?

My mistake. restored!

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/246753008/diff/80001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.h:274: bool
HasScheduledManageTilesThisFrame() const;
On 2014/05/08 02:24:13, brianderson wrote:
> Is this used anywhere?

Oops! added accidently.
Removed.

[Sami, 2014-05-08 14:18:11]

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:388: // After this line, we only want to
send swap request once per frame.
nit: "send a swap request"

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:920: // If we just sent draw request,
it's likely that we are going to produce
nit: "sent a draw request" or "requested a draw"

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:1030: return
(active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&
Should this be HasSwappedThisFrame() or HasSentSwapRequestThisFrame()? To keep
existing behavior I think we should call the latter, but I'm not sure if that's
right.

[simonhong, 2014-05-08 14:50:13]

Dear sami,
Please check my comment.
Thanks for review!

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:388: // After this line, we only want to
send swap request once per frame.
On 2014/05/08 14:18:11, Sami wrote:
> nit: "send a swap request"

Done.

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:920: // If we just sent draw request,
it's likely that we are going to produce
On 2014/05/08 14:18:11, Sami wrote:
> nit: "sent a draw request" or "requested a draw"

Done.

https://codereview.chromium.org/246753008/diff/130001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:1030: return
(active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&
On 2014/05/08 14:18:11, Sami wrote:
> Should this be HasSwappedThisFrame() or HasSentSwapRequestThisFrame()? To keep
> existing behavior I think we should call the latter, but I'm not sure if
that's
> right.

As brian said, we should check whether swap is performed(not just requested) or
not.
With this checking condition, if swap is early out, we can transition from high
latency mode
to low latency mode without skipping BeginMainFrame because this swap is handled
immediately.
If we check whether swap is requested or not in here, we don't know this commit
is swapped or not.
So, we should skip BeginMainFrame even if swap is early out.

[Sami, 2014-05-08 14:53:14]

On 2014/05/08 14:50:13, simonhong wrote:
> As brian said, we should check whether swap is performed(not just requested)
or
> not.
> With this checking condition, if swap is early out, we can transition from
high
> latency mode
> to low latency mode without skipping BeginMainFrame because this swap is
handled
> immediately.
> If we check whether swap is requested or not in here, we don't know this
commit
> is swapped or not.
> So, we should skip BeginMainFrame even if swap is early out.

Thanks for pointing that out, that makes sense. I missed the conversion you had
about this earlier.

lgtm, but please let Brian approve too.

[brianderson, 2014-05-08 17:40:19]

Thanks Simon. lgtm with final comments addressed.

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:925: if (HasRequestedDrawThisFrame())
In order to avoid a readback from triggering a proactive begin frame, this
should really be:
if (HasRequestedSwapThisFrame())

Sorry I didn't catch all these nuances at once.

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:276: bool HasRequestedSwapThisFrame()
const;
nit:
Here and elsewhere, can you reorder these so that it follows the natural order
of actions?
   void AdvanceCurrentFrameNumber();
   bool HasSentBeginMainFrameThisFrame() const;
   bool HasUpdatedVisibleTilesThisFrame() const;
+  bool HasRequestedDrawThisFrame() const;
+  bool HasRequestedSwapThisFrame() const;
   bool HasSwappedThisFrame() const;

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:297: int
last_frame_number_swap_request_sent_;
last_frame_number_swap_requested_

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:299: int
last_frame_number_draw_request_sent_;
last_frame_number_draw_requested_;

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1729: EXPECT_EQ(DRAW_SUCCESS,
Does "git cl format" remove this newline and others in this file?

[simonhong, 2014-05-09 00:18:40]

Dear brian,
Please check my comment.
Many thanks!

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:925: if (HasRequestedDrawThisFrame())
On 2014/05/08 17:40:19, brianderson wrote:
> In order to avoid a readback from triggering a proactive begin frame, this
> should really be:
> if (HasRequestedSwapThisFrame())
> 
> Sorry I didn't catch all these nuances at once.

No problem!
Also HasRequestedDrawThisFrame() is removed because here was the only caller of
it.

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:276: bool HasRequestedSwapThisFrame()
const;
On 2014/05/08 17:40:19, brianderson wrote:
> nit:
> Here and elsewhere, can you reorder these so that it follows the natural order
> of actions?
>    void AdvanceCurrentFrameNumber();
>    bool HasSentBeginMainFrameThisFrame() const;
>    bool HasUpdatedVisibleTilesThisFrame() const;
> +  bool HasRequestedDrawThisFrame() const;
> +  bool HasRequestedSwapThisFrame() const;
>    bool HasSwappedThisFrame() const;

Done.

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:297: int
last_frame_number_swap_request_sent_;
On 2014/05/08 17:40:19, brianderson wrote:
> last_frame_number_swap_requested_

Done.

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:299: int
last_frame_number_draw_request_sent_;
On 2014/05/08 17:40:19, brianderson wrote:
> last_frame_number_draw_requested_;

Done.

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1729: EXPECT_EQ(DRAW_SUCCESS,
On 2014/05/08 17:40:19, brianderson wrote:
> Does "git cl format" remove this newline and others in this file?

Sorry, I can't catch your meaning.
Where is the new line you concern?
Do you mean that there should be a new line between 1278 and 1729?

[brianderson, 2014-05-09 00:27:18]

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:925: if (HasRequestedDrawThisFrame())
On 2014/05/09 00:18:40, simonhong wrote:
> On 2014/05/08 17:40:19, brianderson wrote:
> > In order to avoid a readback from triggering a proactive begin frame, this
> > should really be:
> > if (HasRequestedSwapThisFrame())
> > 
> > Sorry I didn't catch all these nuances at once.
> 
> No problem!
> Also HasRequestedDrawThisFrame() is removed because here was the only caller
of
> it.

Nice!

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1729: EXPECT_EQ(DRAW_SUCCESS,
On 2014/05/09 00:18:40, simonhong wrote:
> On 2014/05/08 17:40:19, brianderson wrote:
> > Does "git cl format" remove this newline and others in this file?
> 
> Sorry, I can't catch your meaning.
> Where is the new line you concern?
> Do you mean that there should be a new line between 1278 and 1729?

Does 1730 fit on line 1729?
At the command prompt, you can run "git cl format" to automatically format your
code. I was wondering if it would catch this.

https://codereview.chromium.org/246753008/diff/190001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/190001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:911: // If we just sent a draw request,
it's likely that we are going to produce
Please update comment.

[simonhong, 2014-05-09 00:35:26]

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/246753008/diff/150001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:1729: EXPECT_EQ(DRAW_SUCCESS,
On 2014/05/09 00:27:18, brianderson wrote:
> On 2014/05/09 00:18:40, simonhong wrote:
> > On 2014/05/08 17:40:19, brianderson wrote:
> > > Does "git cl format" remove this newline and others in this file?
> > 
> > Sorry, I can't catch your meaning.
> > Where is the new line you concern?
> > Do you mean that there should be a new line between 1278 and 1729?
> 
> Does 1730 fit on line 1729?
> At the command prompt, you can run "git cl format" to automatically format
your
> code. I was wondering if it would catch this.

I didn't make it one line because merged line exceeds 80 characters limit.
But 'git cl format' didn't catch anything in both cases(merge into one line or
not).

https://codereview.chromium.org/246753008/diff/190001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/246753008/diff/190001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:911: // If we just sent a draw request,
it's likely that we are going to produce
On 2014/05/09 00:27:18, brianderson wrote:
> Please update comment.

Done.

[simonhong, 2014-05-09 21:20:03]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-05-09 21:22:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonhong@chromium.org/246753008/230001

[commit-bot: I haz the power, 2014-05-11 00:38:10]

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

[commit-bot: I haz the power, 2014-05-11 00:38:11]

Failed to apply patch for cc/scheduler/scheduler_state_machine_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/scheduler/scheduler_state_machine_unittest.cc
  Hunk #1 succeeded at 255 (offset 1 line).
  Hunk #2 succeeded at 329 (offset 1 line).
  Hunk #3 FAILED at 355.
  Hunk #4 FAILED at 389.
  Hunk #5 succeeded at 425 (offset 1 line).
  Hunk #6 FAILED at 436.
  Hunk #7 succeeded at 476 (offset 1 line).
  Hunk #8 succeeded at 502 (offset 1 line).
  Hunk #9 succeeded at 551 (offset 1 line).
  Hunk #10 succeeded at 574 (offset 1 line).
  Hunk #11 succeeded at 602 (offset 1 line).
  Hunk #12 succeeded at 619 (offset 1 line).
  Hunk #13 succeeded at 643 (offset 1 line).
  Hunk #14 succeeded at 663 (offset 1 line).
  Hunk #15 succeeded at 912 (offset 1 line).
  Hunk #16 succeeded at 972 (offset 1 line).
  Hunk #17 succeeded at 1026 (offset 1 line).
  Hunk #18 succeeded at 1139 (offset 1 line).
  Hunk #19 succeeded at 1297 (offset 1 line).
  Hunk #20 succeeded at 1364 (offset 1 line).
  Hunk #21 succeeded at 1424 (offset 1 line).
  Hunk #22 succeeded at 1698 (offset 1 line).
  Hunk #23 succeeded at 1739 (offset 1 line).
  Hunk #24 succeeded at 1813 (offset 1 line).
  Hunk #25 succeeded at 1884 (offset 1 line).
  Hunk #26 succeeded at 1922 (offset 1 line).
  Hunk #27 succeeded at 1958 (offset 1 line).
  Hunk #28 succeeded at 2001 (offset 1 line).
  3 out of 28 hunks FAILED -- saving rejects to file
cc/scheduler/scheduler_state_machine_unittest.cc.rej

Patch:       cc/scheduler/scheduler_state_machine_unittest.cc
Index: cc/scheduler/scheduler_state_machine_unittest.cc
diff --git a/cc/scheduler/scheduler_state_machine_unittest.cc
b/cc/scheduler/scheduler_state_machine_unittest.cc
index
067f019739b1b385f5f2188806bc849b4ab7f156..a00e43a9e9476947daf2639a9a1302d5e9811d2f
100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -254,6 +254,8 @@ TEST(SchedulerStateMachineTest, MainFrameBeforeDrawDisabled)
{
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_EQ(state.CommitState(), SchedulerStateMachine::COMMIT_STATE_IDLE);
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
@@ -326,6 +328,8 @@ TEST(SchedulerStateMachineTest,
MainFrameBeforeActivationEnabled) {
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ANIMATE);
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_EQ(state.CommitState(), SchedulerStateMachine::COMMIT_STATE_IDLE);
@@ -351,14 +355,15 @@ TEST(SchedulerStateMachineTest,
   // We're drawing now.
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   EXPECT_FALSE(state.RedrawPending());
   EXPECT_FALSE(state.CommitPending());
 
   // Failing the draw makes us require a commit.
-  state.DidDrawIfPossibleCompleted(
-      DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
+  state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ANIMATE);
   EXPECT_ACTION_UPDATE_STATE(
@@ -385,13 +390,14 @@ TEST(SchedulerStateMachineTest,
TestFailedDrawForMissingHighResNeedsCommit) {
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_FALSE(state.RedrawPending());
   EXPECT_FALSE(state.CommitPending());
 
   // Missing high res content requires a commit (but not a redraw)
-  state.DidDrawIfPossibleCompleted(
-      DrawSwapReadbackResult::DRAW_ABORTED_MISSING_HIGH_RES_CONTENT);
+  state.DidDrawIfPossibleCompleted(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT);
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
@@ -420,6 +426,8 @@ TEST(SchedulerStateMachineTest,
   // We're drawing now.
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_FALSE(state.RedrawPending());
   EXPECT_FALSE(state.CommitPending());
@@ -430,8 +438,7 @@ TEST(SchedulerStateMachineTest,
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Failing the draw for animation checkerboards makes us require a commit.
-  state.DidDrawIfPossibleCompleted(
-      DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
+  state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   state.OnBeginImplFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ANIMATE);
   EXPECT_ACTION_UPDATE_STATE(
@@ -469,8 +476,7 @@ void TestFailedDrawsEventuallyForceDrawAfterNextCommit(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
 
   // Fail the draw.
-  state.DidDrawIfPossibleCompleted(
-      DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
+  state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_TRUE(state.BeginFrameNeeded());
   EXPECT_TRUE(state.RedrawPending());
@@ -496,6 +502,8 @@ void TestFailedDrawsEventuallyForceDrawAfterNextCommit(
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
 }
 
 TEST(SchedulerStateMachineTest,
@@ -543,10 +551,8 @@ TEST(SchedulerStateMachineTest,
TestFailedDrawsDoNotRestartForcedDraw) {
 
   // Fail the draw enough times to force a redraw,
   // then once more for good measure.
-  for (int i = 0; i < draw_limit + 1; ++i) {
-    state.DidDrawIfPossibleCompleted(
-        DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
-  }
+  for (int i = 0; i < draw_limit + 1; ++i)
+    state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_TRUE(state.BeginFrameNeeded());
   EXPECT_TRUE(state.RedrawPending());
@@ -568,10 +574,8 @@ TEST(SchedulerStateMachineTest,
TestFailedDrawsDoNotRestartForcedDraw) {
 
   // After failing additional draws, we should still be in a forced
   // redraw, but not back in WAITING_FOR_COMMIT.
-  for (int i = 0; i < draw_limit + 1; ++i) {
-    state.DidDrawIfPossibleCompleted(
-        DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
-  }
+  for (int i = 0; i < draw_limit + 1; ++i)
+    state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   EXPECT_TRUE(state.RedrawPending());
   EXPECT_TRUE(state.ForcedRedrawState() ==
     SchedulerStateMachine::FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION);
@@ -598,8 +602,7 @@ TEST(SchedulerStateMachineTest,
TestFailedDrawIsRetriedInNextBeginImplFrame) {
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
 
   // Failing the draw for animation checkerboards makes us require a commit.
-  state.DidDrawIfPossibleCompleted(
-      DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
+  state.DidDrawIfPossibleCompleted(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS);
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -616,6 +619,8 @@ TEST(SchedulerStateMachineTest,
TestFailedDrawIsRetriedInNextBeginImplFrame) {
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
+  state.DidSwapBuffers();
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
@@ -638,7 +643,9 @@ TEST(SchedulerStateMachineTest,
TestDoestDrawTwiceInSameFrame) {
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
-  state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
+  state.DidSwapBuffers();
+  state.DidDrawIfPossibleCompleted(DRAW_SUCCESS);
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Before the next BeginImplFrame, set needs redraw again.
@@ -656,7 +663,9 @@ TEST(SchedulerStateMachineTest,
TestDoestDrawTwiceInSameFrame) {
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
-  state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
+  state.DidSwapBuffers();
+  state.DidDrawIfPossibleCompleted(DRAW_SUCCESS);
+  state.DidSwapBuffersComplete();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // We just swapped, so we should proactively request another BeginImplFrame.
@@ -903,7 +912,9 @@ void TestSetNeedsCommitIsNotLost(bool
main_frame_before_draw_enabled) {
   EXPECT_TRUE(state.active_tree_needs_first_draw());
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
-  state.DidDrawIfPossibleCompleted(DrawSwapReadbackResult::DRAW_SUCCESS);
+  state.DidSwapBuffers();
+  state.DidDrawIfPossib…
(message too large)

[simonhong, 2014-05-11 01:42:28]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-05-11 01:43:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonhong@chromium.org/246753008/270001

[commit-bot: I haz the power, 2014-05-11 02:12:35]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  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-05-11 02:15:35]

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

[commit-bot: I haz the power, 2014-05-11 02:15:36]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[simonhong, 2014-05-11 02:57:54]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-05-11 02:58:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonhong@chromium.org/246753008/270001

[commit-bot: I haz the power, 2014-05-11 03:25:05]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  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-05-11 03:26:55]

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

[commit-bot: I haz the power, 2014-05-11 03:26:57]

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

[simonhong, 2014-05-11 08:19:36]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-05-11 08:19:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonhong@chromium.org/246753008/270001

[commit-bot: I haz the power, 2014-05-11 08:50:29]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  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-05-11 08:53:55]

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

[commit-bot: I haz the power, 2014-05-11 08:53:56]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[simonhong, 2014-05-12 00:05:27]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-05-12 00:05:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonhong@chromium.org/246753008/290001

[commit-bot: I haz the power, 2014-05-12 04:26:05]

Change committed as 269738