Add device policy to disallow shutdown

Add device policy to disallow shutdown

This CL introduces a new boolean device policy kDeviceShutdownIsReboot.

BUG=312541
TESTS=unit_tests

Committed: https://crrev.com/581f2e14651eda89ef76009dab4f7d8d688766f5
Cr-Commit-Position: refs/heads/master@{#308624}

[cschuet (SLOW), 2014-12-05 17:03:55]

cschuet@chromium.org changed reviewers:
+ asvitkine@chromium.org, bartfab@chromium.org, jamescook@chromium.org,
stevenjb@chromium.org, thakis@chromium.org

[cschuet (SLOW), 2014-12-05 17:03:56]

asvitkine@chromium.org: Please review changes in 
tools/metrics/*

bartfab@chromium.org: Please review changes in 
chrome/browser/chromeos/policy/*
components/policy/*

thakis@chromium.org: Please review changes in 
chrome/*
w/o chrome/browser/chromeos/policy/*

stevenjb@chromium.org: Please review changes in 
chromeos/*

jamescook@chromium.org: Please review changes in 
ash/test/*
ash/metrics/*

[James Cook, 2014-12-05 18:50:07]

jamescook@chromium.org changed reviewers:
+ derat@chromium.org

[James Cook, 2014-12-05 18:50:08]

Dan, can you look at ash/* for the shutdown vs. restart changes?  The code looks
OK to me but I'm not particularly familiar with that system.

[Daniel Erat, 2014-12-05 19:06:12]

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
File ash/metrics/user_metrics_recorder.cc (right):

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
ash/metrics/user_metrics_recorder.cc:66: case
ash::UMA_ACCEL_RESTART_POWER_BUTTON:
alphabetize

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
File ash/metrics/user_metrics_recorder.h (right):

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
ash/metrics/user_metrics_recorder.h:113: // Reboot action triggered by the user
please keep alphabetical lists alphabetized: move this up to the other
UMA_ACCEL_ enums and remove the comment

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
chrome/browser/chromeos/settings/device_settings_provider.cc:42:
kAccountsPrefAllowGuest,
did clang-format suggest this change?

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
chrome/browser/chromeos/settings/device_settings_provider.cc:125:
policy.shutdown_allowed().shutdown_allowed());
this indenting is weird; please follow the style that's used in the next block:

new_values_cache->SetBoolean(
    kShutdownAllowed,
    !policy.has_shutdown_allowed() ||
    policy.shutdown_allowed().shutdown_allowed());

[stevenjb, 2014-12-05 20:30:48]

chromeos/ lgtm w/ comment

https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
chromeos/settings/cros_settings_names.cc:166: const char kShutdownAllowed[] =
"cros.device.shutdown_allowed";
nit: "ShutdownAllowed" implies that we disable shutdown entirely. I think that
"ShutdownIsReboot" would be more clear.

[cschuet (SLOW), 2014-12-08 14:37:44]

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
File ash/metrics/user_metrics_recorder.cc (right):

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
ash/metrics/user_metrics_recorder.cc:66: case
ash::UMA_ACCEL_RESTART_POWER_BUTTON:
On 2014/12/05 19:06:11, Daniel Erat wrote:
> alphabetize

Done.

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
chrome/browser/chromeos/settings/device_settings_provider.cc:125:
policy.shutdown_allowed().shutdown_allowed());
On 2014/12/05 19:06:11, Daniel Erat wrote:
> this indenting is weird; please follow the style that's used in the next
block:
> 
> new_values_cache->SetBoolean(
>     kShutdownAllowed,
>     !policy.has_shutdown_allowed() ||
>     policy.shutdown_allowed().shutdown_allowed());

Done.

https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
chromeos/settings/cros_settings_names.cc:166: const char kShutdownAllowed[] =
"cros.device.shutdown_allowed";
On 2014/12/05 20:30:48, stevenjb wrote:
> nit: "ShutdownAllowed" implies that we disable shutdown entirely. I think that
> "ShutdownIsReboot" would be more clear.

Acknowledged.

[Alexei Svitkine (slow), 2014-12-08 15:49:34]

metrics lgtm

[cschuet (SLOW), 2014-12-08 15:59:09]

On 2014/12/05 20:30:48, stevenjb wrote:
> chromeos/ lgtm w/ comment
> 
>
https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
> File chromeos/settings/cros_settings_names.cc (right):
> 
>
https://codereview.chromium.org/776093004/diff/1/chromeos/settings/cros_setti...
> chromeos/settings/cros_settings_names.cc:166: const char kShutdownAllowed[] =
> "cros.device.shutdown_allowed";
> nit: "ShutdownAllowed" implies that we disable shutdown entirely. I think that
> "ShutdownIsReboot" would be more clear.

Thanks for the review. I have renamed the setting according to your suggestions.

[bartfab (slow), 2014-12-08 16:49:46]

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
File ash/wm/lock_state_controller.cc (left):

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
ash/wm/lock_state_controller.cc:298: VLOG(1) << "OnRealShutdownTimeout";
Why did you remove this?

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
File ash/wm/lock_state_controller.cc (right):

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
ash/wm/lock_state_controller.cc:316: delegate_->RequestShutdown();
Nit: If you return here, you can save yourself the else {}.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:587: //
Determines whether the device may be shut down. If this flag is set to true
Nit: s/true/true,/

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:589: // power
off. This policy controls shut down via the UI only.
Nit: s/shut down/shutdown/

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:8: #include
"base/bind_helpers.h"
Nit: Not used.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:17: delegate_
= delegate;
Nit: Move this to the initialization one line above:

delegate_(delegate)

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:24:
shutdown_policy_subscription_.reset();
This is unnecessary. Since we are in the constructor, nobody can have created a
subscription yet.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:51: void
ShutdownPolicyObserver::CheckIfShutdownIsReboot(
You can extract the logic shared by OnShutdownPolicyChanged() and
CheckIfShutdownIsReboot() into a helper method if you make the action to be
taken always be a callback (either a |ShutdownIsRebootCallback| passed in by the
user or a base::Bind() that informs he delegate).

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:63:
callback.Run(shutdown_is_reboot);
Nit: #include "base/callback.h"

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:20: using
ShutdownIsRebootCallback = base::Callback<void(bool)>;
Nit 1: #include "base/callback_forward.h"
Nit 2: Document what this callback is for.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:25: virtual
void OnShutdownPolicyChanged(bool shutdown_allowed) = 0;
s/shutdown_allowed/shutdown_is_reboot/

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:26: protected:
Nit: Add a blank line above.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:33: void
CheckIfShutdownIsReboot(const ShutdownIsRebootCallback& callback);
Nit: Add a comment that explains what this method does.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:37: private:
Niz: Add a blank line above.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:23:
void ExpectationCheckerCallback(bool actual_shutdown_is_reboot) {
This is not ideal for the following reason: If an expectation fails, gtest will
print the line number in which it failed. In this case, line 24 will be printed.
But you use the |ExpectationCheckerCallback| in the same test. If one of the two
instances fails, how do you know which one? It is better to just store the
actual value and then check it in the test body, like you do with
WasCallbackCalled().

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:35:
stub_settings_provider_(new StubCrosSettingsProvider),
Nit: Ownership should be held by a scoped_ptr until it is passed to CroSettings.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:41:
virtual void SetUp() override {
Nit: Add //testing::Test:

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:45:
device_settings_provider_.reset(
1: How about setting the |switches::kStubCrosSettings| command-line flag so that
CrosSettings starts out with a stub provider?
2: Nit: Until |RemoveSettingsProvider| is called and succeeds, you now have
double ownership of the provider, which can lead to double free.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:46:
cros_settings_->GetProvider(chromeos::kShutdownIsReboot));
Nit: No need for chromeos::.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:52:
chromeos::DBusThreadManager::Initialize();
Nit: No need for chromeos::.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:65:
chromeos::DBusThreadManager::Shutdown();
Nit: No need for chromeos::.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:68:
void SetDeviceSettings(bool shutdown_is_reboot) {
Nit: When this method is called, e.g. as SetDeviceSettings(true), it is not
clear what this |true| means. If you rename this method, it will be
self-explanatory, e.g. SetShutdownIsReboot().

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:74:
void SetExpectation(bool expected_shutdown_is_reboot) {
Nit: When this method is called, e.g. as SetExpectation(true), it is not clear
what this |true| means. If you rename this method, it will be self-explanatory,
e.g. ExpectShutdownIsRebootValue().

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:79:
void OnShutdownPolicyChanged(bool shutdown_is_reboot) override {
Nit: Add //ShutdownPolicyObserver::Delegate:

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:88:
scoped_ptr<CrosSettingsProvider> device_settings_provider_;
Nit: #include "base/memory/scoped_ptr.h"

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:97:
int count_delegate_invocations_;
Nit: The word oder does not match Chrome style.
s/count_delegate_invocations/delegate_invocations_count/

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:123:
TEST_F(ShutdownPolicyObserverTest, CheckIfShutdownIsRebootTest) {
Nit: No need for |Test| at the end of a test case name.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:132:
base::Bind(&ShutdownPolicyObserverTest::ExpectationCheckerCallback,
Nit: #include "base/bind.h"

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:133:
base::Unretained(this)));
Nit: #include "base/bind_helpers.h"

[Nico, 2014-12-08 17:40:48]

Please get a chrome/browser/ui/webui/chromeos/login/OWNERS to look at the
webui/chromeos/login code. other than that, chrome/ lgtm once bartfab is happy

[cschuet (SLOW), 2014-12-08 18:57:05]

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
File ash/wm/lock_state_controller.cc (left):

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
ash/wm/lock_state_controller.cc:298: VLOG(1) << "OnRealShutdownTimeout";
On 2014/12/08 16:49:44, bartfab wrote:
> Why did you remove this?

Removed by accident. Restored.

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
File ash/wm/lock_state_controller.cc (right):

https://codereview.chromium.org/776093004/diff/40001/ash/wm/lock_state_contro...
ash/wm/lock_state_controller.cc:316: delegate_->RequestShutdown();
On 2014/12/08 16:49:44, bartfab wrote:
> Nit: If you return here, you can save yourself the else {}.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:587: //
Determines whether the device may be shut down. If this flag is set to true
On 2014/12/08 16:49:44, bartfab wrote:
> Nit: s/true/true,/

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:589: // power
off. This policy controls shut down via the UI only.
On 2014/12/08 16:49:44, bartfab wrote:
> Nit: s/shut down/shutdown/

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:8: #include
"base/bind_helpers.h"
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: Not used.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:17: delegate_
= delegate;
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: Move this to the initialization one line above:
> 
> delegate_(delegate)

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:24:
shutdown_policy_subscription_.reset();
On 2014/12/08 16:49:45, bartfab wrote:
> This is unnecessary. Since we are in the constructor, nobody can have created
a
> subscription yet.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:51: void
ShutdownPolicyObserver::CheckIfShutdownIsReboot(
On 2014/12/08 16:49:45, bartfab wrote:
> You can extract the logic shared by OnShutdownPolicyChanged() and
> CheckIfShutdownIsReboot() into a helper method if you make the action to be
> taken always be a callback (either a |ShutdownIsRebootCallback| passed in by
the
> user or a base::Bind() that informs he delegate).

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.cc:63:
callback.Run(shutdown_is_reboot);
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: #include "base/callback.h"

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h (right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:20: using
ShutdownIsRebootCallback = base::Callback<void(bool)>;
On 2014/12/08 16:49:45, bartfab wrote:
> Nit 1: #include "base/callback_forward.h"
> Nit 2: Document what this callback is for.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:25: virtual
void OnShutdownPolicyChanged(bool shutdown_allowed) = 0;
On 2014/12/08 16:49:45, bartfab wrote:
> s/shutdown_allowed/shutdown_is_reboot/

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:26: protected:
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: Add a blank line above.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:33: void
CheckIfShutdownIsReboot(const ShutdownIsRebootCallback& callback);
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: Add a comment that explains what this method does.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:37: private:
On 2014/12/08 16:49:45, bartfab wrote:
> Niz: Add a blank line above.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:23:
void ExpectationCheckerCallback(bool actual_shutdown_is_reboot) {
On 2014/12/08 16:49:46, bartfab wrote:
> This is not ideal for the following reason: If an expectation fails, gtest
will
> print the line number in which it failed. In this case, line 24 will be
printed.
> But you use the |ExpectationCheckerCallback| in the same test. If one of the
two
> instances fails, how do you know which one? It is better to just store the
> actual value and then check it in the test body, like you do with
> WasCallbackCalled().

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:35:
stub_settings_provider_(new StubCrosSettingsProvider),
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: Ownership should be held by a scoped_ptr until it is passed to
CroSettings.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:41:
virtual void SetUp() override {
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: Add //testing::Test:

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:45:
device_settings_provider_.reset(
On 2014/12/08 16:49:45, bartfab wrote:
> 1: How about setting the |switches::kStubCrosSettings| command-line flag so
that
> CrosSettings starts out with a stub provider?
> 2: Nit: Until |RemoveSettingsProvider| is called and succeeds, you now have
> double ownership of the provider, which can lead to double free.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:46:
cros_settings_->GetProvider(chromeos::kShutdownIsReboot));
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: No need for chromeos::.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:52:
chromeos::DBusThreadManager::Initialize();
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: No need for chromeos::.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:65:
chromeos::DBusThreadManager::Shutdown();
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: No need for chromeos::.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:68:
void SetDeviceSettings(bool shutdown_is_reboot) {
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: When this method is called, e.g. as SetDeviceSettings(true), it is not
> clear what this |true| means. If you rename this method, it will be
> self-explanatory, e.g. SetShutdownIsReboot().

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:74:
void SetExpectation(bool expected_shutdown_is_reboot) {
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: When this method is called, e.g. as SetExpectation(true), it is not clear
> what this |true| means. If you rename this method, it will be
self-explanatory,
> e.g. ExpectShutdownIsRebootValue().

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:79:
void OnShutdownPolicyChanged(bool shutdown_is_reboot) override {
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: Add //ShutdownPolicyObserver::Delegate:

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:88:
scoped_ptr<CrosSettingsProvider> device_settings_provider_;
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: #include "base/memory/scoped_ptr.h"

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:97:
int count_delegate_invocations_;
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: The word oder does not match Chrome style.
> s/count_delegate_invocations/delegate_invocations_count/

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:123:
TEST_F(ShutdownPolicyObserverTest, CheckIfShutdownIsRebootTest) {
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: No need for |Test| at the end of a test case name.

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:132:
base::Bind(&ShutdownPolicyObserverTest::ExpectationCheckerCallback,
On 2014/12/08 16:49:46, bartfab wrote:
> Nit: #include "base/bind.h"

Done.

https://codereview.chromium.org/776093004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:133:
base::Unretained(this)));
On 2014/12/08 16:49:45, bartfab wrote:
> Nit: #include "base/bind_helpers.h"

Done.

[cschuet (SLOW), 2014-12-08 19:00:54]

cschuet@chromium.org changed reviewers:
+ dpolukhin@chromium.org

[cschuet (SLOW), 2014-12-08 19:06:26]

On 2014/12/08 17:40:48, Nico wrote:
> Please get a chrome/browser/ui/webui/chromeos/login/OWNERS to look at the
> webui/chromeos/login code. other than that, chrome/ lgtm once bartfab is happy

Thanks for reviewing! I have added Dmitry Polukhin to review the changes in

chrome/browser/ui/webui/chromeos/login/*

as you suggested.

[Dmitry Polukhin, 2014-12-09 08:33:31]

LGTM chrome/browser/ui/webui/chromeos/login/*

[cschuet (SLOW), 2014-12-09 08:50:11]

I am sorry Dan, it seems I overlook one of your comments. Now done.

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
File ash/metrics/user_metrics_recorder.h (right):

https://codereview.chromium.org/776093004/diff/1/ash/metrics/user_metrics_rec...
ash/metrics/user_metrics_recorder.h:113: // Reboot action triggered by the user
On 2014/12/05 19:06:11, Daniel Erat wrote:
> please keep alphabetical lists alphabetized: move this up to the other
> UMA_ACCEL_ enums and remove the comment

Done.

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
chrome/browser/chromeos/settings/device_settings_provider.cc:42:
kAccountsPrefAllowGuest,
On 2014/12/05 19:06:11, Daniel Erat wrote:
> did clang-format suggest this change?

Oh, yes I didn't even pay attention to this. clang-format introduced this
formatting change. Do you want me to revert it?

[Daniel Erat, 2014-12-09 16:00:36]

lgtm

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
File chrome/browser/chromeos/settings/device_settings_provider.cc (right):

https://codereview.chromium.org/776093004/diff/1/chrome/browser/chromeos/sett...
chrome/browser/chromeos/settings/device_settings_provider.cc:42:
kAccountsPrefAllowGuest,
On 2014/12/09 08:50:11, cschuet wrote:
> On 2014/12/05 19:06:11, Daniel Erat wrote:
> > did clang-format suggest this change?
> 
> Oh, yes I didn't even pay attention to this. clang-format introduced this
> formatting change. Do you want me to revert it?

it's okay with me if you leave it at four. i think that chromium has
historically used two, but i see both styles when i grep -A 2 for '\[\] = {$',
so i'm fine with just going to whatever clang-format says.

[bartfab (slow), 2014-12-15 18:01:57]

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:587: //
Determines whether the device may be shut down. If this flag is set to
Nit: Now that the policy was renamed, I think this comment should be reworded to
something like:

Determines whether the device automatically reboots whenever the user shuts it
down. This policy affects shutdowns triggered from the UI only. If the user
shuts down the device using the power button, it will not automatically reboot,
even if the policy is enabled.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h (right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:16: // This
class observes the device setting "DeviceRebootOnShutdown". Changes to
We use |pipes|, not "quotes," to denote literals.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:22: // the
current state of the "DeviceRebootOnShutdown" policy once a trusted of
Nit 1: We use |pipes|, not "quotes," to denote literals.
Nit 2: s/trusted of/trusted set of/

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:26: // This
delegate is called when the "DeviceRebootOnShutdown" policy changes.
We use |pipes|, not "quotes," to denote literals.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:39: //
|callback| with the trusted state of the "DeviceRebootOnShutdown" policy.
We use |pipes|, not "quotes," to denote literals.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:14:
#include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
Nit: No longer used.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:28:
void ResultCallback(bool actual_reboot_on_shutdown) {
Nit: You can drop the |actual_| prefix now.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:41:
expected_reboot_on_shutdown_(false),
Nit: This member is no longer needed as you set and verify the expectation from
the test body now.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:53:
DBusThreadManager::Initialize();
Nit: Could you check whether this is still necessary?

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:58:
DeviceSettingsService::Get()->UnsetSessionManager();
Nit: Could you check whether this is still necessary?

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:59:
TestingBrowserProcess::GetGlobal()->SetProfileManager(nullptr);
Nit: Could you check whether this is still necessary?

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:60:
base::RunLoop().RunUntilIdle();
Nit: Could you check whether this is still necessary?

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:61:
DBusThreadManager::Shutdown();
Nit: Could you check whether this is still necessary?

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:62:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
You should remove, not re-add, the switch in TearDown().

https://codereview.chromium.org/776093004/diff/80001/chromeos/settings/cros_s...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/776093004/diff/80001/chromeos/settings/cros_s...
chromeos/settings/cros_settings_names.cc:163: // A boolean pref that indicates
whether the user is allowed to shut down the
Nit: Reword this comment to talk about reboots rather than shutdowns. Otherwise,
we have inverse logic that is hard to follow: "indicates whether the user is
allowed to do X... if |true|, Y is forbidden."

https://codereview.chromium.org/776093004/diff/80001/components/policy/resour...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/776093004/diff/80001/components/policy/resour...
components/policy/resources/policy_templates.json:7036: 'caption': '''Forbid
device shutdown''',
Nit: As with the other comments/documentation: Reword in terms of an automatic
reboot instead of a forbidden shutdown.

[cschuet (SLOW), 2014-12-16 10:02:08]

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/policy/proto/chrome_device_policy.proto (right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/policy/proto/chrome_device_policy.proto:587: //
Determines whether the device may be shut down. If this flag is set to
On 2014/12/15 18:01:56, bartfab wrote:
> Nit: Now that the policy was renamed, I think this comment should be reworded
to
> something like:
> 
> Determines whether the device automatically reboots whenever the user shuts it
> down. This policy affects shutdowns triggered from the UI only. If the user
> shuts down the device using the power button, it will not automatically
reboot,
> even if the policy is enabled.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h (right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:16: // This
class observes the device setting "DeviceRebootOnShutdown". Changes to
On 2014/12/15 18:01:56, bartfab wrote:
> We use |pipes|, not "quotes," to denote literals.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:22: // the
current state of the "DeviceRebootOnShutdown" policy once a trusted of
On 2014/12/15 18:01:56, bartfab wrote:
> Nit 1: We use |pipes|, not "quotes," to denote literals.
> Nit 2: s/trusted of/trusted set of/

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:26: // This
delegate is called when the "DeviceRebootOnShutdown" policy changes.
On 2014/12/15 18:01:56, bartfab wrote:
> We use |pipes|, not "quotes," to denote literals.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h:39: //
|callback| with the trusted state of the "DeviceRebootOnShutdown" policy.
On 2014/12/15 18:01:56, bartfab wrote:
> We use |pipes|, not "quotes," to denote literals.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:14:
#include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
On 2014/12/15 18:01:56, bartfab wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:28:
void ResultCallback(bool actual_reboot_on_shutdown) {
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: You can drop the |actual_| prefix now.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:41:
expected_reboot_on_shutdown_(false),
On 2014/12/15 18:01:56, bartfab wrote:
> Nit: This member is no longer needed as you set and verify the expectation
from
> the test body now.

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:53:
DBusThreadManager::Initialize();
On 2014/12/15 18:01:56, bartfab wrote:
> Nit: Could you check whether this is still necessary?

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:58:
DeviceSettingsService::Get()->UnsetSessionManager();
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: Could you check whether this is still necessary?

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:59:
TestingBrowserProcess::GetGlobal()->SetProfileManager(nullptr);
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: Could you check whether this is still necessary?

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:60:
base::RunLoop().RunUntilIdle();
On 2014/12/15 18:01:56, bartfab wrote:
> Nit: Could you check whether this is still necessary?

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:61:
DBusThreadManager::Shutdown();
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: Could you check whether this is still necessary?

Done.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:62:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
On 2014/12/15 18:01:56, bartfab wrote:
> You should remove, not re-add, the switch in TearDown().

Done.

https://codereview.chromium.org/776093004/diff/80001/chromeos/settings/cros_s...
File chromeos/settings/cros_settings_names.cc (right):

https://codereview.chromium.org/776093004/diff/80001/chromeos/settings/cros_s...
chromeos/settings/cros_settings_names.cc:163: // A boolean pref that indicates
whether the user is allowed to shut down the
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: Reword this comment to talk about reboots rather than shutdowns.
Otherwise,
> we have inverse logic that is hard to follow: "indicates whether the user is
> allowed to do X... if |true|, Y is forbidden."

Done.

https://codereview.chromium.org/776093004/diff/80001/components/policy/resour...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/776093004/diff/80001/components/policy/resour...
components/policy/resources/policy_templates.json:7036: 'caption': '''Forbid
device shutdown''',
On 2014/12/15 18:01:57, bartfab wrote:
> Nit: As with the other comments/documentation: Reword in terms of an automatic
> reboot instead of a forbidden shutdown.

Done.

[bartfab (slow), 2014-12-16 10:24:42]

LGTM with a couple of final nits.

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:28:
void ResultCallback(bool actual_reboot_on_shutdown) {
On 2014/12/16 10:02:08, cschuet wrote:
> On 2014/12/15 18:01:57, bartfab wrote:
> > Nit: You can drop the |actual_| prefix now.
> 
> Done.

No, not done.

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:14:
#include "chrome/test/base/testing_browser_process.h"
Nit: No longer used.

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:115:
callback_called_ = false;
Optional nit: You write |callback_called_| directly but read it via a method.
You could make this more consistent by either having methods or direct accesses
for both.

https://codereview.chromium.org/776093004/diff/100001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/776093004/diff/100001/components/policy/resou...
components/policy/resources/policy_templates.json:7038: If this policy is set to
true, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph> will trigger a
reboot when the user shuts down the device. <ph
name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph> replaces all occurrences
of shutdown buttons in the UI by reboot buttons.''',
Nit: Could you add a note that this policy does not apply to the hardware power
button?

[cschuet (SLOW), 2014-12-16 10:53:00]

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:28:
void ResultCallback(bool actual_reboot_on_shutdown) {
On 2014/12/16 10:24:42, bartfab wrote:
> On 2014/12/16 10:02:08, cschuet wrote:
> > On 2014/12/15 18:01:57, bartfab wrote:
> > > Nit: You can drop the |actual_| prefix now.
> > 
> > Done.
> 
> No, not done.

Done.

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
(right):

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:14:
#include "chrome/test/base/testing_browser_process.h"
On 2014/12/16 10:24:42, bartfab wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/776093004/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc:115:
callback_called_ = false;
On 2014/12/16 10:24:42, bartfab wrote:
> Optional nit: You write |callback_called_| directly but read it via a method.
> You could make this more consistent by either having methods or direct
accesses
> for both.

Done.

https://codereview.chromium.org/776093004/diff/100001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/776093004/diff/100001/components/policy/resou...
components/policy/resources/policy_templates.json:7038: If this policy is set to
true, <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph> will trigger a
reboot when the user shuts down the device. <ph
name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph> replaces all occurrences
of shutdown buttons in the UI by reboot buttons.''',
On 2014/12/16 10:24:42, bartfab wrote:
> Nit: Could you add a note that this policy does not apply to the hardware
power
> button?

Done.

[bartfab (slow), 2014-12-16 11:03:02]

Still LGTM.

[cschuet (SLOW), 2014-12-16 17:38:29]

The CQ bit was checked by cschuet@chromium.org

[cschuet (SLOW), 2014-12-16 17:38:31]

The CQ bit was unchecked by cschuet@chromium.org

[cschuet (SLOW), 2014-12-16 17:38:35]

The CQ bit was checked by cschuet@chromium.org

[commit-bot: I haz the power, 2014-12-16 17:38:53]

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

[cschuet (SLOW), 2014-12-16 17:41:48]

The CQ bit was unchecked by cschuet@chromium.org

[cschuet (SLOW), 2014-12-16 18:04:31]

The CQ bit was checked by cschuet@chromium.org

[commit-bot: I haz the power, 2014-12-16 18:04:55]

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

[commit-bot: I haz the power, 2014-12-16 19:04:11]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-12-16 19:04:55]

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