Add device policy to disallow shutdown

chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc

Index: chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc b/chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7afa7e79b126a47f329122e3ef35b036ab801387
--- /dev/null
+++ b/chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer_unittest.cc
@@ -0,0 +1,148 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/chromeos/login/shutdown_policy_observer.h"
+
+#include "base/run_loop.h"
+#include "base/values.h"
+#include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/settings/cros_settings_names.h"
+#include "chromeos/settings/cros_settings_provider.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+class ShutdownPolicyObserverTest : public testing::Test,
+                                   public ShutdownPolicyObserver::Delegate {
+ public:
+  void ExpectationCheckerCallback(bool actual_shutdown_is_reboot) {

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

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().

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

Done.

+    EXPECT_EQ(expected_shutdown_is_reboot_, actual_shutdown_is_reboot);
+    callback_called_ = true;
+  }
+
+  bool WasCallbackCalled() const {
+    return callback_called_;
+  }
+
+ protected:
+  ShutdownPolicyObserverTest()
+      : cros_settings_(nullptr),
+        stub_settings_provider_(new StubCrosSettingsProvider),

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

Nit: Ownership should be held by a scoped_ptr until it is passed to CroSettings.

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

Done.

+        callback_called_(false),
+        expected_shutdown_is_reboot_(false),
+        shutdown_is_reboot_(false),
+        count_delegate_invocations_(0) {}
+
+  virtual void SetUp() override {

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

Nit: Add //testing::Test:

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

Done.

+    cros_settings_ = CrosSettings::Get();
+
+    // Replace the real DeviceSettingsProvider with a stub.
+    device_settings_provider_.reset(

[bartfab (slow), 2014/12/08 16:49:45]

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.

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

Done.

+        cros_settings_->GetProvider(chromeos::kShutdownIsReboot));

[bartfab (slow), 2014/12/08 16:49:45]

Nit: No need for chromeos::.

[cschuet (SLOW), 2014/12/08 18:57:04]

Done.

+    EXPECT_TRUE(device_settings_provider_);
+    EXPECT_TRUE(cros_settings_->RemoveSettingsProvider(
+        device_settings_provider_.get()));
+    cros_settings_->AddSettingsProvider(stub_settings_provider_);
+
+    chromeos::DBusThreadManager::Initialize();

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

Nit: No need for chromeos::.

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

Done.

+  }
+
+  virtual void TearDown() override {
+    // Restore the real DeviceSettingsProvider.
+    EXPECT_TRUE(
+        cros_settings_->RemoveSettingsProvider(stub_settings_provider_));
+    cros_settings_->AddSettingsProvider(device_settings_provider_.release());
+
+    // Shut down the DeviceSettingsService.
+    DeviceSettingsService::Get()->UnsetSessionManager();
+    TestingBrowserProcess::GetGlobal()->SetProfileManager(nullptr);
+    base::RunLoop().RunUntilIdle();
+    chromeos::DBusThreadManager::Shutdown();

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

Nit: No need for chromeos::.

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

Done.

+  }
+
+  void SetDeviceSettings(bool shutdown_is_reboot) {

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

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().

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

Done.

+    const base::FundamentalValue shutdown_is_reboot_value(shutdown_is_reboot);
+    stub_settings_provider_->Set(kShutdownIsReboot, shutdown_is_reboot_value);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void SetExpectation(bool expected_shutdown_is_reboot) {

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

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().

[cschuet (SLOW), 2014/12/08 18:57:04]

Done.

+    expected_shutdown_is_reboot_ = expected_shutdown_is_reboot;
+    callback_called_ = false;
+  }
+
+  void OnShutdownPolicyChanged(bool shutdown_is_reboot) override {

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

Nit: Add //ShutdownPolicyObserver::Delegate:

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

Done.

+    shutdown_is_reboot_ = shutdown_is_reboot;
+    count_delegate_invocations_++;
+  }
+
+ protected:
+  content::TestBrowserThreadBundle thread_bundle_;
+
+  CrosSettings* cros_settings_;
+  scoped_ptr<CrosSettingsProvider> device_settings_provider_;

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

Nit: #include "base/memory/scoped_ptr.h"

[cschuet (SLOW), 2014/12/08 18:57:04]

Done.

+  StubCrosSettingsProvider* stub_settings_provider_;
+
+  ScopedTestDeviceSettingsService test_device_settings_service_;
+  ScopedTestCrosSettings test_cros_settings_;
+
+  bool callback_called_;
+  bool expected_shutdown_is_reboot_;
+  bool shutdown_is_reboot_;
+  int count_delegate_invocations_;

[bartfab (slow), 2014/12/08 16:49:45]

Nit: The word oder does not match Chrome style.
s/count_delegate_invocations/delegate_invocations_count/

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

Done.

+};
+
+TEST_F(ShutdownPolicyObserverTest, RetrieveTrustedDevicePolicies) {
+  ShutdownPolicyObserver shutdown_policy_observer(cros_settings_, this);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0, count_delegate_invocations_);
+
+  SetDeviceSettings(true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, count_delegate_invocations_);
+  EXPECT_TRUE(shutdown_is_reboot_);
+
+  SetDeviceSettings(false);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2, count_delegate_invocations_);
+  EXPECT_FALSE(shutdown_is_reboot_);
+
+  shutdown_policy_observer.Shutdown();
+
+  SetDeviceSettings(true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2, count_delegate_invocations_);
+  EXPECT_FALSE(shutdown_is_reboot_);
+}
+
+TEST_F(ShutdownPolicyObserverTest, CheckIfShutdownIsRebootTest) {

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

Nit: No need for |Test| at the end of a test case name.

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

Done.

+  ShutdownPolicyObserver shutdown_policy_observer(cros_settings_, this);
+  base::RunLoop().RunUntilIdle();
+
+  // Allow shutdown.
+  SetDeviceSettings(true);
+  SetExpectation(true);
+  EXPECT_FALSE(this->WasCallbackCalled());
+  shutdown_policy_observer.CheckIfShutdownIsReboot(
+      base::Bind(&ShutdownPolicyObserverTest::ExpectationCheckerCallback,

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

Nit: #include "base/bind.h"

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

Done.

+                 base::Unretained(this)));

[bartfab (slow), 2014/12/08 16:49:45]

Nit: #include "base/bind_helpers.h"

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

Done.

+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(this->WasCallbackCalled());
+
+  // Forbid shutdown.
+  SetDeviceSettings(false);
+  SetExpectation(false);
+  EXPECT_FALSE(this->WasCallbackCalled());
+  shutdown_policy_observer.CheckIfShutdownIsReboot(
+      base::Bind(&ShutdownPolicyObserverTest::ExpectationCheckerCallback,
+                 base::Unretained(this)));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(this->WasCallbackCalled());
+}
+
+}  // namespace chromeos