Revert 66571 - When an extension is uninstalled, close all desktop notifications from that extension.

This change also refactors the balloon collection code to remove duplication between chrome and chromeos.

Removes some gross removal code which was using fake notifications just to get the right ID.

BUG=58266
TEST=open notifications from extension, uninstall extensions


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65879
Review URL: http://codereview.chromium.org/4635007

TBR=johnnyg@chromium.org
Review URL: http://codereview.chromium.org/5162003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66572 0039d316-1c4b-4281-b951-d872f2087c98

17 files changed, 106 insertions, 292 deletions

diff --git a/chrome/browser/notifications/balloon_collection.cc b/chrome/browser/notifications/balloon_collection.cc
index 7d926ed..a125837 100644
--- a/chrome/browser/notifications/balloon_collection.cc
+++ b/chrome/browser/notifications/balloon_collection.cc
@@ -45,6 +45,7 @@ BalloonCollectionImpl::BalloonCollectionImpl()
 }
 
 BalloonCollectionImpl::~BalloonCollectionImpl() {
+  STLDeleteElements(&balloons_);
 }
 
 void BalloonCollectionImpl::Add(const Notification& notification,
@@ -58,11 +59,10 @@ void BalloonCollectionImpl::Add(const Notification& notification,
   new_balloon->SetPosition(layout_.OffScreenLocation(), false);
   new_balloon->Show();
 #if USE_OFFSETS
-  int count = base_.count();
-  if (count > 0)
-    new_balloon->set_offset(base_.balloons()[count - 1]->offset());
+  if (balloons_.size() > 0)
+    new_balloon->set_offset(balloons_[balloons_.size() - 1]->offset());
 #endif
-  base_.Add(new_balloon);
+  balloons_.push_back(new_balloon);
   PositionBalloons(false);
 
   // There may be no listener in a unit test.
@@ -74,24 +74,28 @@ void BalloonCollectionImpl::Add(const Notification& notification,
     on_collection_changed_callback_->Run();
 }
 
-bool BalloonCollectionImpl::RemoveById(const std::string& id) {
-  return base_.CloseById(id);
-}
-
-bool BalloonCollectionImpl::RemoveBySourceOrigin(const GURL& origin) {
-  return base_.CloseAllBySourceOrigin(origin);
+bool BalloonCollectionImpl::Remove(const Notification& notification) {
+  Balloons::iterator iter;
+  for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) {
+    if (notification.IsSame((*iter)->notification())) {
+      // Balloon.CloseByScript() will cause OnBalloonClosed() to be called on
+      // this object, which will remove it from the collection and free it.
+      (*iter)->CloseByScript();
+      return true;
+    }
+  }
+  return false;
 }
 
 bool BalloonCollectionImpl::HasSpace() const {
-  int count = base_.count();
-  if (count < kMinAllowedBalloonCount)
+  if (count() < kMinAllowedBalloonCount)
     return true;
 
   int max_balloon_size = 0;
   int total_size = 0;
   layout_.GetMaxLinearSize(&max_balloon_size, &total_size);
 
-  int current_max_size = max_balloon_size * count;
+  int current_max_size = max_balloon_size * count();
   int max_allowed_size = static_cast<int>(total_size *
                                           kPercentBalloonFillFactor);
   return current_max_size < max_allowed_size - max_balloon_size;
@@ -110,16 +114,16 @@ void BalloonCollectionImpl::DisplayChanged() {
 
 void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
   // We want to free the balloon when finished.
-  const Balloons& balloons = base_.balloons();
-  Balloons::const_iterator it = balloons.begin();
+  scoped_ptr<Balloon> closed(source);
+  Balloons::iterator it = balloons_.begin();
 
 #if USE_OFFSETS
   gfx::Point offset;
   bool apply_offset = false;
-  while (it != balloons.end()) {
+  while (it != balloons_.end()) {
     if (*it == source) {
-      ++it;
-      if (it != balloons.end()) {
+      it = balloons_.erase(it);
+      if (it != balloons_.end()) {
         apply_offset = true;
         offset.set_y((source)->offset().y() - (*it)->offset().y() +
             (*it)->content_size().height() - source->content_size().height());
@@ -134,9 +138,15 @@ void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
   // leaves the balloon area.
   if (apply_offset)
     AddMessageLoopObserver();
+#else
+  for (; it != balloons_.end(); ++it) {
+    if (*it == source) {
+      balloons_.erase(it);
+      break;
+    }
+  }
 #endif
 
-  base_.Remove(source);
   PositionBalloons(true);
 
   // There may be no listener in a unit test.
@@ -149,13 +159,9 @@ void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
 }
 
 void BalloonCollectionImpl::PositionBalloonsInternal(bool reposition) {
-  const Balloons& balloons = base_.balloons();
-
   layout_.RefreshSystemMetrics();
   gfx::Point origin = layout_.GetLayoutOrigin();
-  for (Balloons::const_iterator it = balloons.begin();
-       it != balloons.end();
-       ++it) {
+  for (Balloons::iterator it = balloons_.begin(); it != balloons_.end(); ++it) {
     gfx::Point upper_left = layout_.NextPosition((*it)->GetViewSize(), &origin);
     (*it)->SetPosition(upper_left, reposition);
   }
@@ -182,10 +188,7 @@ void BalloonCollectionImpl::CancelOffsets() {
   // Unhook from listening to all UI events.
   RemoveMessageLoopObserver();
 
-  const Balloons& balloons = base_.balloons();
-  for (Balloons::const_iterator it = balloons.begin();
-       it != balloons.end();
-       ++it)
+  for (Balloons::iterator it = balloons_.begin(); it != balloons_.end(); ++it)
     (*it)->set_offset(gfx::Point(0, 0));
 
   PositionBalloons(true);
diff --git a/chrome/browser/notifications/balloon_collection.h b/chrome/browser/notifications/balloon_collection.h
index 2d75686..499b937 100644
--- a/chrome/browser/notifications/balloon_collection.h
+++ b/chrome/browser/notifications/balloon_collection.h
@@ -9,13 +9,11 @@
 #pragma once
 
 #include <deque>
-#include <string>
 
 #include "base/callback.h"
 #include "base/scoped_ptr.h"
 
 class Balloon;
-class GURL;
 class Notification;
 class Profile;
 
@@ -46,13 +44,9 @@ class BalloonCollection {
   virtual void Add(const Notification& notification,
                    Profile* profile) = 0;
 
-  // Removes any balloons that have this notification id.  Returns
+  // Removes a balloon from the collection if present.  Returns
   // true if anything was removed.
-  virtual bool RemoveById(const std::string& id) = 0;
-
-  // Removes any balloons that have this source origin.  Returns
-  // true if anything was removed.
-  virtual bool RemoveBySourceOrigin(const GURL& source_origin) = 0;
+  virtual bool Remove(const Notification& notification) = 0;
 
   // Is there room to add another notification?
   virtual bool HasSpace() const = 0;
diff --git a/chrome/browser/notifications/balloon_collection_base.cc b/chrome/browser/notifications/balloon_collection_base.cc
deleted file mode 100644
index 94765c0..0000000
--- a/chrome/browser/notifications/balloon_collection_base.cc
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright (c) 2010 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/notifications/balloon_collection_base.h"
-
-#include "base/stl_util-inl.h"
-#include "chrome/browser/notifications/balloon.h"
-#include "chrome/browser/notifications/notification.h"
-#include "googleurl/src/gurl.h"
-
-BalloonCollectionBase::BalloonCollectionBase() {
-}
-
-BalloonCollectionBase::~BalloonCollectionBase() {
-  STLDeleteElements(&balloons_);
-}
-
-void BalloonCollectionBase::Add(Balloon* balloon) {
-  balloons_.push_back(balloon);
-}
-
-void BalloonCollectionBase::Remove(Balloon* balloon) {
-  // Free after removing.
-  scoped_ptr<Balloon> to_delete(balloon);
-  Balloons::iterator iter;
-  for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) {
-    if ((*iter) == balloon) {
-      balloons_.erase(iter);
-      return;
-    }
-  }
-}
-
-bool BalloonCollectionBase::CloseById(const std::string& id) {
-  // Use a local list of balloons to close to avoid breaking
-  // iterator changes on each close.
-  Balloons to_close;
-  Balloons::iterator iter;
-  for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) {
-    if ((*iter)->notification().notification_id() == id)
-      to_close.push_back(*iter);
-  }
-  for (iter = to_close.begin(); iter != to_close.end(); ++iter)
-    (*iter)->CloseByScript();
-
-  return !to_close.empty();
-}
-
-bool BalloonCollectionBase::CloseAllBySourceOrigin(
-    const GURL& source_origin) {
-  // Use a local list of balloons to close to avoid breaking
-  // iterator changes on each close.
-  Balloons to_close;
-  Balloons::iterator iter;
-  for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) {
-    if ((*iter)->notification().origin_url() == source_origin)
-      to_close.push_back(*iter);
-  }
-  for (iter = to_close.begin(); iter != to_close.end(); ++iter)
-    (*iter)->CloseByScript();
-
-  return !to_close.empty();
-}
-
-Balloon* BalloonCollectionBase::FindBalloon(
-    const Notification& notification) {
-  Balloons::iterator iter;
-  for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) {
-    if ((*iter)->notification().notification_id() ==
-        notification.notification_id()) {
-      return *iter;
-    }
-  }
-  return NULL;
-}
diff --git a/chrome/browser/notifications/balloon_collection_base.h b/chrome/browser/notifications/balloon_collection_base.h
deleted file mode 100644
index 8511030..0000000
--- a/chrome/browser/notifications/balloon_collection_base.h
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright (c) 2010 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.
-
-// Handles the visible notification (or balloons).
-
-#ifndef CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_
-#define CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_
-#pragma once
-
-#include <deque>
-#include <string>
-
-#include "base/basictypes.h"
-
-class Balloon;
-class GURL;
-class Notification;
-
-// This class provides support for implementing a BalloonCollection
-// including the parts common between Chrome UI and ChromeOS UI.
-class BalloonCollectionBase {
- public:
-  BalloonCollectionBase();
-  virtual ~BalloonCollectionBase();
-
-  typedef std::deque<Balloon*> Balloons;
-
-  // Adds a balloon to the collection. Takes ownership of pointer.
-  virtual void Add(Balloon* balloon);
-
-  // Removes a balloon from the collection (if present).  Frees
-  // the pointer after removal.
-  virtual void Remove(Balloon* balloon);
-
-  // Finds any balloon matching the given notification id, and
-  // calls CloseByScript on it.  Returns true if anything was
-  // found.
-  virtual bool CloseById(const std::string& id);
-
-  // Finds all balloons matching the given notification source,
-  // and calls CloseByScript on them.  Returns true if anything
-  // was found.
-  virtual bool CloseAllBySourceOrigin(const GURL& source_origin);
-
-  const Balloons& balloons() const { return balloons_; }
-
-  // Returns the balloon matching the given notification, or
-  // NULL if there is no matching balloon.
-  Balloon* FindBalloon(const Notification& notification);
-
-  // The number of balloons being displayed.
-  int count() const { return static_cast<int>(balloons_.size()); }
-
- private:
-  // Queue of active balloons.  Pointers are owned by this class.
-  Balloons balloons_;
-
-  DISALLOW_COPY_AND_ASSIGN(BalloonCollectionBase);
-};
-
-#endif  // CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_
diff --git a/chrome/browser/notifications/balloon_collection_impl.h b/chrome/browser/notifications/balloon_collection_impl.h
index 3596a25..a9c7afc 100644
--- a/chrome/browser/notifications/balloon_collection_impl.h
+++ b/chrome/browser/notifications/balloon_collection_impl.h
@@ -13,7 +13,6 @@
 #include "base/basictypes.h"
 #include "base/message_loop.h"
 #include "chrome/browser/notifications/balloon_collection.h"
-#include "chrome/browser/notifications/balloon_collection_base.h"
 #include "gfx/point.h"
 #include "gfx/rect.h"
 
@@ -42,13 +41,14 @@ class BalloonCollectionImpl : public BalloonCollection
   // BalloonCollection interface.
   virtual void Add(const Notification& notification,
                    Profile* profile);
-  virtual bool RemoveById(const std::string& id);
-  virtual bool RemoveBySourceOrigin(const GURL& source_origin);
+  virtual bool Remove(const Notification& notification);
   virtual bool HasSpace() const;
   virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size);
   virtual void DisplayChanged();
   virtual void OnBalloonClosed(Balloon* source);
-  virtual const Balloons& GetActiveBalloons() { return base_.balloons(); }
+  virtual const Balloons& GetActiveBalloons() {
+    return balloons_;
+  }
 
   // MessageLoopForUI::Observer interface.
 #if defined(OS_WIN)
@@ -135,6 +135,9 @@ class BalloonCollectionImpl : public BalloonCollection
                                Profile* profile);
 
  private:
+  // The number of balloons being displayed.
+  int count() const { return balloons_.size(); }
+
   // Adjusts the positions of the balloons (e.g., when one is closed).
   // Implemented by each platform for specific UI requirements.
   void PositionBalloons(bool is_reposition);
@@ -147,12 +150,6 @@ class BalloonCollectionImpl : public BalloonCollection
   static gfx::Rect GetMacWorkArea();
 #endif
 
-  // Base implementation for the collection of active balloons.
-  BalloonCollectionBase base_;
-
-  // The layout parameters for balloons in this collection.
-  Layout layout_;
-
 #if USE_OFFSETS
   // Start and stop observing all UI events.
   void AddMessageLoopObserver();
@@ -166,7 +163,16 @@ class BalloonCollectionImpl : public BalloonCollection
 
   // Is the current cursor in the balloon area?
   bool IsCursorInBalloonCollection() const;
+#endif
 
+  // Queue of active balloons.
+  typedef std::deque<Balloon*> Balloons;
+  Balloons balloons_;
+
+  // The layout parameters for balloons in this collection.
+  Layout layout_;
+
+#if USE_OFFSETS
   // Factory for generating delayed reposition tasks on mouse motion.
   ScopedRunnableMethodFactory<BalloonCollectionImpl> reposition_factory_;
 
diff --git a/chrome/browser/notifications/balloon_collection_linux.cc b/chrome/browser/notifications/balloon_collection_linux.cc
index 08354a0..f15c713 100644
--- a/chrome/browser/notifications/balloon_collection_linux.cc
+++ b/chrome/browser/notifications/balloon_collection_linux.cc
@@ -46,11 +46,10 @@ void BalloonCollectionImpl::DidProcessEvent(GdkEvent* event) {
 }
 
 bool BalloonCollectionImpl::IsCursorInBalloonCollection() const {
-  const Balloons& balloons = base_.balloons();
-  if (balloons.empty())
+  if (balloons_.empty())
     return false;
 
-  gfx::Point upper_left = balloons[balloons.size() - 1]->GetPosition();
+  gfx::Point upper_left = balloons_[balloons_.size() - 1]->GetPosition();
   gfx::Point lower_right = layout_.GetLayoutOrigin();
 
   gfx::Rect bounds = gfx::Rect(upper_left.x(),
diff --git a/chrome/browser/notifications/balloon_collection_win.cc b/chrome/browser/notifications/balloon_collection_win.cc
index 07bcd18..8915662 100644
--- a/chrome/browser/notifications/balloon_collection_win.cc
+++ b/chrome/browser/notifications/balloon_collection_win.cc
@@ -44,11 +44,10 @@ void BalloonCollectionImpl::DidProcessMessage(const MSG& msg) {
 }
 
 bool BalloonCollectionImpl::IsCursorInBalloonCollection() const {
-  const Balloons& balloons = base_.balloons();
-  if (balloons.empty())
+  if (balloons_.empty())
     return false;
 
-  gfx::Point upper_left = balloons[balloons.size() - 1]->GetPosition();
+  gfx::Point upper_left = balloons_[balloons_.size() - 1]->GetPosition();
   gfx::Point lower_right = layout_.GetLayoutOrigin();
 
   gfx::Rect bounds = gfx::Rect(upper_left.x(),
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc
index 065d355..e7530e4 100644
--- a/chrome/browser/notifications/desktop_notification_service.cc
+++ b/chrome/browser/notifications/desktop_notification_service.cc
@@ -215,7 +215,7 @@ DesktopNotificationService::DesktopNotificationService(Profile* profile,
     NotificationUIManager* ui_manager)
     : profile_(profile),
       ui_manager_(ui_manager) {
-  prefs_registrar_.Init(profile_->GetPrefs());
+  registrar_.Init(profile_->GetPrefs());
   InitPrefs();
   StartObserving();
 }
@@ -260,24 +260,16 @@ void DesktopNotificationService::InitPrefs() {
 
 void DesktopNotificationService::StartObserving() {
   if (!profile_->IsOffTheRecord()) {
-    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());
+    registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting, this);
+    registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this);
+    registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this);
   }
-
-  notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
-                              Source<Profile>(profile_));
 }
 
 void DesktopNotificationService::StopObserving() {
   if (!profile_->IsOffTheRecord()) {
-    prefs_registrar_.RemoveAll();
+    registrar_.RemoveAll();
   }
-  notification_registrar_.RemoveAll();
 }
 
 void DesktopNotificationService::GrantPermission(const GURL& origin) {
@@ -307,24 +299,11 @@ void DesktopNotificationService::DenyPermission(const GURL& origin) {
 void DesktopNotificationService::Observe(NotificationType type,
                                          const NotificationSource& source,
                                          const NotificationDetails& details) {
-  if (NotificationType::PREF_CHANGED == type) {
-    const std::string& name = *Details<std::string>(details).ptr();
-    OnPrefsChanged(name);
-  } else if (NotificationType::EXTENSION_UNLOADED == type) {
-    // 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());
-  } else if (NotificationType::PROFILE_DESTROYED == type) {
-    StopObserving();
-  }
-}
-
-void DesktopNotificationService::OnPrefsChanged(const std::string& pref_name) {
+  DCHECK(NotificationType::PREF_CHANGED == type);
   PrefService* prefs = profile_->GetPrefs();
+  const std::string& name = *Details<std::string>(details).ptr();
 
-  if (pref_name == prefs::kDesktopNotificationAllowedOrigins) {
+  if (name == prefs::kDesktopNotificationAllowedOrigins) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -338,7 +317,7 @@ void DesktopNotificationService::OnPrefsChanged(const std::string& pref_name) {
             prefs_cache_.get(),
             &NotificationsPrefsCache::SetCacheAllowedOrigins,
             allowed_origins));
-  } else if (pref_name == prefs::kDesktopNotificationDeniedOrigins) {
+  } else if (name == prefs::kDesktopNotificationDeniedOrigins) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -352,7 +331,7 @@ void DesktopNotificationService::OnPrefsChanged(const std::string& pref_name) {
             prefs_cache_.get(),
             &NotificationsPrefsCache::SetCacheDeniedOrigins,
             denied_origins));
-  } else if (pref_name == prefs::kDesktopNotificationDefaultContentSetting) {
+  } else if (name == prefs::kDesktopNotificationDefaultContentSetting) {
     NotificationService::current()->Notify(
         NotificationType::DESKTOP_NOTIFICATION_DEFAULT_CHANGED,
         Source<DesktopNotificationService>(this),
@@ -584,7 +563,9 @@ bool DesktopNotificationService::CancelDesktopNotification(
   scoped_refptr<NotificationObjectProxy> proxy(
       new NotificationObjectProxy(process_id, route_id, notification_id,
                                   false));
-  return ui_manager_->CancelById(proxy->id());
+  // TODO(johnnyg): clean up this "empty" notification.
+  Notification notif(GURL(), GURL(), string16(), string16(), proxy);
+  return ui_manager_->Cancel(notif);
 }
 
 
diff --git a/chrome/browser/notifications/desktop_notification_service.h b/chrome/browser/notifications/desktop_notification_service.h
index 53fa56f..d92d7e6 100644
--- a/chrome/browser/notifications/desktop_notification_service.h
+++ b/chrome/browser/notifications/desktop_notification_service.h
@@ -6,24 +6,24 @@
 #define CHROME_BROWSER_NOTIFICATIONS_DESKTOP_NOTIFICATION_SERVICE_H_
 #pragma once
 
-#include <string>
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/ref_counted.h"
 #include "base/string16.h"
+#include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/common/content_settings.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
 #include "googleurl/src/gurl.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebTextDirection.h"
 
-class Notification;
 class NotificationUIManager;
 class NotificationsPrefsCache;
 class PrefService;
 class Profile;
+class Task;
 class TabContents;
 struct ViewHostMsg_ShowNotification_Params;
 
@@ -121,8 +121,6 @@ class DesktopNotificationService : public NotificationObserver {
   void StartObserving();
   void StopObserving();
 
-  void OnPrefsChanged(const std::string& pref_name);
-
   // Takes a notification object and shows it in the UI.
   void ShowNotification(const Notification& notification);
 
@@ -145,8 +143,7 @@ class DesktopNotificationService : public NotificationObserver {
   // UI for desktop toasts.
   NotificationUIManager* ui_manager_;
 
-  PrefChangeRegistrar prefs_registrar_;
-  NotificationRegistrar notification_registrar_;
+  PrefChangeRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(DesktopNotificationService);
 };
diff --git a/chrome/browser/notifications/desktop_notifications_unittest.cc b/chrome/browser/notifications/desktop_notifications_unittest.cc
index ebb6ab8..99466901 100644
--- a/chrome/browser/notifications/desktop_notifications_unittest.cc
+++ b/chrome/browser/notifications/desktop_notifications_unittest.cc
@@ -16,18 +16,26 @@ std::string DesktopNotificationsTest::log_output_;
 
 void MockBalloonCollection::Add(const Notification& notification,
                                 Profile* profile) {
-  // Swap in a logging proxy for the purpose of logging calls that
+  // Swap in the logging proxy for the purpose of logging calls that
   // would be made into javascript, then pass this down to the
   // balloon collection.
-  Notification test_notification(
-      notification.origin_url(),
-      notification.content_url(),
-      notification.display_source(),
-      notification.replace_id(),
-      new LoggingNotificationProxy(notification.notification_id()));
+  Notification test_notification(notification.origin_url(),
+                                 notification.content_url(),
+                                 notification.display_source(),
+                                 string16(), /* replace_id */
+                                 log_proxy_.get());
   BalloonCollectionImpl::Add(test_notification, profile);
 }
 
+bool MockBalloonCollection::Remove(const Notification& notification) {
+  Notification test_notification(notification.origin_url(),
+                                 notification.content_url(),
+                                 notification.display_source(),
+                                 string16(), /* replace_id */
+                                 log_proxy_.get());
+  return BalloonCollectionImpl::Remove(test_notification);
+}
+
 Balloon* MockBalloonCollection::MakeBalloon(const Notification& notification,
                                             Profile* profile) {
   // Start with a normal balloon but mock out the view.
diff --git a/chrome/browser/notifications/desktop_notifications_unittest.h b/chrome/browser/notifications/desktop_notifications_unittest.h
index 9f556e4..3b02b7b 100644
--- a/chrome/browser/notifications/desktop_notifications_unittest.h
+++ b/chrome/browser/notifications/desktop_notifications_unittest.h
@@ -22,14 +22,15 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 class DesktopNotificationsTest;
-typedef LoggingNotificationDelegate<DesktopNotificationsTest>
+typedef LoggingNotificationProxyBase<DesktopNotificationsTest>
     LoggingNotificationProxy;
 
 // Test version of the balloon collection which counts the number
 // of notifications that are added to it.
 class MockBalloonCollection : public BalloonCollectionImpl {
  public:
-  MockBalloonCollection() {}
+  MockBalloonCollection() :
+    log_proxy_(new LoggingNotificationProxy()) {}
 
   // Our mock collection has an area large enough for a fixed number
   // of balloons.
@@ -39,6 +40,7 @@ class MockBalloonCollection : public BalloonCollectionImpl {
   // BalloonCollectionImpl overrides
   virtual void Add(const Notification& notification,
                    Profile* profile);
+  virtual bool Remove(const Notification& notification);
   virtual bool HasSpace() const { return count() < kMockBalloonSpace; }
   virtual Balloon* MakeBalloon(const Notification& notification,
                                Profile* profile);
@@ -61,6 +63,7 @@ class MockBalloonCollection : public BalloonCollectionImpl {
 
  private:
   std::deque<Balloon*> balloons_;
+  scoped_refptr<LoggingNotificationProxy> log_proxy_;
 };
 
 class DesktopNotificationsTest : public testing::Test {
diff --git a/chrome/browser/notifications/notification.cc b/chrome/browser/notifications/notification.cc
index e6d69a6..c429efa 100644
--- a/chrome/browser/notifications/notification.cc
+++ b/chrome/browser/notifications/notification.cc
@@ -34,3 +34,7 @@ Notification& Notification::operator=(const Notification& notification) {
   delegate_ = notification.delegate();
   return *this;
 }
+
+bool Notification::IsSame(const Notification& other) const {
+  return delegate()->id() == other.delegate()->id();
+}
diff --git a/chrome/browser/notifications/notification.h b/chrome/browser/notifications/notification.h
index 2040cf3..7b0ab8c 100644
--- a/chrome/browser/notifications/notification.h
+++ b/chrome/browser/notifications/notification.h
@@ -6,8 +6,6 @@
 #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_H_
 #pragma once
 
-#include <string>
-
 #include "base/basictypes.h"
 #include "chrome/browser/notifications/notification_object_proxy.h"
 #include "googleurl/src/gurl.h"
@@ -44,7 +42,7 @@ class Notification {
   void Click() const { delegate()->Click(); }
   void Close(bool by_user) const { delegate()->Close(by_user); }
 
-  std::string notification_id() const { return delegate()->id(); }
+  bool IsSame(const Notification& other) const;
 
  private:
   NotificationDelegate* delegate() const { return delegate_.get(); }
diff --git a/chrome/browser/notifications/notification_options_menu_model.cc b/chrome/browser/notifications/notification_options_menu_model.cc
index 4370e63..97d269c 100644
--- a/chrome/browser/notifications/notification_options_menu_model.cc
+++ b/chrome/browser/notifications/notification_options_menu_model.cc
@@ -10,7 +10,6 @@
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
-#include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notifications_prefs_cache.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/content_settings_types.h"
diff --git a/chrome/browser/notifications/notification_test_util.h b/chrome/browser/notifications/notification_test_util.h
index fbaec4e..4cbf600 100644
--- a/chrome/browser/notifications/notification_test_util.h
+++ b/chrome/browser/notifications/notification_test_util.h
@@ -6,8 +6,6 @@
 #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_TEST_UTIL_H_
 #pragma once
 
-#include <string>
-
 #include "chrome/browser/notifications/notification_object_proxy.h"
 #include "chrome/browser/notifications/balloon.h"
 #include "gfx/size.h"
@@ -16,7 +14,7 @@
 // the notification events are not important.
 class MockNotificationDelegate : public NotificationDelegate {
  public:
-  explicit MockNotificationDelegate(const std::string& id) : id_(id) {}
+  explicit MockNotificationDelegate(std::string id) : id_(id) {}
   virtual ~MockNotificationDelegate() {}
 
   // NotificationDelegate interface.
@@ -28,8 +26,6 @@ class MockNotificationDelegate : public NotificationDelegate {
 
  private:
   std::string id_;
-
-  DISALLOW_COPY_AND_ASSIGN(MockNotificationDelegate);
 };
 
 // Mock implementation of Javascript object proxy which logs events that
@@ -39,11 +35,10 @@ class MockNotificationDelegate : public NotificationDelegate {
 // |Logger| class provided in template must implement method
 // static void log(string);
 template<class Logger>
-class LoggingNotificationDelegate : public NotificationDelegate {
+class LoggingNotificationProxyBase : public NotificationObjectProxy {
  public:
-  explicit LoggingNotificationDelegate(std::string id)
-      : notification_id_(id) {
-  }
+  LoggingNotificationProxyBase() :
+      NotificationObjectProxy(0, 0, 0, false) {}
 
   // NotificationObjectProxy override
   virtual void Display() {
@@ -52,22 +47,12 @@ class LoggingNotificationDelegate : public NotificationDelegate {
   virtual void Error() {
     Logger::log("notification error\n");
   }
-  virtual void Click() {
-    Logger::log("notification clicked\n");
-  }
   virtual void Close(bool by_user) {
     if (by_user)
       Logger::log("notification closed by user\n");
     else
       Logger::log("notification closed by script\n");
   }
-  virtual std::string id() const {
-    return notification_id_;
-  }
- private:
-  std::string notification_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(LoggingNotificationDelegate);
 };
 
 // Test version of a balloon view which doesn't do anything
diff --git a/chrome/browser/notifications/notification_ui_manager.cc b/chrome/browser/notifications/notification_ui_manager.cc
index 27f7626..adc44cf 100644
--- a/chrome/browser/notifications/notification_ui_manager.cc
+++ b/chrome/browser/notifications/notification_ui_manager.cc
@@ -66,34 +66,17 @@ void NotificationUIManager::Add(const Notification& notification,
   CheckAndShowNotifications();
 }
 
-bool NotificationUIManager::CancelById(const std::string& id) {
-  // See if this ID hasn't been shown yet.
+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.
   NotificationDeque::iterator iter;
   for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) {
-    if ((*iter)->notification().notification_id() == id) {
+    if (notification.IsSame((*iter)->notification())) {
       show_queue_.erase(iter);
       return true;
     }
   }
-  // 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 removed = false;
-  NotificationDeque::iterator iter;
-  for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) {
-    if ((*iter)->notification().origin_url() == source) {
-      iter = show_queue_.erase(iter);
-      removed = true;
-    } else {
-      ++iter;
-    }
-  }
-
-  return balloon_collection_->RemoveBySourceOrigin(source) || removed;
+  return balloon_collection_->Remove(notification);
 }
 
 void NotificationUIManager::CheckAndShowNotifications() {
diff --git a/chrome/browser/notifications/notification_ui_manager.h b/chrome/browser/notifications/notification_ui_manager.h
index 6d46198..10687a9 100644
--- a/chrome/browser/notifications/notification_ui_manager.h
+++ b/chrome/browser/notifications/notification_ui_manager.h
@@ -7,7 +7,6 @@
 #pragma once
 
 #include <deque>
-#include <string>
 
 #include "base/id_map.h"
 #include "base/scoped_ptr.h"
@@ -44,14 +43,8 @@ class NotificationUIManager
   virtual void Add(const Notification& notification,
                    Profile* profile);
 
-  // Removes any notifications matching the supplied ID, either currently
-  // displayed or in the queue.  Returns true if anything was removed.
-  virtual bool CancelById(const std::string& notification_id);
-
-  // Removes any notifications matching the supplied source origin
-  // (which could be an extension ID), either currently displayed or in the
-  // queue.  Returns true if anything was removed.
-  virtual bool CancelAllBySourceOrigin(const GURL& source_origin);
+  // Removes a notification.
+  virtual bool Cancel(const Notification& notification);
 
   // Returns balloon collection.
   BalloonCollection* balloon_collection() {