Link GCM heartbeat with wake on wifi preference

Link GCM heartbeat with wake on wifi preference

Currently, the GCM heartbeat manager schedules a timer that will always
wake up the system from suspend in order to send a heartbeat message.
This is bad for battery life if the user doesn't actually want to use
the wake on packet feature.  To deal with this problem:

- Add a new function, SetWakeFromSuspend, to the AlarmTimer class so
  that its wake from suspend ability can be changed on the fly.
- Drill a hole through GCM's beautiful abstraction layers so that we can
  get to the HeartbeatManager via the GCMDriver interface.
- Hook up the endpoint in the GCMDriver interface to
  WakeOnWifiManager::OnPreferenceChanged so that the wake from suspend
  property for all the GCM heartbeat timers is changed along with
  shill's settings.

BUG=397328, chrome-os-partner:34142
Committed: https://crrev.com/192a92175999160848b161f09b5c4a798e39d82c
Cr-Commit-Position: refs/heads/master@{#307151}

[Chirantan Ekbote, 2014-11-21 03:27:34]

chirantan@chromium.org changed reviewers:
+ derat@chromium.org, stevenjb@chromium.org, thestig@chromium.org,
zea@chromium.org

[Chirantan Ekbote, 2014-11-21 03:27:35]

Please take a look.

Lei: Please review base/
Dan: Please review timers/
Steven: Please review chromeos/net
Nicolas: Please review gcm.  I'm not particularly happy with the callback that I
had to add to deal with GCMDriver::SetWakeFromSuspendForHeartbeat being called
before the GCM service had started.  I considered making a GCMStartedObserver
class but that seemed like overkill for this.  I'd be happy to listen to any
suggestions you have to make this nicer.

Thanks :-)

[Daniel Erat, 2014-11-21 14:46:49]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
when you set this to false do you actually still care about the timer firing, or
could you just stop the timer instead?

[Chirantan Ekbote, 2014-11-21 18:09:51]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
On 2014/11/21 14:46:49, Daniel Erat wrote:
> when you set this to false do you actually still care about the timer firing,
or
> could you just stop the timer instead?

I'm not sure that SetWakeFromSuspend(false) should also imply Stop(), although
that would definitely make the code simpler.  Put another way, GCM still cares
about regularly sending heartbeats when the system is awake even if we're no
longer waking from suspend to do it.  It's just a question of whether we deal
with the restart within this function or have the callers deal with it instead. 
I chose to do the former but I could easily change it to the latter if that's
what we decide to go with.

Also, if we choose to go with stopping the timer then we would have to do it no
matter what the value of |wake_from_suspend| is because otherwise it gets a
little messy, especially when going from false -> true.

[Daniel Erat, 2014-11-21 22:25:18]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.cc (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.cc:45: // If the timer's current wake from suspend
setting already matches the
nit: delete unnecessary comment; the pattern of bailing out early when the
requested setting already matches the current one is awfully common :-)

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.cc:59: base::Timer::desired_run_time() -
base::TimeTicks::Now());
nit: should probably cap this to a minimum of base::TimeDelta() just in case we
happen to pass the desired run time while this method is running

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
On 2014/11/21 18:09:51, chirantan wrote:
> On 2014/11/21 14:46:49, Daniel Erat wrote:
> > when you set this to false do you actually still care about the timer
firing,
> or
> > could you just stop the timer instead?
> 
> I'm not sure that SetWakeFromSuspend(false) should also imply Stop(), although
> that would definitely make the code simpler.  Put another way, GCM still cares
> about regularly sending heartbeats when the system is awake even if we're no
> longer waking from suspend to do it.  It's just a question of whether we deal
> with the restart within this function or have the callers deal with it
instead. 
> I chose to do the former but I could easily change it to the latter if that's
> what we decide to go with.
> 
> Also, if we choose to go with stopping the timer then we would have to do it
no
> matter what the value of |wake_from_suspend| is because otherwise it gets a
> little messy, especially when going from false -> true.

it seems weird to me to have an AlarmTimer class that exists solely to wake the
system and run a timer, except it only works on chrome os and is unlikely to be
implemented anywhere else in the foreseeable future, except it also has a method
you can call on chrome os to make it not work there either. i still think it
might be cleaner to have this be a chrome-os-only class and push all the logic
about whether to use it or whether to disable it up into the callers -- it feels
like some of the callers' logic is leaking into this class otherwise.

