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

chrome/browser/component_updater/component_updater_service.cc

Index: chrome/browser/component_updater/component_updater_service.cc
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index 804885582df9a2b4c893b5fc161e377c760fa879..fb709a1739e6ffad5d3e638652a4be275d97d189 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -311,6 +311,9 @@ class CrxUpdateService : public ComponentUpdateService {
   virtual Status Stop() OVERRIDE;
   virtual Status RegisterComponent(const CrxComponent& component) OVERRIDE;
   virtual Status CheckForUpdateSoon(const CrxComponent& component) OVERRIDE;
+  virtual bool IsUpdateNotificationForComponent(
+      const content::NotificationSource& source,
+      const CrxComponent& component) OVERRIDE;
 
   // The only purpose of this class is to forward the
   // UtilityProcessHostClient callbacks so CrxUpdateService does
@@ -984,6 +987,15 @@ void CrxUpdateService::DoneInstalling(const std::string& component_id,
   ScheduleNextRun(false);
 }
 
+bool CrxUpdateService::IsUpdateNotificationForComponent(

[Sorin Jianu, 2013/07/03 23:10:04]

What is strange about this function is that is seems to be a utility function
that queries some other module. I would just make this a non-member function and
make it available in component_updater_service.h.

I believe cpu@ made a comment that he'd be open to make this a non-member.

[jvoung (off chromium), 2013/07/04 01:11:04]

One argument for having it be a member function, is that the function has
knowledge about the type of the notifications that this "CrxUpdateService"
implementation sends.  In this case, it's a std::string, which happens to be the
CRX's app ID.

If there was a different implementation, it could send a different kind of
notification (something unrelated to CRXes).  However, there's currently no way
for the observer to know that "chrome::NOTIFICATION_COMPONENT_UPDATE_FOUND" came
from this implementation, and therefore should check against this
implementation's IsUpdateNotificationForComponent() method.  Perhaps the source
should be a pointer to the component update service (which is what is done for
chrome::NOTIFICATION_COMPONENT_UPDATER_STARTED), and the details should the ID
string.  Then you could do source->IsUpdateNotificationForComponent(details).


On the other hand, a simple utility function also works, because this is the
only implementation for now...

Thoughts?

+    const content::NotificationSource& src,
+    const CrxComponent& component) {
+  std::string id =
+      HexStringToID(StringToLowerASCII(base::HexEncode(&component.pk_hash[0],
+                                       component.pk_hash.size()/2)));
+  return content::Source<std::string>(src)->compare(id) == 0;
+}
+
 // The component update factory. Using the component updater as a singleton
 // is the job of the browser process.
 ComponentUpdateService* ComponentUpdateServiceFactory(