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

chrome/browser/notifications/notification_ui_manager.cc

Index: chrome/browser/notifications/notification_ui_manager.cc
===================================================================
--- chrome/browser/notifications/notification_ui_manager.cc	(revision 65581)
+++ chrome/browser/notifications/notification_ui_manager.cc	(working copy)
@@ -66,19 +66,40 @@
   CheckAndShowNotifications();
 }
 
-bool NotificationUIManager::Cancel(const Notification& notification) {
-  // First look through the notifications that haven't been shown.  If not
-  // found there, call to the active balloon collection to tear it down.
+bool NotificationUIManager::CancelById(const std::string& id) {
+  // See if this ID hasn't been shown yet.
   NotificationDeque::iterator iter;
   for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) {
-    if (notification.IsSame((*iter)->notification())) {
+    if ((*iter)->notification().notification_id() == id) {
       show_queue_.erase(iter);
       return true;
     }
   }
-  return balloon_collection_->Remove(notification);
+  // If it has been shown, remove it from the balloon collections.
+  return balloon_collection_->RemoveById(id);
 }
 
+bool NotificationUIManager::CancelAllBySourceOrigin(const GURL& source) {
+  // Same pattern as CancelById, but more complicated than the above
+  // because there may be multiple notifications from the same source.
+  bool searching = true;
+  bool removed = false;
+  while (searching) {
+    searching = false;

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

This seems like an odd way to implement this. I thought that deque.erase()
returned a valid iterator pointing to the next element - I'm wondering if you
could replace the entire while(searching) loop with:

for (iter = begin(); iter != end(); ) {
  if ((*iter)->notification().origin_url() == source) {
    iter = show_queue_.erase(iter);
    removed = true;
  } else {
    ++iter;
  }
}

That way you don't have to start back at the beginning of the loop again and the
logic is simpler?

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

thanks, i tend to play it safe when modifying iterators but your code works and
is much tighter.

+    NotificationDeque::iterator iter;
+    for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) {
+      if ((*iter)->notification().origin_url() == source) {
+        show_queue_.erase(iter);
+        searching = true;
+        removed = true;
+        break;
+      }
+    }
+  }
+
+  return balloon_collection_->RemoveBySourceOrigin(source) || removed;
+}
+
 void NotificationUIManager::CheckAndShowNotifications() {
   // TODO(johnnyg): http://crbug.com/25061 - Check for user idle/presentation.
   ShowNotifications();