[Chirantan Ekbote, 2014-11-24 23:28:35]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
On 2014/11/21 22:25:18, Daniel Erat wrote:
> On 2014/11/21 18:09:51, chirantan wrote:
> > On 2014/11/21 14:46:49, Daniel Erat wrote:
> > > when you set this to false do you actually still care about the timer
> firing,
> > or
> > > could you just stop the timer instead?
> > 
> > I'm not sure that SetWakeFromSuspend(false) should also imply Stop(),
although
> > that would definitely make the code simpler.  Put another way, GCM still
cares
> > about regularly sending heartbeats when the system is awake even if we're no
> > longer waking from suspend to do it.  It's just a question of whether we
deal
> > with the restart within this function or have the callers deal with it
> instead. 
> > I chose to do the former but I could easily change it to the latter if
that's
> > what we decide to go with.
> > 
> > Also, if we choose to go with stopping the timer then we would have to do it
> no
> > matter what the value of |wake_from_suspend| is because otherwise it gets a
> > little messy, especially when going from false -> true.
> 
> it seems weird to me to have an AlarmTimer class that exists solely to wake
the
> system and run a timer, except it only works on chrome os and is unlikely to
be
> implemented anywhere else in the foreseeable future, except it also has a
method
> you can call on chrome os to make it not work there either. i still think it
> might be cleaner to have this be a chrome-os-only class and push all the logic
> about whether to use it or whether to disable it up into the callers -- it
feels
> like some of the callers' logic is leaking into this class otherwise.

So this is still a chrome-os-only class and the build target is only available
when OS_CHROMEOS is defined.  That hasn't changed.

Re: pushing the logic up to the caller: This would mean that we change the timer
used for heartbeats whenever the preference changes, which would mean we do the
equivalent of this function in the HeartbeatManager instead.  I have no problem
with this.  zea@: what do you think?

[Nicolas Zea, 2014-12-03 21:12:38]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
On 2014/11/24 23:28:35, chirantan wrote:
> On 2014/11/21 22:25:18, Daniel Erat wrote:
> > On 2014/11/21 18:09:51, chirantan wrote:
> > > On 2014/11/21 14:46:49, Daniel Erat wrote:
> > > > when you set this to false do you actually still care about the timer
> > firing,
> > > or
> > > > could you just stop the timer instead?
> > > 
> > > I'm not sure that SetWakeFromSuspend(false) should also imply Stop(),
> although
> > > that would definitely make the code simpler.  Put another way, GCM still
> cares
> > > about regularly sending heartbeats when the system is awake even if we're
no
> > > longer waking from suspend to do it.  It's just a question of whether we
> deal
> > > with the restart within this function or have the callers deal with it
> > instead. 
> > > I chose to do the former but I could easily change it to the latter if
> that's
> > > what we decide to go with.
> > > 
> > > Also, if we choose to go with stopping the timer then we would have to do
it
> > no
> > > matter what the value of |wake_from_suspend| is because otherwise it gets
a
> > > little messy, especially when going from false -> true.
> > 
> > it seems weird to me to have an AlarmTimer class that exists solely to wake
> the
> > system and run a timer, except it only works on chrome os and is unlikely to
> be
> > implemented anywhere else in the foreseeable future, except it also has a
> method
> > you can call on chrome os to make it not work there either. i still think it
> > might be cleaner to have this be a chrome-os-only class and push all the
logic
> > about whether to use it or whether to disable it up into the callers -- it
> feels
> > like some of the callers' logic is leaking into this class otherwise.
> 
> So this is still a chrome-os-only class and the build target is only available
> when OS_CHROMEOS is defined.  That hasn't changed.
> 
> Re: pushing the logic up to the caller: This would mean that we change the
timer
> used for heartbeats whenever the preference changes, which would mean we do
the
> equivalent of this function in the HeartbeatManager instead.  I have no
problem
> with this.  zea@: what do you think?

So, my concern is that GCM, and I suspect other callers, also don't care about
the alarm aspect. Disabling the alarm is a user defined (or possibly just
system-wide) preference. It seems wrong to me to make GCM aware of what is in my
mind an implementation detail of CrOS alarms, especially if its more than just
GCM that will be concerned about this kind of thing (hence why we put this in
components/ in the first place right?)

