Generalize network_event_log -> device_event_log

Generalize network_event_log -> device_event_log

This CL:
* Moves network_event_log.cc -> device_event_log_impl.cc
* Defines macros and initialization code in device_event_log.h
* Modifies network_event_log.h to use device_event_log instead.
* Converts PowerManagerClient logging to use POWER_LOG as an immediately useful example.
* Adds the non-network log entries to a 'device_log' entry in chrome://system and feedback reports.

BUG=293739
Committed: https://crrev.com/252993533a1b5cb7bcec6ab058f1d62144ca999e
Cr-Commit-Position: refs/heads/master@{#307086}

[stevenjb, 2014-12-02 19:59:27]

Patchset #2 (id:20001) has been deleted

[stevenjb, 2014-12-02 20:02:04]

Patchset #2 (id:40001) has been deleted

[stevenjb, 2014-12-02 20:03:23]

Patchset #2 (id:60001) has been deleted

[stevenjb, 2014-12-02 20:04:24]

Patchset #2 (id:80001) has been deleted

[stevenjb, 2014-12-02 20:13:14]

Patchset #2 (id:100001) has been deleted

[stevenjb, 2014-12-02 20:19:59]

stevenjb@chromium.org changed reviewers:
+ benchan@chromium.org, derat@chromium.org, jennyz@chromium.org,
pneubeck@chromium.org, rkc@chromium.org

[stevenjb, 2014-12-02 20:20:00]

rkc@ - owner for c/b/chromeos/system_logs/
derat@ - ptal at power_manager_client.cc
benchan@, jennyz@ - please review other changes
pneubeck@ - FYI

[Daniel Erat, 2014-12-02 20:43:16]

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:209: POWER_LOG(ERROR) << "Error calling "
<< power_manager::kSetPolicyMethod;
does this still end up in a log file on disk? if i'm looking at a system that's
misbehaving i'd like this message to be easy to find. can POWER_LOG(ERROR)
messages be duplicated to the main chrome log?

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:419: POWER_LOG(DEBUG) << "Received power
supply poll signal.";
why did you use different levels for this and for
PeripheralBatteryStatusReceived? they were both VLOG(1) before, and i believe
that they're both called in response to signals emitted periodically by powerd.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:52: LOG_LEVEL_EVENT = 2,
please add comments describing how to use these levels. is USER for
user-triggered events while EVENT is for system-triggered ones?

[stevenjb, 2014-12-02 21:14:39]

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:209: POWER_LOG(ERROR) << "Error calling "
<< power_manager::kSetPolicyMethod;
On 2014/12/02 20:43:16, Daniel Erat wrote:
> does this still end up in a log file on disk? if i'm looking at a system
that's
> misbehaving i'd like this message to be easy to find. can POWER_LOG(ERROR)
> messages be duplicated to the main chrome log?

POWER_LOG(ERROR) still logs to LOG(ERROR). Other levels go to VLOG(1) (but with
--vmodule=device_log*=1). Sound reasonable?

I've been thinking about the merits of also spooling the log to disk alongside
the chrome log, but that's not currently a priority.

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:419: POWER_LOG(DEBUG) << "Received power
supply poll signal.";
On 2014/12/02 20:43:16, Daniel Erat wrote:
> why did you use different levels for this and for
> PeripheralBatteryStatusReceived? they were both VLOG(1) before, and i believe
> that they're both called in response to signals emitted periodically by
powerd.

I guess it seemed more like a VLOG(2) to me since it's just a poll. I could make
both DEBUG or neither, whichever you prefer.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:52: LOG_LEVEL_EVENT = 2,
On 2014/12/02 20:43:16, Daniel Erat wrote:
> please add comments describing how to use these levels. is USER for
> user-triggered events while EVENT is for system-triggered ones?

I put comments above, I will refer to that here.

[Daniel Erat, 2014-12-02 21:20:39]

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:209: POWER_LOG(ERROR) << "Error calling "
<< power_manager::kSetPolicyMethod;
On 2014/12/02 21:14:38, stevenjb wrote:
> On 2014/12/02 20:43:16, Daniel Erat wrote:
> > does this still end up in a log file on disk? if i'm looking at a system
> that's
> > misbehaving i'd like this message to be easy to find. can POWER_LOG(ERROR)
> > messages be duplicated to the main chrome log?
> 
> POWER_LOG(ERROR) still logs to LOG(ERROR). Other levels go to VLOG(1) (but
with
> --vmodule=device_log*=1). Sound reasonable?
> 
> I've been thinking about the merits of also spooling the log to disk alongside
> the chrome log, but that's not currently a priority.
> 

cool, thanks. as long as ERROR ends up on disk i'm happy.

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:390:
DEVICE_LOG(device_event_log::LOG_TYPE_POWER, log_level)
please just use EVENT for both of these; we already log at USER when sending the
request

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:419: POWER_LOG(DEBUG) << "Received power
supply poll signal.";
On 2014/12/02 21:14:38, stevenjb wrote:
> On 2014/12/02 20:43:16, Daniel Erat wrote:
> > why did you use different levels for this and for
> > PeripheralBatteryStatusReceived? they were both VLOG(1) before, and i
believe
> > that they're both called in response to signals emitted periodically by
> powerd.
> 
> I guess it seemed more like a VLOG(2) to me since it's just a poll. I could
make
> both DEBUG or neither, whichever you prefer.

i think that these are sent every 30 seconds or so. these messages don't contain
much useful information (powerd logs more details when it's emitting the
signals), so i'm fine with DEBUG.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:52: LOG_LEVEL_EVENT = 2,
On 2014/12/02 21:14:38, stevenjb wrote:
> On 2014/12/02 20:43:16, Daniel Erat wrote:
> > please add comments describing how to use these levels. is USER for
> > user-triggered events while EVENT is for system-triggered ones?
> 
> I put comments above, I will refer to that here.

ah, thanks. i scanned a bit above but missed that comment.

[Ben Chan, 2014-12-02 21:42:53]

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/system_logs/device_event_log_source.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.cc:1: // Copyright
(c) 2014 The Chromium Authors. All rights reserved.
drop (c)

http://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/system_logs/device_event_log_source.h (right):

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.h:1: // Copyright
(c) 2014 The Chromium Authors. All rights reserved.
drop (c)

http://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.cc:1: // Copyright (c) 2014 The Chromium Authors. All
rights reserved.
drop (c)

http://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:1: // Copyright (c) 2014 The Chromium Authors. All
rights reserved.
drop (c)

http://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:33: #define DEVICE_LOG(type, level)                 
                 \
nit: perhaps default DEVICE_LOG first

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:43: LOG_TYPE_NON_NETWORK,  // Match all non NETWORK
log types
why is NETWORK log a special case?  do we expect to have NON_POWER in the
future?

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:67: // Adds an entry to the global instance if
initialized. Also always logs the
nit: perhaps describe what happens if the instance isn't initialized?

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:76: // |description| and calls AddEntry().
will this function be deprecated after the migration is done? if so, perhaps add
a TODO?

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log_impl.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl.h:1: // Copyright (c) 2014 The Chromium Authors.
All rights reserved.
drop (c)

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log_impl_unittest.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl_unittest.cc:1: // Copyright (c) 2014 The Chromium
Authors. All rights reserved.
drop (c)

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl_unittest.cc:33: virtual void SetUp() override {
drop "virtual" when you specify "override"

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance

" For clarity, use exactly one of override, final, or virtual when declaring an
override. "

this will fail cpplint check

[jennyz, 2014-12-02 22:16:08]

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/system_logs/device_event_log_source.h (right):

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.h:12: // Fetches
memory usage details.
Is the class comment still accurate? It looks like inherit from the old file.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.cc:28: if (g_device_event_log)
Is it normal to re-initialize g_device_event_log without Shutdown first?

https://codereview.chromium.org/773703002/diff/120001/chromeos/network/networ...
File chromeos/network/network_handler.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/network/networ...
chromeos/network/network_handler.cc:33: device_event_log::Initialize(0 /*
default max entries */);
It is a little weird to me that device_event_log is for all devices logging
purpose, but its initialization be hooked up to NetworkHandler.

[stevenjb, 2014-12-03 00:16:04]

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/system_logs/device_event_log_source.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.cc:1: // Copyright
(c) 2014 The Chromium Authors. All rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)
> 
> http://www.chromium.org/developers/coding-style#TOC-File-headers

Done. (This was copied from an existing older file).

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/system_logs/device_event_log_source.h (right):

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.h:1: // Copyright
(c) 2014 The Chromium Authors. All rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)
> 
> http://www.chromium.org/developers/coding-style#TOC-File-headers

