Fire notifications when reboot is requested, not scheduled

chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc

Index: chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc
diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc b/chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc
index 970eeeb48c37a5ca48125cc766bd8576fa2060a8..6b8f2520bc370f5e388b8bb1df764c00d94a74ff 100644
--- a/chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc
+++ b/chrome/browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc
@@ -7,13 +7,20 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/prefs/pref_service.h"
+#include "base/run_loop.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/scoped_path_override.h"
 #include "base/threading/sequenced_worker_pool.h"
@@ -22,32 +29,47 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chromeos/system/automatic_reboot_manager.h"
+#include "chrome/browser/chromeos/system/automatic_reboot_manager_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_paths.h"
 #include "chromeos/dbus/update_engine_client.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/test/test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
-class KioskAppUpdateServiceTest : public extensions::PlatformAppBrowserTest {
+namespace {
+
+void RunCallback(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+                 const base::Closure& callback) {
+  task_runner->PostTask(FROM_HERE, callback);
+}
+
+} // namespace
+
+class KioskAppUpdateServiceTest
+    : public extensions::PlatformAppBrowserTest,
+      public system::AutomaticRebootManagerObserver {
  public:
-  KioskAppUpdateServiceTest() : app_(NULL), update_service_(NULL) {}
-  virtual ~KioskAppUpdateServiceTest() {}
+  KioskAppUpdateServiceTest()
+      : app_(NULL),
+        update_service_(NULL),
+        automatic_reboot_manager_(NULL) {}
+
+  ~KioskAppUpdateServiceTest() override {}
 
   // extensions::PlatformAppBrowserTest overrides:
-  virtual void SetUpOnMainThread() override {
-    extensions::PlatformAppBrowserTest::SetUpOnMainThread();
+  void SetUpInProcessBrowserTestFixture() override {
+    extensions::PlatformAppBrowserTest::SetUpInProcessBrowserTestFixture();
 
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     const base::FilePath& temp_dir = temp_dir_.path();
 
-    const base::TimeDelta uptime = base::TimeDelta::FromHours(1);
+    const base::TimeDelta uptime = base::TimeDelta::FromHours(3);
     const std::string uptime_seconds =
         base::DoubleToString(uptime.InSecondsF());
     const base::FilePath uptime_file = temp_dir.Append("uptime");
@@ -56,6 +78,10 @@ class KioskAppUpdateServiceTest : public extensions::PlatformAppBrowserTest {
                   uptime_file, uptime_seconds.c_str(), uptime_seconds.size()));
     uptime_file_override_.reset(
         new base::ScopedPathOverride(chromeos::FILE_UPTIME, uptime_file));
+  }
+
+  void SetUpOnMainThread() override {
+    extensions::PlatformAppBrowserTest::SetUpOnMainThread();
 
     app_ = LoadExtension(
         test_data_dir_.AppendASCII("api_test/runtime/on_restart_required"));
@@ -65,10 +91,44 @@ class KioskAppUpdateServiceTest : public extensions::PlatformAppBrowserTest {
     command->AppendSwitch(switches::kForceAppMode);
     command->AppendSwitchASCII(switches::kAppId, app_->id());
 
-    update_service_ = KioskAppUpdateServiceFactory::GetForProfile(profile());
-    update_service_->set_app_id(app_->id());
+    automatic_reboot_manager_ =
+        g_browser_process->platform_part()->automatic_reboot_manager();
+
+    // Wait for the |automatic_reboot_manager_| to finish initializing.
+    base::RunLoop run_loop;
+    base::SequencedWorkerPool* worker_pool =
+        content::BrowserThread::GetBlockingPool();
+    // Ensure that the initialization task the |automatic_reboot_manager_| posts
+    // to the blocking pool has finished by posting another task with the same
+    // |SequenceToken| and waiting for it to finish.
+    worker_pool->PostSequencedWorkerTask(
+        worker_pool->GetNamedSequenceToken("automatic-reboot-manager"),
+        FROM_HERE,
+        base::Bind(&RunCallback,
+                   base::MessageLoopProxy::current(),
+                   run_loop.QuitClosure()));
+    // Ensure that the |automatic_reboot_manager_| has had a chance to process
+    // the result of the task posted to the blocking pool.
+    base::RunLoop().RunUntilIdle();

[xiyuan, 2014/11/18 22:37:16]

Should we be doing "run_loop.Run()" so that it blocks until the posted quit
closure is called?

[bartfab (slow), 2014/11/19 09:21:08]

Oops, I was not spinning |run_loop| at all. My intention had been to use two
|RunLoop| instances: The first one is |run_loop|, which should wait until the
quit closure is called. A second instance should just flush any new tasks posted
by the UI loop to itself as a result. I put in the second but forgot the first
:).

+
+    automatic_reboot_manager_->AddObserver(this);
+  }
+
+  // system::AutomaticRebootManagerObserver:
+  void OnRebootRequested(
+      system::AutomaticRebootManagerObserver::Reason) override {
+    if (run_loop_)
+      run_loop_->Quit();
+  }
 
-    content::RunAllBlockingPoolTasksUntilIdle();
+  void WillDestroyAutomaticRebootManager() override {
+    automatic_reboot_manager_->RemoveObserver(this);
+  }
+
+  void CreateKioskAppUpdateService() {
+    EXPECT_FALSE(update_service_);
+    update_service_ = KioskAppUpdateServiceFactory::GetForProfile(profile());
+    update_service_->Init(app_->id());
   }
 
   void FireAppUpdateAvailable() {
@@ -78,8 +138,16 @@ class KioskAppUpdateServiceTest : public extensions::PlatformAppBrowserTest {
   void FireUpdatedNeedReboot() {
     UpdateEngineClient::Status status;
     status.status = UpdateEngineClient::UPDATE_STATUS_UPDATED_NEED_REBOOT;
-    g_browser_process->platform_part()->automatic_reboot_manager()->
-        UpdateStatusChanged(status);
+    run_loop_.reset(new base::RunLoop);
+    automatic_reboot_manager_->UpdateStatusChanged(status);
+    run_loop_->Run();
+  }
+
+  void RequestPeriodicReboot() {
+    run_loop_.reset(new base::RunLoop);
+    g_browser_process->local_state()->SetInteger(
+        prefs::kUptimeLimit, base::TimeDelta::FromHours(2).InSeconds());
+    run_loop_->Run();
   }
 
  private:
@@ -87,30 +155,63 @@ class KioskAppUpdateServiceTest : public extensions::PlatformAppBrowserTest {
   scoped_ptr<base::ScopedPathOverride> uptime_file_override_;
   const extensions::Extension* app_;  // Not owned.
   KioskAppUpdateService* update_service_;  // Not owned.
+  system::AutomaticRebootManager* automatic_reboot_manager_;  // Not owned.
+  scoped_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(KioskAppUpdateServiceTest);
 };
 
+// Verifies that the app is notified a reboot is required when an app update
+// becomes available.
 IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, AppUpdate) {
-  FireAppUpdateAvailable();
+  CreateKioskAppUpdateService();
 
   ExtensionTestMessageListener listener("app_update", false);
+  FireAppUpdateAvailable();
   listener.WaitUntilSatisfied();
 }
 
+// Verifies that the app is notified a reboot is required when an OS update is
+// applied while Chrome is running and the policy to reboot after update is
+// enabled.
 IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, OsUpdate) {
+  CreateKioskAppUpdateService();
+
+  g_browser_process->local_state()->SetBoolean(prefs::kRebootAfterUpdate, true);
+  ExtensionTestMessageListener listener("os_update", false);
+  FireUpdatedNeedReboot();
+  listener.WaitUntilSatisfied();
+}
+
+// Verifies that the app is notified a reboot is required when a periodic reboot
+// is requested while Chrome is running.
+IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, Periodic) {
+  CreateKioskAppUpdateService();
+
+  ExtensionTestMessageListener listener("periodic", false);
+  RequestPeriodicReboot();
+  listener.WaitUntilSatisfied();
+}
+
+// Verifies that the app is notified a reboot is required when an OS update was
+// applied before Chrome was started and the policy to reboot after update is
+// enabled.
+IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, StartAfterOsUpdate) {
   g_browser_process->local_state()->SetBoolean(prefs::kRebootAfterUpdate, true);
   FireUpdatedNeedReboot();
 
   ExtensionTestMessageListener listener("os_update", false);
+  CreateKioskAppUpdateService();
   listener.WaitUntilSatisfied();
 }
 
-IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, Periodic) {
-  g_browser_process->local_state()->SetInteger(
-      prefs::kUptimeLimit, base::TimeDelta::FromMinutes(30).InSeconds());
+// Verifies that the app is notified a reboot is required when a periodic reboot
+// was requested before Chrome was started.
+IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, StartAfterPeriodic) {
+  RequestPeriodicReboot();
 
   ExtensionTestMessageListener listener("periodic", false);
+  CreateKioskAppUpdateService();
   listener.WaitUntilSatisfied();
 }