Dan: my understanding is that the AlarmTimer exists to allow CrOS to control the
timer. As such, deciding whether to wake from suspend is part of that decision.

Perhaps we should, instead of having a SetWakeFromSuspend method, have the
AlarmTimer implement some sort of listener method through which it can register
for changes to the profile/system wide pref controlling whether
wake-from-suspend is enabled? The constructor can therefore just take a
PrefService object or something.

[Chirantan Ekbote, 2014-12-03 22:59:18]

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
File components/timers/alarm_timer.h (right):

https://codereview.chromium.org/745123002/diff/20001/components/timers/alarm_...
components/timers/alarm_timer.h:70: void SetWakeFromSuspend(bool
wake_from_suspend);
On 2014/12/03 21:12:38, Nicolas Zea wrote:
> 
> So, my concern is that GCM, and I suspect other callers, also don't care about
> the alarm aspect. Disabling the alarm is a user defined (or possibly just
> system-wide) preference. It seems wrong to me to make GCM aware of what is in
my
> mind an implementation detail of CrOS alarms, especially if its more than just
> GCM that will be concerned about this kind of thing (hence why we put this in
> components/ in the first place right?)
> 

We put this in components/ because that was the only way to not duplicate this
class in Chrome and Chrome OS code.  Also I disagree that other callers don't
care about the alarm aspect; that's pretty much the only reason they are even
using this class.  powerd uses it to schedule a battery check and will never
want to turn off the wake from suspend feature, and shill simply stops the timer
if it doesn't need to wake from suspend anymore.  GCM is the unusual case
because the timer needs to keep working but waking from suspend is a waste of
power if it is not necessary.

> Dan: my understanding is that the AlarmTimer exists to allow CrOS to control
the
> timer. As such, deciding whether to wake from suspend is part of that
decision.
> 
> Perhaps we should, instead of having a SetWakeFromSuspend method, have the
> AlarmTimer implement some sort of listener method through which it can
register
> for changes to the profile/system wide pref controlling whether
> wake-from-suspend is enabled? The constructor can therefore just take a
> PrefService object or something.

The powerd use case is an example of a usage of this class that has nothing to
do with wake on wifi so I don't think adding a dependency on the wake on wifi
pref makes sense here.  Plus, shill and powerd don't use PrefService and the
pref names are in chrome/, which this class cannot depend on.

For now the problem (unnecessary wakes from suspend) is restricted to GCM's
usage of the AlarmTimer.  The question is just whether we should fix it within
this class or whether we should fix it in the way GCM uses this class.

[Nicolas Zea, 2014-12-03 23:10:21]

I see. Okay, I misunderstood then. I was under the impression that AlarmTimer
had or was going to have other use cases outside of GCM that also didn't really
care about how the underlying alarm is implemented.

Okay, I'm shifting more towards making GCM wake-from-suspend-aware. As long as
that's largely contained within the heartbeat manager itself that should be ok.
Ideally this wouldn't need to affect most other parts of the gcm codebase (aside
from possibly plumbing through some sort of event notifier/listener or
something).

[Chirantan Ekbote, 2014-12-03 23:32:20]

On 2014/12/03 23:10:21, Nicolas Zea wrote:
> I see. Okay, I misunderstood then. I was under the impression that AlarmTimer
> had or was going to have other use cases outside of GCM that also didn't
really
> care about how the underlying alarm is implemented.
> 
> Okay, I'm shifting more towards making GCM wake-from-suspend-aware. As long as
> that's largely contained within the heartbeat manager itself that should be
ok.
> Ideally this wouldn't need to affect most other parts of the gcm codebase
(aside
> from possibly plumbing through some sort of event notifier/listener or
> something).

sgtm.  I was thinking of making a HeartbeatManager::UpdateHeartbeatTimer
function and calling it from GCMDriverDesktop::IOWorker so that most of the
other plumbing from this CL will remain unchanged.

[Chirantan Ekbote, 2014-12-04 23:11:25]

This patch set moves the wake from suspend stuff out of the AlarmTimer and into
GCM.  Remaining changes to AlarmTimer are cosmetic (making an accessor const and
reordering functions).  There is also just a one line change to
base/timer/timer.h to remove a duplicate accessor function that I added by
mistake in a previous CL.

