7 files changed, 127 insertions, 45 deletions

diff --git a/chrome/browser/status_icons/status_icon.cc b/chrome/browser/status_icons/status_icon.cc
new file mode 100644
index 0000000..85fd175
--- /dev/null
+++ b/chrome/browser/status_icons/status_icon.cc
@@ -0,0 +1,30 @@
+// 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 <algorithm>
+
+#include "chrome/browser/status_icons/status_icon.h"
+
+void StatusIcon::AddObserver(StatusIconObserver* observer) {
+  observers_.push_back(observer);
+}
+
+void StatusIcon::RemoveObserver(StatusIconObserver* observer) {
+  std::vector<StatusIconObserver*>::iterator iter =
+      std::find(observers_.begin(), observers_.end(), observer);
+  if (iter != observers_.end())
+    observers_.erase(iter);
+}
+
+void StatusIcon::DispatchClickEvent() {
+  // Walk observers, call callback for each one.
+  for (std::vector<StatusIconObserver*>::const_iterator iter =
+           observers_.begin();
+       iter != observers_.end();
+       ++iter) {
+    StatusIconObserver* observer = *iter;
+    observer->OnClicked();
+  }
+}
+
diff --git a/chrome/browser/status_icons/status_icon.h b/chrome/browser/status_icons/status_icon.h
index a2bc089..3050026 100644
--- a/chrome/browser/status_icons/status_icon.h
+++ b/chrome/browser/status_icons/status_icon.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_STATUS_ICONS_STATUS_ICON_H_
 #define CHROME_BROWSER_STATUS_ICONS_STATUS_ICON_H_
 
+#include <vector>
+
 #include "base/string16.h"
 
 class SkBitmap;
@@ -14,6 +16,7 @@ class StatusIcon {
   // Creates a new StatusIcon.
   static StatusIcon* Create();
 
+  StatusIcon() {}
   virtual ~StatusIcon() {}
 
   // Sets the image associated with this status icon.
@@ -33,9 +36,16 @@ class StatusIcon {
     virtual void OnClicked() = 0;
   };
 
-  // Adds/removes a observer for status bar events.
-  virtual void AddObserver(StatusIcon::StatusIconObserver* observer) = 0;
-  virtual void RemoveObserver(StatusIcon::StatusIconObserver* observer) = 0;
+  // Adds/removes an observer for status bar events.
+  void AddObserver(StatusIconObserver* observer);
+  void RemoveObserver(StatusIconObserver* observer);
+
+  // Dispatches a click event to the observers.
+  void DispatchClickEvent();
+
+ private:
+  std::vector<StatusIconObserver*> observers_;
+  DISALLOW_COPY_AND_ASSIGN(StatusIcon);
 };
 
 
diff --git a/chrome/browser/status_icons/status_icon_unittest.cc b/chrome/browser/status_icons/status_icon_unittest.cc
new file mode 100644
index 0000000..de791c9c0
--- /dev/null
+++ b/chrome/browser/status_icons/status_icon_unittest.cc
@@ -0,0 +1,47 @@
+// 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/status_icons/status_icon.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class MockStatusIconObserver : public StatusIcon::StatusIconObserver {
+ public:
+  MOCK_METHOD0(OnClicked, void());
+};
+
+// Define pure virtual functions so we can test base class functionality.
+class TestStatusIcon : public StatusIcon {
+ public:
+  TestStatusIcon() {}
+  virtual void SetImage(const SkBitmap& image) {}
+  virtual void SetPressedImage(const SkBitmap& image) {}
+  virtual void SetToolTip(const string16& tool_tip) {}
+};
+
+TEST(StatusIconTest, ObserverAdd) {
+  // Make sure that observers are invoked when we click items.
+  TestStatusIcon icon;
+  MockStatusIconObserver observer, observer2;
+  EXPECT_CALL(observer, OnClicked()).Times(2);
+  EXPECT_CALL(observer2, OnClicked());
+  icon.AddObserver(&observer);
+  icon.DispatchClickEvent();
+  icon.AddObserver(&observer2);
+  icon.DispatchClickEvent();
+  icon.RemoveObserver(&observer);
+  icon.RemoveObserver(&observer2);
+}
+
+TEST(StatusIconTest, ObserverRemove) {
+  // Make sure that observers are no longer invoked after they are removed.
+  TestStatusIcon icon;
+  MockStatusIconObserver observer;
+  EXPECT_CALL(observer, OnClicked());
+  icon.AddObserver(&observer);
+  icon.DispatchClickEvent();
+  icon.RemoveObserver(&observer);
+  icon.DispatchClickEvent();
+}
+
diff --git a/chrome/browser/status_icons/status_tray.cc b/chrome/browser/status_icons/status_tray.cc
index 1415babe..b7ae49c 100644
--- a/chrome/browser/status_icons/status_tray.cc
+++ b/chrome/browser/status_icons/status_tray.cc
@@ -7,14 +7,18 @@
 #include "base/stl_util-inl.h"
 #include "chrome/browser/status_icons/status_icon.h"
 
