Consistently use notifications from component updater w/ on-demand PNaCl.

chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc

Index: chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc
diff --git a/chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc
index 066867aff154ee4c924e004f8c9d3bfc7a9bf04c..6041b5d238dc7aba9e92a9d59cf26f24c89d1646 100644
--- a/chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc
+++ b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc
@@ -4,28 +4,46 @@
 
 #include "chrome/browser/component_updater/pnacl/pnacl_updater_observer.h"
 
-#include "base/logging.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/component_updater/pnacl/pnacl_component_installer.h"
-#include "content/public/browser/notification_service.h"
+#include "content/public/browser/browser_thread.h"
 
-PnaclUpdaterObserver::PnaclUpdaterObserver(
-    PnaclComponentInstaller* installer) : pnacl_installer_(installer) {
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
-                 content::NotificationService::AllSources());
+using content::BrowserThread;
+
+PnaclUpdaterObserver::PnaclUpdaterObserver(PnaclComponentInstaller* pci)
+    : must_observe_(false), pnacl_installer_(pci) {}
+
+PnaclUpdaterObserver::~PnaclUpdaterObserver() {}
+
+void PnaclUpdaterObserver::EnsureObserving() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  must_observe_ = true;
+}
+
+void PnaclUpdaterObserver::StopObserving() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  must_observe_ = false;
 }
 
-PnaclUpdaterObserver::~PnaclUpdaterObserver() { }
-
-void PnaclUpdaterObserver::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING) {
-    // If the component updater sleeps before a NotifyInstallSuccess,
-    // then requests for installs were likely skipped, or an error occurred.
-    pnacl_installer_->NotifyInstallError();
-    return;
+void PnaclUpdaterObserver::OnEvent(Events event, int extra) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (must_observe_) {
+    switch (event) {
+      default:
+        break;
+      case COMPONENT_UPDATE_READY:
+        // If the component updater says there is an UPDATE_READY w/ source
+        // being the PNaCl ID, then installation is handed off to the PNaCl
+        // installer and we can stop observing. The installer will be the one
+        // to run the callback w/ success or failure.

[Sorin Jianu, 2013/08/05 19:24:58]

There is case where this would break but it is going to be hard to fix with the
current design and implementation of the code in the component updater.

This function will be called when the CRX (or a differential update of it) has
been downloaded. So in the case of a diff update, we call it, we unsubscribe for
notifications here, but if the patch fails, CUS will try the full, and if the
full update fails too, the NotifyInstallError will not get called. 

[jvoung (off chromium), 2013/08/05 20:36:03]

Good point about how the install could take more than one try (diff and full). 
Right now we are only using it to install from version 0.  Does that also
attempt a diff first?

+        must_observe_ = false;
+        break;
+      case COMPONENT_UPDATER_SLEEPING:
+        // If the component updater sleeps before an UPDATE_READY for this
+        // component, then requests for installs were likely skipped,
+        // an error occurred, or there was no new update.
+        must_observe_ = false;
+        pnacl_installer_->NotifyInstallError();
+        break;
+    }
   }
 }