Please take a look.

[Daniel Erat, 2014-12-04 23:23:16]

lgtm for the now-minimal non-gcm bits. :-)

as a side note, i think that something might be messed up with your account on
codereview.chromium.org -- replies from you always show up with a "From:
reply@chromiumcodereview-hr.appspotmail.com". do you have a nickname set at
https://codereview.chromium.org/settings?

https://codereview.chromium.org/745123002/diff/40001/components/timers/rtc_al...
File components/timers/rtc_alarm.cc (right):

https://codereview.chromium.org/745123002/diff/40001/components/timers/rtc_al...
components/timers/rtc_alarm.cc:91: if (delay <=
base::TimeDelta::FromMicroseconds(0)) {
any reason you can't use the shorter base::TimeDelta() here and below? that's
what i usually use when i want a zero-length delta.

[Chirantan Ekbote, 2014-12-04 23:31:40]

I do have a nickname set but I noticed the reply@chromiumcodereview... thing
started happening after I turned on the "Add plus role" checkbox in the
settings.  I've turned it off now since I didn't actually set up any filters
based on it.  It should show the correct address now.

https://codereview.chromium.org/745123002/diff/40001/components/timers/rtc_al...
File components/timers/rtc_alarm.cc (right):

https://codereview.chromium.org/745123002/diff/40001/components/timers/rtc_al...
components/timers/rtc_alarm.cc:91: if (delay <=
base::TimeDelta::FromMicroseconds(0)) {
On 2014/12/04 23:23:16, Daniel Erat wrote:
> any reason you can't use the shorter base::TimeDelta() here and below? that's
> what i usually use when i want a zero-length delta.

I did it mainly to make it painfully explicit and to be consistent with how
base::Timer performs this check.  I can change it to base::TimeDelta() though.

[Nicolas Zea, 2014-12-05 00:53:05]

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_client.h (right):

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_client.h:306: virtual MCSClient* GetMCSClient() const
= 0;
I'm not a fan of exposing all the logic of MCSClient outside of the GCMClient.
How about just having a UpdateHeartbeatTimer method that just gets plumbed
through to the heartbeat manager?

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_desktop.cc (right):

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_desktop.cc:692: gcm_started_callback_ =
I don't think we need a generic callback here until other tasks might actually
be run. We can just cache the wake value in a boolean
wake_from_suspend_enabled_, and pass that to the GCMClient when (or just after)
it gets created.

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
File google_apis/gcm/engine/heartbeat_manager.cc (right):

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
google_apis/gcm/engine/heartbeat_manager.cc:86: Stop();
I don't think you should be calling Stop() here. If we were waiting for a
heartbeat ack, we should still be waiting for one even if the user happened to
flip the pref.

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
File google_apis/gcm/engine/heartbeat_manager_unittest.cc (right):

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
google_apis/gcm/engine/heartbeat_manager_unittest.cc:194:
base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(2));
why is this necessary?

[Chirantan Ekbote, 2014-12-05 21:49:05]

ptal

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_client.h (right):

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_client.h:306: virtual MCSClient* GetMCSClient() const
= 0;
On 2014/12/05 00:53:04, Nicolas Zea wrote:
> I'm not a fan of exposing all the logic of MCSClient outside of the GCMClient.
> How about just having a UpdateHeartbeatTimer method that just gets plumbed
> through to the heartbeat manager?

Done.

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_desktop.cc (right):

https://codereview.chromium.org/745123002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_desktop.cc:692: gcm_started_callback_ =
On 2014/12/05 00:53:05, Nicolas Zea wrote:
> I don't think we need a generic callback here until other tasks might actually
> be run. We can just cache the wake value in a boolean
> wake_from_suspend_enabled_, and pass that to the GCMClient when (or just
after)
> it gets created.

Done.

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
File google_apis/gcm/engine/heartbeat_manager.cc (right):

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
google_apis/gcm/engine/heartbeat_manager.cc:86: Stop();
On 2014/12/05 00:53:05, Nicolas Zea wrote:
> I don't think you should be calling Stop() here. If we were waiting for a
> heartbeat ack, we should still be waiting for one even if the user happened to
> flip the pref.

Done.

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
File google_apis/gcm/engine/heartbeat_manager_unittest.cc (right):

https://codereview.chromium.org/745123002/diff/40001/google_apis/gcm/engine/h...
google_apis/gcm/engine/heartbeat_manager_unittest.cc:194:
base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(2));
On 2014/12/05 00:53:05, Nicolas Zea wrote:
> why is this necessary?