-StatusTray::StatusTray(StatusIconFactory* factory)
-    : factory_(factory) {
+StatusTray::StatusTray() {
 }
 
 StatusTray::~StatusTray() {
+  RemoveAllIcons();
+}
+
+void StatusTray::RemoveAllIcons() {
   // Walk any active status icons and delete them.
   STLDeleteContainerPairSecondPointers(status_icons_.begin(),
                                        status_icons_.end());
+  status_icons_.clear();
 }
 
 StatusIcon* StatusTray::GetStatusIcon(const string16& identifier) {
@@ -23,7 +27,7 @@ StatusIcon* StatusTray::GetStatusIcon(const string16& identifier) {
     return iter->second;
 
   // No existing StatusIcon, create a new one.
-  StatusIcon* icon = factory_->CreateIcon();
+  StatusIcon* icon = CreateStatusIcon();
   if (icon)
     status_icons_[identifier] = icon;
   return icon;
diff --git a/chrome/browser/status_icons/status_tray.h b/chrome/browser/status_icons/status_tray.h
index 663bfa8..30a60aa 100644
--- a/chrome/browser/status_icons/status_tray.h
+++ b/chrome/browser/status_icons/status_tray.h
@@ -10,20 +10,15 @@
 
 class StatusIcon;
 
-// Factory interface for creating icons to improve testability.
-class StatusIconFactory {
- public:
-  virtual StatusIcon* CreateIcon() = 0;
-  virtual ~StatusIconFactory() { }
-};
-
 // Provides a cross-platform interface to the system's status tray, and exposes
 // APIs to add/remove icons to the tray and attach context menus.
 class StatusTray {
  public:
-  // Takes ownership of |factory|.
-  explicit StatusTray(StatusIconFactory* factory);
-  ~StatusTray();
+  // Static factory method that is implemented separately for each platform to
+  // produce the appropriate platform-specific instance.
+  static StatusTray* Create();
+
+  virtual ~StatusTray();
 
   // Gets the current status icon associated with this identifier, or creates
   // a new one if none exists. The StatusTray retains ownership of the
@@ -33,15 +28,27 @@ class StatusTray {
   // Removes the current status icon associated with this identifier, if any.
   void RemoveStatusIcon(const string16& identifier);
 
- private:
+ protected:
+  StatusTray();
+  // Factory method for creating a status icon.
+  virtual StatusIcon* CreateStatusIcon() = 0;
+
+  // Removes all StatusIcons (used by derived classes to clean up in case they
+  // track external state used by the StatusIcons).
+  void RemoveAllIcons();
+
   typedef base::hash_map<string16, StatusIcon*> StatusIconMap;
+  // Returns the list of active status icons so subclasses can operate on them.
+  const StatusIconMap& status_icons() { return status_icons_; }
+
+ private:
   // Map containing all active StatusIcons.
   // Key: String identifiers (passed in to GetStatusIcon)
   // Value: The StatusIcon associated with that identifier (strong pointer -
   //   StatusIcons are freed when the StatusTray destructor is called).
   StatusIconMap status_icons_;
 
-  scoped_ptr<StatusIconFactory> factory_;
+  DISALLOW_COPY_AND_ASSIGN(StatusTray);
 };
 
 #endif  // CHROME_BROWSER_STATUS_ICONS_STATUS_TRAY_H_
diff --git a/chrome/browser/status_icons/status_tray_manager.cc b/chrome/browser/status_icons/status_tray_manager.cc
index f0ef268..39b6c85 100644
--- a/chrome/browser/status_icons/status_tray_manager.cc
+++ b/chrome/browser/status_icons/status_tray_manager.cc
@@ -14,21 +14,6 @@
 #include "grit/browser_resources.h"
 #include "grit/theme_resources.h"
 
-class StatusIconFactoryImpl : public StatusIconFactory {
- public:
-  virtual StatusIcon* CreateIcon();
-};
-
-StatusIcon* StatusIconFactoryImpl::CreateIcon() {
-#ifdef OS_MACOSX
-  return StatusIcon::Create();
-#else
-  // TODO(atwilson): Add support for non-Mac platforms.
-  return 0;
-#endif
-}
-
-
 StatusTrayManager::StatusTrayManager() {
 }
 
@@ -36,9 +21,10 @@ StatusTrayManager::~StatusTrayManager() {
 }
 
 void StatusTrayManager::Init(Profile* profile) {
+#if !defined(OS_LINUX)
   DCHECK(profile);
   profile_ = profile;
-  status_tray_.reset(new StatusTray(new StatusIconFactoryImpl()));
+  status_tray_.reset(StatusTray::Create());
   StatusIcon* icon = status_tray_->GetStatusIcon(ASCIIToUTF16("chrome_main"));
   if (icon) {
     // Create an icon and add ourselves as a click observer on it
@@ -50,6 +36,7 @@ void StatusTrayManager::Init(Profile* profile) {
     icon->SetPressedImage(*pressed);
     icon->AddObserver(this);
   }
+#endif
 }
 
 void StatusTrayManager::OnClicked() {
diff --git a/chrome/browser/status_icons/status_tray_unittest.cc b/chrome/browser/status_icons/status_tray_unittest.cc
index e224fac..538a132 100644
--- a/chrome/browser/status_icons/status_tray_unittest.cc
+++ b/chrome/browser/status_icons/status_tray_unittest.cc
@@ -18,34 +18,31 @@ class MockStatusIcon : public StatusIcon {
   virtual void RemoveObserver(StatusIcon::StatusIconObserver* observer) {}
 };
 
-class TestStatusIconFactory : public StatusIconFactory {
+class TestStatusTray : public StatusTray {
  public:
-  MOCK_METHOD0(CreateIcon, StatusIcon*());
+  MOCK_METHOD0(CreateStatusIcon, StatusIcon*());
 };
 
 TEST(StatusTrayTest, Create) {
   // Check for creation and leaks.
-  TestStatusIconFactory* factory = new TestStatusIconFactory();
-  StatusTray tray(factory);
-  EXPECT_CALL(*factory, CreateIcon()).WillOnce(Return(new MockStatusIcon()));
+  TestStatusTray tray;
+  EXPECT_CALL(tray, CreateStatusIcon()).WillOnce(Return(new MockStatusIcon()));
   tray.GetStatusIcon(ASCIIToUTF16("test"));
 }
 
 TEST(StatusTrayTest, GetIconTwice) {
-  TestStatusIconFactory* factory = new TestStatusIconFactory();
-  StatusTray tray(factory);
+  TestStatusTray tray;
   string16 id = ASCIIToUTF16("test");
   // We should not try to create a new icon if we get the same ID twice.
-  EXPECT_CALL(*factory, CreateIcon()).WillOnce(Return(new MockStatusIcon()));
+  EXPECT_CALL(tray, CreateStatusIcon()).WillOnce(Return(new MockStatusIcon()));
   StatusIcon* icon =  tray.GetStatusIcon(id);
   EXPECT_EQ(icon, tray.GetStatusIcon(id));
 }
 
 TEST(StatusTrayTest, GetIconAfterRemove) {
-  TestStatusIconFactory* factory = new TestStatusIconFactory();
-  StatusTray tray(factory);
+  TestStatusTray tray;
   string16 id = ASCIIToUTF16("test");
-  EXPECT_CALL(*factory, CreateIcon()).Times(2)
+  EXPECT_CALL(tray, CreateStatusIcon()).Times(2)
       .WillOnce(Return(new MockStatusIcon()))
       .WillOnce(Return(new MockStatusIcon()));
   StatusIcon* icon =  tray.GetStatusIcon(id);