Done.

https://codereview.chromium.org/773703002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/device_event_log_source.h:12: // Fetches
memory usage details.
On 2014/12/02 22:16:08, jennyz wrote:
> Is the class comment still accurate? It looks like inherit from the old file.

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:390:
DEVICE_LOG(device_event_log::LOG_TYPE_POWER, log_level)
On 2014/12/02 21:20:39, Daniel Erat wrote:
> please just use EVENT for both of these; we already log at USER when sending
the
> request
Looking at the results on my Pixel, this is a bit spammy, so I changed it to
DEBUG. Let me know if you'd prefer to change it to EVENT.

We weren't actually logging events for increase/decrease brightness requests so
I added them.

https://codereview.chromium.org/773703002/diff/120001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:419: POWER_LOG(DEBUG) << "Received power
supply poll signal.";
On 2014/12/02 21:20:39, Daniel Erat wrote:
> On 2014/12/02 21:14:38, stevenjb wrote:
> > On 2014/12/02 20:43:16, Daniel Erat wrote:
> > > why did you use different levels for this and for
> > > PeripheralBatteryStatusReceived? they were both VLOG(1) before, and i
> believe
> > > that they're both called in response to signals emitted periodically by
> > powerd.
> > 
> > I guess it seemed more like a VLOG(2) to me since it's just a poll. I could
> make
> > both DEBUG or neither, whichever you prefer.
> 
> i think that these are sent every 30 seconds or so. these messages don't
contain
> much useful information (powerd logs more details when it's emitting the
> signals), so i'm fine with DEBUG.

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.cc:1: // Copyright (c) 2014 The Chromium Authors. All
rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)
> 
> http://www.chromium.org/developers/coding-style#TOC-File-headers

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.cc:28: if (g_device_event_log)
On 2014/12/02 22:16:08, jennyz wrote:
> Is it normal to re-initialize g_device_event_log without Shutdown first? 

