When an extension is uninstalled, close all desktop notifications from that e...

chrome/browser/notifications/desktop_notification_service.cc

Index: chrome/browser/notifications/desktop_notification_service.cc
===================================================================
--- chrome/browser/notifications/desktop_notification_service.cc	(revision 65581)
+++ chrome/browser/notifications/desktop_notification_service.cc	(working copy)
@@ -215,7 +215,7 @@
     NotificationUIManager* ui_manager)
     : profile_(profile),
       ui_manager_(ui_manager) {
-  registrar_.Init(profile_->GetPrefs());
+  prefs_registrar_.Init(profile_->GetPrefs());
   InitPrefs();
   StartObserving();
 }
@@ -260,15 +260,20 @@
 
 void DesktopNotificationService::StartObserving() {
   if (!profile_->IsOffTheRecord()) {
-    registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting, this);
-    registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this);
-    registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this);
+    prefs_registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting,
+                         this);
+    prefs_registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this);
+    prefs_registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this);
+
+    notification_registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
+                                NotificationService::AllSources());
   }
 }
 
 void DesktopNotificationService::StopObserving() {
   if (!profile_->IsOffTheRecord()) {
-    registrar_.RemoveAll();
+    prefs_registrar_.RemoveAll();
+    notification_registrar_.RemoveAll();
   }
 }
 
@@ -299,11 +304,22 @@
 void DesktopNotificationService::Observe(NotificationType type,
                                          const NotificationSource& source,
                                          const NotificationDetails& details) {
-  DCHECK(NotificationType::PREF_CHANGED == type);
+  if (NotificationType::PREF_CHANGED == type) {
+    const std::string& name = *Details<std::string>(details).ptr();
+    OnPrefsChanged(name);
+  } else if (NotificationType::EXTENSION_UNLOADED) {

[Andrew T Wilson (Slow), 2010/11/11 01:07:44]

Should this be "type == NotificationType::PREF_CHANGED"?

[John Gregg, 2010/11/11 18:13:51]

yikes, good catch.  kinda funny that when there are only two types being
observed this actually works :)

+    // Remove all notifications currently shown or queued by the extension
+    // which was unloaded.
+    Extension* extension = Details<Extension>(details).ptr();
+    if (extension)
+      ui_manager_->CancelAllBySourceOrigin(extension->url());

[Andrew T Wilson (Slow), 2010/11/11 01:07:44]

Does this work for applications, which have web_extents? Maybe we don't want to
close notifications when you uninstall apps, but uninstalling an app may remove
permission for notifications for that domain which is something to keep in mind.

[John Gregg, 2010/11/11 18:13:51]

I don't think that would be necessary to fix the bug in question, but I can
follow up on it.

+  }
+}
+
+void DesktopNotificationService::OnPrefsChanged(const std::string& pref_name) {
   PrefService* prefs = profile_->GetPrefs();
-  const std::string& name = *Details<std::string>(details).ptr();
 
-  if (name == prefs::kDesktopNotificationAllowedOrigins) {
+  if (pref_name == prefs::kDesktopNotificationAllowedOrigins) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -317,7 +333,7 @@
             prefs_cache_.get(),
             &NotificationsPrefsCache::SetCacheAllowedOrigins,
             allowed_origins));
-  } else if (name == prefs::kDesktopNotificationDeniedOrigins) {
+  } else if (pref_name == prefs::kDesktopNotificationDeniedOrigins) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -331,7 +347,7 @@
             prefs_cache_.get(),
             &NotificationsPrefsCache::SetCacheDeniedOrigins,
             denied_origins));
-  } else if (name == prefs::kDesktopNotificationDefaultContentSetting) {
+  } else if (pref_name == prefs::kDesktopNotificationDefaultContentSetting) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_DEFAULT_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -563,9 +579,7 @@
   scoped_refptr<NotificationObjectProxy> proxy(
       new NotificationObjectProxy(process_id, route_id, notification_id,
                                   false));
-  // TODO(johnnyg): clean up this "empty" notification.
-  Notification notif(GURL(), GURL(), string16(), string16(), proxy);
-  return ui_manager_->Cancel(notif);
+  return ui_manager_->CancelById(proxy->id());
 }