diff options
author | atwilson@google.com <atwilson@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 21:29:05 +0000 |
---|---|---|
committer | atwilson@google.com <atwilson@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 21:29:05 +0000 |
commit | a24642a800c1712eeaa791f0fb0ed1527885c6fb (patch) | |
tree | 925671bf6414abcc8da68cd48a6085fa0007dd6b /chrome/browser/status_icons | |
parent | 45e9bcaa8bd6ff0eb2bf61d7100b1faacd2ccef4 (diff) | |
download | chromium_src-a24642a800c1712eeaa791f0fb0ed1527885c6fb.zip chromium_src-a24642a800c1712eeaa791f0fb0ed1527885c6fb.tar.gz chromium_src-a24642a800c1712eeaa791f0fb0ed1527885c6fb.tar.bz2 |
Implement status icons on windows.
Refactor existing status icon code to allow platform-specific StatusTray implementation to
track common state for all status icons.
BUG=37375
TEST=new unit tests
Review URL: http://codereview.chromium.org/1136005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42538 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/status_icons')
-rw-r--r-- | chrome/browser/status_icons/status_icon.cc | 30 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_icon.h | 16 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_icon_unittest.cc | 47 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_tray.cc | 10 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_tray.h | 31 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_tray_manager.cc | 19 | ||||
-rw-r--r-- | chrome/browser/status_icons/status_tray_unittest.cc | 19 |
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); |