CrOS - Fix use-after-free in balloon notifications

The OnStale notification was using pointer to a closed (and hence deleted) balloon.

BUG=chromium-os:20356
TEST=none

Review URL: http://codereview.chromium.org/7887012

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

4 files changed, 39 insertions, 27 deletions

diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc
index dba581c..e7ba8a0 100644
--- a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc
+++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc
@@ -131,7 +131,7 @@ void BalloonCollectionImpl::ResizeBalloon(Balloon* balloon,
 
 void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
   notification_ui_->Remove(source);
-  base_.Remove(source);
+  base_.Remove(source);  // Deletes |source|.
 
   // There may be no listener in a unit test.
   if (space_change_listener_)
diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.h b/chrome/browser/chromeos/notifications/balloon_collection_impl.h
index e4a6d81..e8b3980 100644
--- a/chrome/browser/chromeos/notifications/balloon_collection_impl.h
+++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.h
@@ -60,21 +60,21 @@ class BalloonCollectionImpl : public BalloonCollection,
 
   // BalloonCollectionInterface overrides
   virtual void Add(const Notification& notification,
-                   Profile* profile);
-  virtual bool RemoveById(const std::string& id);
-  virtual bool RemoveBySourceOrigin(const GURL& origin);
-  virtual void RemoveAll();
-  virtual bool HasSpace() const;
-  virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size);
-  virtual void SetPositionPreference(PositionPreference position) {}
-  virtual void DisplayChanged() {}
-  virtual void OnBalloonClosed(Balloon* source);
-  virtual const Balloons& GetActiveBalloons();
+                   Profile* profile) OVERRIDE;
+  virtual bool RemoveById(const std::string& id) OVERRIDE;
+  virtual bool RemoveBySourceOrigin(const GURL& origin) OVERRIDE;
+  virtual void RemoveAll() OVERRIDE;
+  virtual bool HasSpace() const OVERRIDE;
+  virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size) OVERRIDE;
+  virtual void SetPositionPreference(PositionPreference position) OVERRIDE {}
+  virtual void DisplayChanged() OVERRIDE {}
+  virtual void OnBalloonClosed(Balloon* source) OVERRIDE;
+  virtual const Balloons& GetActiveBalloons() OVERRIDE;
 
   // NotificationObserver overrides:
   virtual void Observe(int type,
                        const NotificationSource& source,
-                       const NotificationDetails& details);
+                       const NotificationDetails& details) OVERRIDE;
 
   // Adds a callback for WebUI message. Returns true if the callback
   // is succssfully registered, or false otherwise. It fails to add if
diff --git a/chrome/browser/chromeos/notifications/notification_panel.cc b/chrome/browser/chromeos/notifications/notification_panel.cc
index eee95fd..7bbbf65 100644
--- a/chrome/browser/chromeos/notifications/notification_panel.cc
+++ b/chrome/browser/chromeos/notifications/notification_panel.cc
@@ -240,6 +240,16 @@ class BalloonSubContainer : public views::View {
     return NULL;
   }
 
+  // Returns true if the |view| is in the container.
+  // |view| can be a deleted pointer - we do not dereference it.
+  bool HasChildView(View* view) const {
+    for (int i = 0; i < child_count(); ++i) {
+      if (child_at(i) == view)
+        return true;
+    }
+    return false;
+  }
+
  private:
   gfx::Size preferred_size_;
   int margin_;
@@ -345,9 +355,10 @@ class BalloonContainer : public views::View {
   }
 
   // Returns true if the |view| is contained in the panel.
+  // |view| can be a deleted pointer - we do not dereference it.
   bool HasBalloonView(View* view) {
-    return view->parent() == sticky_container_ ||
-        view->parent() == non_sticky_container_;
+    return sticky_container_->HasChildView(view) ||
+        non_sticky_container_->HasChildView(view);
   }
 
   // Updates the bounds so that all notifications are visible.
@@ -420,7 +431,7 @@ NotificationPanel::~NotificationPanel() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// NottificationPanel public.
+// NotificationPanel public.
 
 void NotificationPanel::Show() {
   if (!panel_widget_) {
@@ -796,6 +807,7 @@ void NotificationPanel::StartStaleTimer(Balloon* balloon) {
 }
 
 void NotificationPanel::OnStale(BalloonViewImpl* view) {
+  // Note: |view| may point to deleted memory.
   if (balloon_container_->HasBalloonView(view) && !view->stale()) {
     view->set_stale();
     // don't update panel on stale
diff --git a/chrome/browser/chromeos/notifications/notification_panel.h b/chrome/browser/chromeos/notifications/notification_panel.h
index 92385bc..13c5e03 100644
--- a/chrome/browser/chromeos/notifications/notification_panel.h
+++ b/chrome/browser/chromeos/notifications/notification_panel.h
@@ -89,25 +89,25 @@ class NotificationPanel : public PanelController::Delegate,
   void Hide();
 
   // BalloonCollectionImpl::NotificationUI overrides..
-  virtual void Add(Balloon* balloon);
-  virtual bool Update(Balloon* balloon);
-  virtual void Remove(Balloon* balloon);
-  virtual void Show(Balloon* balloon);
+  virtual void Add(Balloon* balloon) OVERRIDE;
+  virtual bool Update(Balloon* balloon) OVERRIDE;
+  virtual void Remove(Balloon* balloon) OVERRIDE;
+  virtual void Show(Balloon* balloon) OVERRIDE;
   virtual void ResizeNotification(Balloon* balloon,
-                                  const gfx::Size& size);
-  virtual void SetActiveView(BalloonViewImpl* view);
+                                  const gfx::Size& size) OVERRIDE;
+  virtual void SetActiveView(BalloonViewImpl* view) OVERRIDE;
 
   // PanelController::Delegate overrides.
-  virtual string16 GetPanelTitle();
-  virtual SkBitmap GetPanelIcon();
-  virtual bool CanClosePanel();
-  virtual void ClosePanel();
-  virtual void ActivatePanel();
+  virtual string16 GetPanelTitle() OVERRIDE;
+  virtual SkBitmap GetPanelIcon() OVERRIDE;
+  virtual bool CanClosePanel() OVERRIDE;
+  virtual void ClosePanel() OVERRIDE;
+  virtual void ActivatePanel() OVERRIDE;
 
   // NotificationObserver overrides:
   virtual void Observe(int type,
                        const NotificationSource& source,
-                       const NotificationDetails& details);
+                       const NotificationDetails& details) OVERRIDE;
 
   // Called when a mouse left the panel window.
   void OnMouseLeave();