Fire notifications when reboot is requested, not scheduled

chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc

 // Copyright 2013 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/chromeos/app_mode/kiosk_app_update_service.h"
 
 #include "base/logging.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
@@ skipping 26 matching lines @@
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile_)->extension_service();
   if (service)
     service->AddUpdateObserver(this);
 
   if (automatic_reboot_manager_)
     automatic_reboot_manager_->AddObserver(this);
 
   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() {
 }
 
 void KioskAppUpdateService::StartAppUpdateRestartTimer() {
   if (restart_timer_.IsRunning())
     return;
 
   // Setup timer to force restart once the wait period expires.
@@ skipping 29 matching lines @@
       app_id_, profile_, extension);
 
   extensions::RuntimeEventRouter::DispatchOnRestartRequiredEvent(
       profile_,
       app_id_,
       extensions::core_api::runtime::OnRestartRequired::REASON_APP_UPDATE);
 
   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) {
     case REBOOT_REASON_OS_UPDATE:
       restart_reason =
           extensions::core_api::runtime::OnRestartRequired::REASON_OS_UPDATE;
       break;
     case REBOOT_REASON_PERIODIC:
       restart_reason =
           extensions::core_api::runtime::OnRestartRequired::REASON_PERIODIC;
       break;
     default:
       NOTREACHED() << "Unknown reboot reason=" << reason;
       return;
   }
 
   extensions::RuntimeEventRouter::DispatchOnRestartRequiredEvent(
       profile_, app_id_, restart_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;
 }
 
 void KioskAppUpdateService::OnKioskAppCacheUpdated(const std::string& app_id) {
   if (app_id != app_id_)
     return;
 
   extensions::RuntimeEventRouter::DispatchOnRestartRequiredEvent(
       profile_,
       app_id_,
@@ skipping 31 matching lines @@
 }
 
 KeyedService* KioskAppUpdateServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   return new KioskAppUpdateService(
       Profile::FromBrowserContext(context),
       g_browser_process->platform_part()->automatic_reboot_manager());
 }
 
 }  // namespace chromeos