I don't know about "normal", but it is harmless. I suppose since we don't
currently need this functionality it might be clearer just to remove it and
CHECK instead since that is more common.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:1: // Copyright (c) 2014 The Chromium Authors. All
rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)
> 
> http://www.chromium.org/developers/coding-style#TOC-File-headers

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:33: #define DEVICE_LOG(type, level)                 
                 \
On 2014/12/02 21:42:53, Ben Chan wrote:
> nit: perhaps default DEVICE_LOG first

I put this last since usually one of the above will be used. I'll clarify that
with a comment.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:43: LOG_TYPE_NON_NETWORK,  // Match all non NETWORK
log types
On 2014/12/02 21:42:53, Ben Chan wrote:
> why is NETWORK log a special case?  do we expect to have NON_POWER in the
> future?

I'll add a comment. Currently there is much more network logging than anything
else and I expect that to continue for a while. If that changes we can consider
different approach.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:67: // Adds an entry to the global instance if
initialized. Also always logs the
On 2014/12/02 21:42:53, Ben Chan wrote:
> nit: perhaps describe what happens if the instance isn't initialized?

I thought I did. I will rephrase this.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.h:76: // |description| and calls AddEntry().
On 2014/12/02 21:42:53, Ben Chan wrote:
> will this function be deprecated after the migration is done? if so, perhaps
add
> a TODO?

