Fire notifications when reboot is requested, not scheduled

chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc

Index: chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc
diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc b/chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc
index ea96dd648c7617533ffbaa5b71cc9c453eac2aac..d41f5f84c46b4c8d7adeb74684ae4a3b51bef48a 100644
--- a/chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc
+++ b/chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc
@@ -44,6 +44,9 @@ KioskAppUpdateService::KioskAppUpdateService(
 
   if (KioskAppManager::Get())
     KioskAppManager::Get()->AddObserver(this);
+
+  if (automatic_reboot_manager_->reboot_requested())
+    OnRebootRequested(automatic_reboot_manager_->reboot_reason());

[xiyuan, 2014/11/17 18:42:52]

Think we should make KioskAppUpdateService::set_app_id a method and move this
code there so that we do this with the app running. Doing this in ctor might be
too early.

[bartfab (slow), 2014/11/17 19:58:24]

Done.

 }
 
 KioskAppUpdateService::~KioskAppUpdateService() {
@@ -93,7 +96,7 @@ void KioskAppUpdateService::OnAppUpdateAvailable(
   StartAppUpdateRestartTimer();
 }
 
-void KioskAppUpdateService::OnRebootScheduled(Reason reason) {
+void KioskAppUpdateService::OnRebootRequested(Reason reason) {
   extensions::core_api::runtime::OnRestartRequired::Reason restart_reason =
       extensions::core_api::runtime::OnRestartRequired::REASON_NONE;
   switch (reason) {
@@ -115,7 +118,6 @@ void KioskAppUpdateService::OnRebootScheduled(Reason reason) {
 }
 
 void KioskAppUpdateService::WillDestroyAutomaticRebootManager() {
-  automatic_reboot_manager_->RemoveObserver(this);

[xiyuan, 2014/11/17 18:42:52]

Why do we need to remove this?

[bartfab (slow), 2014/11/17 19:58:24]

We want to make sure that all observers are notified when the manager is about
to be destroyed so that they can reset their pointers to it.

In the original implementation, each observer was additionally required to
remove itself as an observer. The ObserverList in the manager would then verify
that it is empty during construction. This explicit removal does not really gain
us anything but carries the cost of an additional method call fore-and-back. I
changed it so that the ObserverList implicitly removes all observers on
destruction. They still get notified before of course, as they always did, so
that they can reset their pointers to the manager.

[xiyuan, 2014/11/17 20:08:03]

Thanks for the clarification. I am slightly prefer to do the empty check but
also okay with the proposed change.

[bartfab (slow), 2014/11/18 11:04:46]

I changed back to the empty check. I slightly prefer the other way, but you are
an owner, so your preference wins :).

   automatic_reboot_manager_ = NULL;
 }