Because otherwise the test would be too fast to reliably see a difference in the
scheduled heartbeat time.  But since the new patch set makes the timer change
transparent, this is no longer necessary.

[Nicolas Zea, 2014-12-05 22:28:11]

LGTM with a question. Thanks for working this out!

https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_desktop.cc (right):

https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_desktop.cc:762:
WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_);
nit: should we only call this if wake_from_suspend_enabled_ is true?

[Chirantan Ekbote, 2014-12-05 22:38:46]

https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_desktop.cc (right):

https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_desktop.cc:762:
WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_);
On 2014/12/05 22:28:11, Nicolas Zea wrote:
> nit: should we only call this if wake_from_suspend_enabled_ is true?

Not necessarily.  On chrome os, the HeartbeatManager is initialized with an
AlarmTimer so we would need to call this even if wake_from_suspend_enabled_ was
false.

Maybe if I change that code to always create a base::Timer, then we could change
this to only be called if wake_from_suspend_enabled_ was true.  wdyt?

[Nicolas Zea, 2014-12-05 22:59:58]

On 2014/12/05 22:38:46, chirantan wrote:
>
https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
> File components/gcm_driver/gcm_driver_desktop.cc (right):
> 
>
https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
> components/gcm_driver/gcm_driver_desktop.cc:762:
> WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_);
> On 2014/12/05 22:28:11, Nicolas Zea wrote:
> > nit: should we only call this if wake_from_suspend_enabled_ is true?
> 
> Not necessarily.  On chrome os, the HeartbeatManager is initialized with an
> AlarmTimer so we would need to call this even if wake_from_suspend_enabled_
was
> false.
> 
> Maybe if I change that code to always create a base::Timer, then we could
change
> this to only be called if wake_from_suspend_enabled_ was true.  wdyt?

Hmm, yeah, I think I'd prefer the default to always be a base::Timer, and only
change if necessary.

[Chirantan Ekbote, 2014-12-05 23:12:54]

On 2014/12/05 22:59:58, Nicolas Zea wrote:
> On 2014/12/05 22:38:46, chirantan wrote:
> >
>
https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
> > File components/gcm_driver/gcm_driver_desktop.cc (right):
> > 
> >
>
https://codereview.chromium.org/745123002/diff/80001/components/gcm_driver/gc...
> > components/gcm_driver/gcm_driver_desktop.cc:762:
> > WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_);
> > On 2014/12/05 22:28:11, Nicolas Zea wrote:
> > > nit: should we only call this if wake_from_suspend_enabled_ is true?
> > 
> > Not necessarily.  On chrome os, the HeartbeatManager is initialized with an
> > AlarmTimer so we would need to call this even if wake_from_suspend_enabled_
> was
> > false.
> > 
> > Maybe if I change that code to always create a base::Timer, then we could
> change
> > this to only be called if wake_from_suspend_enabled_ was true.  wdyt?
> 
> Hmm, yeah, I think I'd prefer the default to always be a base::Timer, and only
> change if necessary.

Ok, I just made that change.  It actually allowed me to clean up a bunch of
other places as well.  Please take another look.

[Nicolas Zea, 2014-12-05 23:14:52]

Nice, LGTM!

[Chirantan Ekbote, 2014-12-05 23:15:08]

The CQ bit was checked by chirantan@chromium.org

[commit-bot: I haz the power, 2014-12-05 23:15:50]

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

[commit-bot: I haz the power, 2014-12-06 00:10:14]

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

[commit-bot: I haz the power, 2014-12-06 00:10:16]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Chirantan Ekbote, 2014-12-06 02:23:32]

The CQ bit was checked by chirantan@chromium.org

[commit-bot: I haz the power, 2014-12-06 02:23:55]

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

[commit-bot: I haz the power, 2014-12-06 03:30:54]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-12-06 03:31:48]

Patchset 8 (id:??) landed as
https://crrev.com/192a92175999160848b161f09b5c4a798e39d82c
Cr-Commit-Position: refs/heads/master@{#307151}