Someday maybe? There are 352 instances of NET_LOG_EVENT, etc, I don't expect to
deprecate this any time soon.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log_impl.h (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl.h:1: // Copyright (c) 2014 The Chromium Authors.
All rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log_impl_unittest.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl_unittest.cc:1: // Copyright (c) 2014 The Chromium
Authors. All rights reserved.
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop (c)

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log_impl_unittest.cc:33: virtual void SetUp() override {
On 2014/12/02 21:42:53, Ben Chan wrote:
> drop "virtual" when you specify "override"
> 
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance
> 
> " For clarity, use exactly one of override, final, or virtual when declaring
an
> override. "
> 
> this will fail cpplint check

Done.

https://codereview.chromium.org/773703002/diff/120001/chromeos/network/networ...
File chromeos/network/network_handler.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/network/networ...
chromeos/network/network_handler.cc:33: device_event_log::Initialize(0 /*
default max entries */);
On 2014/12/02 22:16:08, jennyz wrote:
> It is a little weird to me that device_event_log is for all devices logging
> purpose, but its initialization be hooked up to NetworkHandler.

Good point. Moved to DBusThreadManager.

[Daniel Erat, 2014-12-03 15:07:21]

lgtm for power_manager_client.cc

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:126: POWER_LOG(USER) <<
"SetScreenBrightnessPercent: " << percent
this might be spammy enough that we don't want to log it. i think it's only
called now when you hit a brightness key to make the bubble appear and then
quickly drag the slider on the bubble before it disappears; might want to check
it.

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:599: POWER_LOG(EVENT) << "Got " <<
power_manager::kInputEventSignal << " signal:"
USER might make more sense here since this corresponds to user input

[jennyz, 2014-12-03 19:12:43]

lgtm

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
File chromeos/device_event_log.cc (right):

https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
chromeos/device_event_log.cc:28: if (g_device_event_log)
On 2014/12/03 00:16:03, stevenjb wrote:
> On 2014/12/02 22:16:08, jennyz wrote:
> > Is it normal to re-initialize g_device_event_log without Shutdown first? 
> 
> I don't know about "normal", but it is harmless. I suppose since we don't
> currently need this functionality it might be clearer just to remove it and
> CHECK instead since that is more common.

Acknowledged.

[Ben Chan, 2014-12-04 02:18:28]

On 2014/12/03 19:12:43, jennyz wrote:
> lgtm
> 
>
https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
> File chromeos/device_event_log.cc (right):
> 
>
https://codereview.chromium.org/773703002/diff/120001/chromeos/device_event_l...
> chromeos/device_event_log.cc:28: if (g_device_event_log)
> On 2014/12/03 00:16:03, stevenjb wrote:
> > On 2014/12/02 22:16:08, jennyz wrote:
> > > Is it normal to re-initialize g_device_event_log without Shutdown first? 
> > 
> > I don't know about "normal", but it is harmless. I suppose since we don't
> > currently need this functionality it might be clearer just to remove it and
> > CHECK instead since that is more common.
> 
> Acknowledged.

lgtm

[rkc, 2014-12-05 19:46:54]

lgtm

[stevenjb, 2014-12-05 20:14:06]

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:126: POWER_LOG(USER) <<
"SetScreenBrightnessPercent: " << percent
On 2014/12/03 15:07:20, Daniel Erat wrote:
> this might be spammy enough that we don't want to log it. i think it's only
> called now when you hit a brightness key to make the bubble appear and then
> quickly drag the slider on the bubble before it disappears; might want to
check
> it.

Yeah, this is spammy, I'll remove it. It turns out so are the above brightness
change requests, so I removed them also.

https://codereview.chromium.org/773703002/diff/130001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:599: POWER_LOG(EVENT) << "Got " <<
power_manager::kInputEventSignal << " signal:"
On 2014/12/03 15:07:20, Daniel Erat wrote:
> USER might make more sense here since this corresponds to user input

Done.

[stevenjb, 2014-12-05 20:14:11]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-12-05 20:15:35]

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

[commit-bot: I haz the power, 2014-12-05 21:35:48]

Committed patchset #4 (id:150001)

[commit-bot: I haz the power, 2014-12-05 21:36:38]

Patchset 4 (id:??) landed as
https://crrev.com/252993533a1b5cb7bcec6ab058f1d62144ca999e
Cr-Commit-Position: refs/heads/master@{#307086}