diff options
10 files changed, 441 insertions, 423 deletions
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc index c1edaba..86cfb1f 100644 --- a/chrome/browser/background/background_contents_service.cc +++ b/chrome/browser/background/background_contents_service.cc @@ -108,18 +108,16 @@ class CrashNotificationDelegate : public NotificationDelegate { void ShowBalloon(const Extension* extension, Profile* profile) { #if defined(ENABLE_NOTIFICATIONS) + string16 title; // no notifiaction title string16 message = l10n_util::GetStringFUTF16( extension->is_app() ? IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE : IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE, UTF8ToUTF16(extension->name())); - string16 content_url = DesktopNotificationService::CreateDataUrl( - extension->GetIconURL(ExtensionIconSet::EXTENSION_ICON_SMALLISH, - ExtensionIconSet::MATCH_BIGGER), - string16(), message, WebKit::WebTextDirectionDefault); - Notification notification( - extension->url(), GURL(content_url), string16(), string16(), - new CrashNotificationDelegate(profile, extension)); - g_browser_process->notification_ui_manager()->Add(notification, profile); + GURL icon_url(extension->GetIconURL(ExtensionIconSet::EXTENSION_ICON_SMALLISH, + ExtensionIconSet::MATCH_BIGGER)); + DesktopNotificationService::AddNotification( + extension->url(), title, message, icon_url, + new CrashNotificationDelegate(profile, extension), profile); #endif } diff --git a/chrome/browser/chromeos/extensions/file_browser_notifications.cc b/chrome/browser/chromeos/extensions/file_browser_notifications.cc index 946337d..c0ac6ea 100644 --- a/chrome/browser/chromeos/extensions/file_browser_notifications.cc +++ b/chrome/browser/chromeos/extensions/file_browser_notifications.cc @@ -4,24 +4,200 @@ #include "chrome/browser/chromeos/extensions/file_browser_notifications.h" +#include "ash/ash_switches.h" #include "base/bind.h" +#include "base/command_line.h" #include "base/message_loop.h" +#include "base/stl_util.h" #include "base/string_number_conversions.h" #include "base/utf_string_conversions.h" #include "chrome/browser/chromeos/notifications/system_notification.h" +#include "chrome/browser/notifications/desktop_notification_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/webui/web_ui_util.h" #include "content/public/browser/browser_thread.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" #include "ui/base/l10n/l10n_util.h" +#include "ui/base/resource/resource_bundle.h" + +namespace { + +int GetIconId(FileBrowserNotifications::NotificationType type) { + switch (type) { + case FileBrowserNotifications::DEVICE: + case FileBrowserNotifications::FORMAT_SUCCESS: + case FileBrowserNotifications::FORMAT_START: + return IDR_PAGEINFO_INFO; + case FileBrowserNotifications::DEVICE_FAIL: + case FileBrowserNotifications::FORMAT_START_FAIL: + case FileBrowserNotifications::FORMAT_FAIL: + return IDR_PAGEINFO_WARNING_MAJOR; + default: + NOTREACHED(); + return 0; + } +} + +string16 GetTitle(FileBrowserNotifications::NotificationType type) { + int id; + switch (type) { + case FileBrowserNotifications::DEVICE: + case FileBrowserNotifications::DEVICE_FAIL: + id = IDS_REMOVABLE_DEVICE_DETECTION_TITLE; + break; + case FileBrowserNotifications::FORMAT_START: + id = IDS_FORMATTING_OF_DEVICE_PENDING_TITLE; + break; + case FileBrowserNotifications::FORMAT_START_FAIL: + case FileBrowserNotifications::FORMAT_SUCCESS: + case FileBrowserNotifications::FORMAT_FAIL: + id = IDS_FORMATTING_OF_DEVICE_FINISHED_TITLE; + break; + default: + NOTREACHED(); + id = 0; + } + return l10n_util::GetStringUTF16(id); +} + +string16 GetMessage(FileBrowserNotifications::NotificationType type) { + int id; + switch (type) { + case FileBrowserNotifications::DEVICE: + id = IDS_REMOVABLE_DEVICE_SCANNING_MESSAGE; + break; + case FileBrowserNotifications::DEVICE_FAIL: + id = IDS_DEVICE_UNSUPPORTED_DEFAULT_MESSAGE; + break; + case FileBrowserNotifications::FORMAT_FAIL: + id = IDS_FORMATTING_FINISHED_FAILURE_MESSAGE; + break; + case FileBrowserNotifications::FORMAT_SUCCESS: + id = IDS_FORMATTING_FINISHED_SUCCESS_MESSAGE; + break; + case FileBrowserNotifications::FORMAT_START: + id = IDS_FORMATTING_OF_DEVICE_PENDING_MESSAGE; + break; + case FileBrowserNotifications::FORMAT_START_FAIL: + id = IDS_FORMATTING_STARTED_FAILURE_MESSAGE; + break; + default: + NOTREACHED(); + id = 0; + } + return l10n_util::GetStringUTF16(id); +} + +} // namespace + +// Simple wrapper class to support ether old chromeos SystemNotifications or +// new ash notifications. +class FileBrowserNotifications::NotificationMessage { + public: + class Delegate : public NotificationDelegate { + public: + Delegate(const base::WeakPtr<FileBrowserNotifications>& host, + const std::string& id) + : host_(host), + id_(id) {} + virtual ~Delegate() {} + virtual void Display() OVERRIDE {} + virtual void Error() OVERRIDE {} + virtual void Close(bool by_user) OVERRIDE { + if (host_) + host_->HideNotificationById(id_); + } + virtual void Click() OVERRIDE { + // TODO(tbarzic): Show more info page once we have one. + } + virtual std::string id() const OVERRIDE { return id_; } + virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE { + return NULL; + } + + private: + base::WeakPtr<FileBrowserNotifications> host_; + std::string id_; + + DISALLOW_COPY_AND_ASSIGN(Delegate); + }; + + NotificationMessage(FileBrowserNotifications* host, + Profile* profile, + NotificationType type, + const std::string& notification_id, + const string16& message) + : profile_(profile), + type_(type), + notification_id_(notification_id), + message_(message) { + if (CommandLine::ForCurrentProcess()->HasSwitch( + ash::switches::kAshNotify)) { + const gfx::ImageSkia& icon = + *ResourceBundle::GetSharedInstance().GetImageSkiaNamed( + GetIconId(type_)); + notification_id_ = DesktopNotificationService::AddIconNotification( + GURL(), GetTitle(type_), message, icon, + new Delegate(host->AsWeakPtr(), notification_id_), profile_); + } else { + system_notification_.reset( + new chromeos::SystemNotification(profile_, + notification_id_, + GetIconId(type_), + GetTitle(type_))); + } + } + + ~NotificationMessage() { + if (system_notification_.get() && system_notification_->visible()) + system_notification_->Hide(); + } + + void Show() { + if (system_notification_.get()) + system_notification_->Show(message_, false, false); + // Ash notifications are shown automatically (only) when created. + } + + void set_message(const string16& message) { message_ = message; } + + private: + Profile* profile_; + NotificationType type_; + std::string notification_id_; + string16 message_; + scoped_ptr<chromeos::SystemNotification> system_notification_; + + DISALLOW_COPY_AND_ASSIGN(NotificationMessage); +}; + +struct FileBrowserNotifications::MountRequestsInfo { + bool mount_success_exists; + bool fail_message_finalized; + bool fail_notification_shown; + bool non_parent_device_failed; + bool device_notification_hidden; + int fail_notifications_count; + + MountRequestsInfo() : mount_success_exists(false), + fail_message_finalized(false), + fail_notification_shown(false), + non_parent_device_failed(false), + device_notification_hidden(false), + fail_notifications_count(0) { + } +}; FileBrowserNotifications::FileBrowserNotifications(Profile* profile) : profile_(profile) { } FileBrowserNotifications::~FileBrowserNotifications() { + STLDeleteContainerPairSecondPointers(notification_map_.begin(), + notification_map_.end()); } void FileBrowserNotifications::RegisterDevice(const std::string& path) { @@ -113,215 +289,111 @@ void FileBrowserNotifications::ManageNotificationsOnMountCompleted( void FileBrowserNotifications::ShowNotification(NotificationType type, const std::string& path) { - ShowNotificationWithMessage(type, path, - l10n_util::GetStringUTF16(GetMessageId(type))); + ShowNotificationWithMessage(type, path, GetMessage(type)); } void FileBrowserNotifications::ShowNotificationWithMessage( - NotificationType type, const std::string& path, const string16& message) { - std::string notification_id; - CreateNotificationId(type, path, ¬ification_id); - chromeos::SystemNotification* notification = - CreateNotification(notification_id, GetIconId(type), GetTitleId(type)); - if (HasMoreInfoLink(type)) { - notification->Show(message, GetLinkText(), GetLinkCallback(), false, false); - } else { - notification->Show(message, false, false); - } + NotificationType type, + const std::string& path, + const string16& message) { + std::string notification_id = CreateNotificationId(type, path); + hidden_notifications_.erase(notification_id); + ShowNotificationById(type, notification_id, message); } void FileBrowserNotifications::ShowNotificationDelayed( - NotificationType type, const std::string& path, base::TimeDelta delay) { - std::string notification_id; - CreateNotificationId(type, path, ¬ification_id); - CreateNotification(notification_id, GetIconId(type), GetTitleId(type)); - - PostDelayedShowNotificationTask(notification_id, type, - l10n_util::GetStringUTF16(GetMessageId(type)), - delay); + NotificationType type, + const std::string& path, + base::TimeDelta delay) { + std::string notification_id = CreateNotificationId(type, path); + hidden_notifications_.erase(notification_id); + MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&FileBrowserNotifications::ShowNotificationById, AsWeakPtr(), + type, notification_id, GetMessage(type)), + delay); } void FileBrowserNotifications::HideNotification(NotificationType type, const std::string& path) { - std::string notification_id; - CreateNotificationId(type, path, ¬ification_id); - NotificationMap::iterator it = notifications_.find(notification_id); - if (it != notifications_.end()) { - if (it->second->visible()) - it->second->Hide(); - notifications_.erase(it); - } + std::string notification_id = CreateNotificationId(type, path); + HideNotificationById(notification_id); } void FileBrowserNotifications::HideNotificationDelayed( NotificationType type, const std::string& path, base::TimeDelta delay) { - PostDelayedHideNotificationTask(type, path, delay); -} - -void FileBrowserNotifications::PostDelayedShowNotificationTask( - const std::string& notification_id, NotificationType type, - const string16& message, base::TimeDelta delay) { - MessageLoop::current()->PostDelayedTask(FROM_HERE, - base::Bind(&ShowNotificationDelayedTask, notification_id, type, - message, AsWeakPtr()), + MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&FileBrowserNotifications::HideNotification, AsWeakPtr(), + type, path), delay); } -// static -void FileBrowserNotifications::ShowNotificationDelayedTask( - const std::string& notification_id, NotificationType type, - const string16& message, - base::WeakPtr<FileBrowserNotifications> self) { - if (!self) - return; - - NotificationMap::iterator it = self->notifications_.find(notification_id); - if (it != self->notifications_.end()) { - if (self->HasMoreInfoLink(type)) { - it->second->Show(message, self->GetLinkText(), self->GetLinkCallback(), - false, false); - } else { - it->second->Show(message, false, false); - } - } -} - -void FileBrowserNotifications::PostDelayedHideNotificationTask( - NotificationType type, const std::string path, base::TimeDelta delay) { - MessageLoop::current()->PostDelayedTask(FROM_HERE, - base::Bind(&HideNotificationDelayedTask, type, path, AsWeakPtr()), - delay); -} - -// static -void FileBrowserNotifications::HideNotificationDelayedTask( - NotificationType type, const std::string& path, - base::WeakPtr<FileBrowserNotifications> self) { - if (self) - self->HideNotification(type, path); -} - -chromeos::SystemNotification* FileBrowserNotifications::CreateNotification( - const std::string& notification_id, int icon_id, int title_id) { - if (!profile_) { - NOTREACHED(); - return NULL; - } - - NotificationMap::iterator it = notifications_.find(notification_id); - if (it != notifications_.end()) - return it->second.get(); - - chromeos::SystemNotification* notification = - new chromeos::SystemNotification( - profile_, - notification_id, - icon_id, - l10n_util::GetStringUTF16(title_id)); - - notifications_.insert(NotificationMap::value_type(notification_id, - linked_ptr<chromeos::SystemNotification>(notification))); - - return notification; -} - -void FileBrowserNotifications::CreateNotificationId(NotificationType type, - const std::string& path, - std::string* id) { +std::string FileBrowserNotifications::CreateNotificationId( + NotificationType type, + const std::string& path) { + std::string id; switch (type) { - case(DEVICE): - *id = "D"; + case DEVICE: + id = "D"; break; - case(DEVICE_FAIL): - *id = "DF"; + case DEVICE_FAIL: + id = "DF"; break; - case(FORMAT_START): - *id = "FS"; + case FORMAT_START: + id = "FS"; break; default: - *id = "FF"; + id = "FF"; } if (type == DEVICE_FAIL) { MountRequestsMap::const_iterator it = mount_requests_.find(path); if (it != mount_requests_.end()) - id->append(base::IntToString(it->second.fail_notifications_count)); + id.append(base::IntToString(it->second.fail_notifications_count)); } - id->append(path); + id.append(path); + return id; } -int FileBrowserNotifications::GetIconId(NotificationType type) { - switch (type) { - case(DEVICE): - case(FORMAT_SUCCESS): - case(FORMAT_START): - return IDR_PAGEINFO_INFO; - case(DEVICE_FAIL): - case(FORMAT_START_FAIL): - case(FORMAT_FAIL): - return IDR_PAGEINFO_WARNING_MAJOR; - default: - NOTREACHED(); - return 0; +FileBrowserNotifications::NotificationMessage* +FileBrowserNotifications::GetNotification(NotificationType type, + const std::string& notification_id, + const string16& message) { + NotificationMap::iterator iter = notification_map_.find(notification_id); + if (iter != notification_map_.end()) { + iter->second->set_message(message); + return iter->second; } + NotificationMessage* new_message = + new NotificationMessage(this, profile_, type, notification_id, message); + notification_map_[notification_id] = new_message; + return new_message; } -int FileBrowserNotifications::GetTitleId(NotificationType type) { - switch (type) { - case(DEVICE): - case(DEVICE_FAIL): - return IDS_REMOVABLE_DEVICE_DETECTION_TITLE; - case(FORMAT_START): - return IDS_FORMATTING_OF_DEVICE_PENDING_TITLE; - case(FORMAT_START_FAIL): - case(FORMAT_SUCCESS): - case(FORMAT_FAIL): - return IDS_FORMATTING_OF_DEVICE_FINISHED_TITLE; - default: - NOTREACHED(); - return 0; +void FileBrowserNotifications::ShowNotificationById( + NotificationType type, + const std::string& notification_id, + const string16& message) { + if (hidden_notifications_.find(notification_id) != + hidden_notifications_.end()) { + // Notification was hidden after a delayed show was requested. + hidden_notifications_.erase(notification_id); + return; } + NotificationMessage* notification = + GetNotification(type, notification_id, message); + notification->Show(); } -int FileBrowserNotifications::GetMessageId(NotificationType type) { - switch (type) { - case(DEVICE): - return IDS_REMOVABLE_DEVICE_SCANNING_MESSAGE; - case(DEVICE_FAIL): - return IDS_DEVICE_UNSUPPORTED_DEFAULT_MESSAGE; - case(FORMAT_FAIL): - return IDS_FORMATTING_FINISHED_FAILURE_MESSAGE; - case(FORMAT_SUCCESS): - return IDS_FORMATTING_FINISHED_SUCCESS_MESSAGE; - case(FORMAT_START): - return IDS_FORMATTING_OF_DEVICE_PENDING_MESSAGE; - case(FORMAT_START_FAIL): - return IDS_FORMATTING_STARTED_FAILURE_MESSAGE; - default: - NOTREACHED(); - return 0; +void FileBrowserNotifications::HideNotificationById(const std::string& id) { + NotificationMap::iterator it = notification_map_.find(id); + if (it != notification_map_.end()) { + delete it->second; + notification_map_.erase(it); + } else { + // Mark as hidden so it does not get shown from a delayed task. + hidden_notifications_.insert(id); } } - -void FileBrowserNotifications::OnLinkClicked(const ListValue* args) { - // TODO(tbarzic): Show more info page when we'll have one. - NOTREACHED(); -} - -bool FileBrowserNotifications::HasMoreInfoLink(NotificationType type) { - // TODO(tbarzic): Make this return type == DEVICE_FAIL; when more info page - // gets defined. - return false; -} - -const string16& FileBrowserNotifications::GetLinkText() { - if (link_text_.empty()) - link_text_ = l10n_util::GetStringUTF16(IDS_LEARN_MORE); - return link_text_; -} - -chromeos::BalloonViewHost::MessageCallback -FileBrowserNotifications::GetLinkCallback() { - return base::Bind(&FileBrowserNotifications::OnLinkClicked, AsWeakPtr()); -} diff --git a/chrome/browser/chromeos/extensions/file_browser_notifications.h b/chrome/browser/chromeos/extensions/file_browser_notifications.h index c646581..039264a 100644 --- a/chrome/browser/chromeos/extensions/file_browser_notifications.h +++ b/chrome/browser/chromeos/extensions/file_browser_notifications.h @@ -6,13 +6,13 @@ #define CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_NOTIFICATIONS_H_ #include <map> +#include <set> #include <string> #include "base/basictypes.h" -#include "base/memory/linked_ptr.h" #include "base/memory/weak_ptr.h" #include "base/string16.h" -#include "chrome/browser/chromeos/notifications/system_notification.h" +#include "base/time.h" class Profile; @@ -31,9 +31,6 @@ class FileBrowserNotifications GDATA_SYNC_FAIL, }; - typedef std::map<std::string, linked_ptr<chromeos::SystemNotification> > - NotificationMap; - explicit FileBrowserNotifications(Profile* profile); virtual ~FileBrowserNotifications(); @@ -49,76 +46,48 @@ class FileBrowserNotifications void ManageNotificationOnGDataSyncProgress(int count); void ManageNotificationOnGDataSyncFinish(bool success); + // Retreives message body based on |type|. void ShowNotification(NotificationType type, const std::string& path); - void ShowNotificationDelayed(NotificationType type, - const std::string& path, - base::TimeDelta delay); + // Primary method for showing a notification. Virtual for mock in unittest. virtual void ShowNotificationWithMessage(NotificationType type, const std::string& path, const string16& message); + void ShowNotificationDelayed(NotificationType type, + const std::string& path, + base::TimeDelta delay); + // Primary method for hiding a notification. Virtual for mock in unittest. virtual void HideNotification(NotificationType type, const std::string& path); void HideNotificationDelayed(NotificationType type, const std::string& path, base::TimeDelta delay); - const NotificationMap& notifications() const { return notifications_; } - - protected: - virtual void PostDelayedShowNotificationTask( - const std::string& notification_id, - NotificationType type, - const string16& message, - base::TimeDelta delay); - static void ShowNotificationDelayedTask(const std::string& notification_id, - NotificationType type, - const string16& message, - base::WeakPtr<FileBrowserNotifications> self); - - virtual void PostDelayedHideNotificationTask( - NotificationType type, const std::string path, base::TimeDelta delay); - static void HideNotificationDelayedTask(NotificationType type, - const std::string& path, - base::WeakPtr<FileBrowserNotifications> self); + size_t GetNotificationCountForTest() const { + return notification_map_.size(); + } + bool HasNotificationForTest(const std::string& id) const { + return notification_map_.find(id) != notification_map_.end(); + } private: - struct MountRequestsInfo { - bool mount_success_exists; - bool fail_message_finalized; - bool fail_notification_shown; - bool non_parent_device_failed; - bool device_notification_hidden; - int fail_notifications_count; - - MountRequestsInfo() : mount_success_exists(false), - fail_message_finalized(false), - fail_notification_shown(false), - non_parent_device_failed(false), - device_notification_hidden(false), - fail_notifications_count(0) { - } - }; + class NotificationMessage; + struct MountRequestsInfo; typedef std::map<std::string, MountRequestsInfo> MountRequestsMap; - - chromeos::SystemNotification* CreateNotification(const std::string& id, - int title_id, - int icon_id); - - void CreateNotificationId(NotificationType type, - const std::string& path, - std::string* id); - int GetMessageId(NotificationType type); - int GetIconId(NotificationType type); - int GetTitleId(NotificationType type); - - void OnLinkClicked(const base::ListValue* arg); - bool HasMoreInfoLink(NotificationType type); - const string16& GetLinkText(); - chromeos::BalloonViewHost::MessageCallback GetLinkCallback(); - - string16 link_text_; - NotificationMap notifications_; + typedef std::map<std::string, NotificationMessage*> NotificationMap; + + std::string CreateNotificationId(NotificationType type, + const std::string& path); + NotificationMessage* GetNotification(NotificationType type, + const std::string& path, + const string16& message); + void ShowNotificationById(NotificationType type, + const std::string& notification_id, + const string16& message); + void HideNotificationById(const std::string& id); + + NotificationMap notification_map_; + std::set<std::string> hidden_notifications_; MountRequestsMap mount_requests_; Profile* profile_; diff --git a/chrome/browser/chromeos/extensions/file_browser_notifications_browsertest.cc b/chrome/browser/chromeos/extensions/file_browser_notifications_browsertest.cc index 3d6276a..58a02d7 100644 --- a/chrome/browser/chromeos/extensions/file_browser_notifications_browsertest.cc +++ b/chrome/browser/chromeos/extensions/file_browser_notifications_browsertest.cc @@ -9,8 +9,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/ui/browser.h" -#include "chrome/browser/notifications/balloon.h" -#include "chrome/browser/notifications/balloon_collection.h" #include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/test/base/in_process_browser_test.h" @@ -18,63 +16,9 @@ namespace chromeos { -class MockFileBrowserNotifications : public FileBrowserNotifications { - public: - explicit MockFileBrowserNotifications(Profile* profile) - : FileBrowserNotifications(profile) { - } - virtual ~MockFileBrowserNotifications() {} - - // Records the notification so we can force it to show later. - virtual void PostDelayedShowNotificationTask( - const std::string& notification_id, - NotificationType type, - const string16& message, - base::TimeDelta delay) OVERRIDE { - show_callback_data_.id = notification_id; - show_callback_data_.type = type; - show_callback_data_.message = message; - } - - // Records the notification so we can force it to hide later. - virtual void PostDelayedHideNotificationTask(NotificationType type, - const std::string path, - base::TimeDelta delay) OVERRIDE { - hide_callback_data_.type = type; - hide_callback_data_.path = path; - } - - void ExecuteShow() { - FileBrowserNotifications::ShowNotificationDelayedTask( - show_callback_data_.id, show_callback_data_.type, - show_callback_data_.message, AsWeakPtr()); - } - - void ExecuteHide() { - FileBrowserNotifications::HideNotificationDelayedTask( - hide_callback_data_.type, hide_callback_data_.path, AsWeakPtr()); - } - - private: - struct ShowCallbackData { - std::string id; - NotificationType type; - string16 message; - }; - - ShowCallbackData show_callback_data_; - - struct HideCallbackData { - NotificationType type; - std::string path; - }; - - HideCallbackData hide_callback_data_; -}; - class FileBrowserNotificationsTest : public InProcessBrowserTest { public: - FileBrowserNotificationsTest() : collection_(NULL) {} + FileBrowserNotificationsTest() {} virtual void CleanUpOnMainThread() OVERRIDE { notifications_.reset(); @@ -84,171 +28,123 @@ class FileBrowserNotificationsTest : public InProcessBrowserTest { // This must be initialized late in test startup. void InitNotifications() { Profile* profile = browser()->profile(); - notifications_.reset(new MockFileBrowserNotifications(profile)); - collection_ = - g_browser_process->notification_ui_manager()->balloon_collection(); + notifications_.reset(new FileBrowserNotifications(profile)); } bool FindNotification(const std::string& id) { - return notifications_->notifications().find(id) != - notifications_->notifications().end(); + return notifications_->HasNotificationForTest(id); } - bool FindBalloon(const std::string& id) { - const std::deque<Balloon*>& balloons = collection_->GetActiveBalloons(); - for (std::deque<Balloon*>::const_iterator it = balloons.begin(); - it != balloons.end(); - ++it) { - Balloon* balloon = *it; - if (balloon->notification().notification_id() == id) - return true; - } - return false; - } - - BalloonCollection* collection_; - scoped_ptr<MockFileBrowserNotifications> notifications_; + scoped_ptr<FileBrowserNotifications> notifications_; }; IN_PROC_BROWSER_TEST_F(FileBrowserNotificationsTest, TestBasic) { InitNotifications(); - // We start with no balloons. - EXPECT_EQ(0u, collection_->GetActiveBalloons().size()); - - // Showing a notification both updates our data and shows a balloon. + // Showing a notification adds a new notification. notifications_->ShowNotification(FileBrowserNotifications::DEVICE, "path"); - EXPECT_EQ(1u, notifications_->notifications().size()); + EXPECT_EQ(1u, notifications_->GetNotificationCountForTest()); EXPECT_TRUE(FindNotification("Dpath")); - EXPECT_EQ(1u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - // Updating the same notification maintains the same balloon. + // Updating the same notification maintains the same count. notifications_->ShowNotification(FileBrowserNotifications::DEVICE, "path"); - EXPECT_EQ(1u, notifications_->notifications().size()); + EXPECT_EQ(1u, notifications_->GetNotificationCountForTest()); EXPECT_TRUE(FindNotification("Dpath")); - EXPECT_EQ(1u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - // A new notification adds a new balloon. + // A new notification increases the count. notifications_->ShowNotification(FileBrowserNotifications::DEVICE_FAIL, "path"); - EXPECT_EQ(2u, notifications_->notifications().size()); + EXPECT_EQ(2u, notifications_->GetNotificationCountForTest()); EXPECT_TRUE(FindNotification("DFpath")); EXPECT_TRUE(FindNotification("Dpath")); - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("DFpath")); - EXPECT_TRUE(FindBalloon("Dpath")); // Hiding a notification removes it from our data. notifications_->HideNotification(FileBrowserNotifications::DEVICE_FAIL, "path"); - EXPECT_EQ(1u, notifications_->notifications().size()); + EXPECT_EQ(1u, notifications_->GetNotificationCountForTest()); EXPECT_FALSE(FindNotification("DFpath")); EXPECT_TRUE(FindNotification("Dpath")); - - // Balloons don't go away until we run the message loop. - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("DFpath")); - EXPECT_TRUE(FindBalloon("Dpath")); - - // Running the message loop allows the balloon to disappear. - ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(1u, collection_->GetActiveBalloons().size()); - EXPECT_FALSE(FindBalloon("DFpath")); - EXPECT_TRUE(FindBalloon("Dpath")); }; -// TODO(jamescook): This test is flaky on linux_chromeos, occasionally causing -// this assertion failure inside Gtk: -// "murrine_style_draw_box: assertion `height >= -1' failed" -// There may be an underlying bug in the ChromeOS notification code. -// I'm not marking it as FLAKY because this doesn't happen on the bots. -#define MAYBE_ShowDelayedTest ShowDelayedTest +// Note: Delayed tests use a delay time of 0 so that tasks wille execute +// when RunAllPendingInMessageLoop() is called. IN_PROC_BROWSER_TEST_F(FileBrowserNotificationsTest, ShowDelayedTest) { InitNotifications(); - // Adding a delayed notification does not show a balloon. + // Adding a delayed notification does not create a notification. notifications_->ShowNotificationDelayed(FileBrowserNotifications::DEVICE, "path", - base::TimeDelta::FromSeconds(3)); - EXPECT_EQ(0u, collection_->GetActiveBalloons().size()); + base::TimeDelta::FromSeconds(0)); + EXPECT_EQ(0u, notifications_->GetNotificationCountForTest()); + EXPECT_FALSE(FindNotification("Dpath")); - // Forcing the show to happen makes the balloon appear. - notifications_->ExecuteShow(); + // Running the message loop should create the notification. ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(1u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); + EXPECT_EQ(1u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("Dpath")); // Showing a notification both immediately and delayed results in one - // additional balloon. + // additional notification. notifications_->ShowNotificationDelayed(FileBrowserNotifications::DEVICE_FAIL, "path", - base::TimeDelta::FromSeconds(3)); + base::TimeDelta::FromSeconds(0)); notifications_->ShowNotification(FileBrowserNotifications::DEVICE_FAIL, "path"); - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - EXPECT_TRUE(FindBalloon("DFpath")); + EXPECT_EQ(2u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("DFpath")); - // When the delayed notification arrives, it's an update, so we still only - // have two balloons. - notifications_->ExecuteShow(); + // When the delayed notification is processed, it's an update, so we still + // only have two notifications. ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - EXPECT_TRUE(FindBalloon("DFpath")); + EXPECT_EQ(2u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("DFpath")); // If we schedule a show for later, then hide before it becomes visible, - // the balloon should not be added. + // the notification should not be added. notifications_->ShowNotificationDelayed(FileBrowserNotifications::FORMAT_FAIL, "path", - base::TimeDelta::FromSeconds(3)); + base::TimeDelta::FromSeconds(0)); notifications_->HideNotification(FileBrowserNotifications::FORMAT_FAIL, "path"); - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - EXPECT_TRUE(FindBalloon("DFpath")); - EXPECT_FALSE(FindBalloon("Fpath")); - - // Even when we try to force the show, nothing appears, because the balloon - // was explicitly hidden. - notifications_->ExecuteShow(); + EXPECT_EQ(2u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("Dpath")); + EXPECT_TRUE(FindNotification("DFpath")); + EXPECT_FALSE(FindNotification("Fpath")); + + // Even after processing messages, no new notification should be added. ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(2u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); - EXPECT_TRUE(FindBalloon("DFpath")); - EXPECT_FALSE(FindBalloon("Fpath")); + EXPECT_EQ(2u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("Dpath")); + EXPECT_TRUE(FindNotification("DFpath")); + EXPECT_FALSE(FindNotification("Fpath")); } IN_PROC_BROWSER_TEST_F(FileBrowserNotificationsTest, HideDelayedTest) { InitNotifications(); - // Showing now, and scheduling a hide for later, results in one balloon. + // Showing now, and scheduling a hide for later, results in one notification. notifications_->ShowNotification(FileBrowserNotifications::DEVICE, "path"); notifications_->HideNotificationDelayed(FileBrowserNotifications::DEVICE, "path", - base::TimeDelta::FromSeconds(3)); - EXPECT_EQ(1u, collection_->GetActiveBalloons().size()); - EXPECT_TRUE(FindBalloon("Dpath")); + base::TimeDelta::FromSeconds(0)); + EXPECT_EQ(1u, notifications_->GetNotificationCountForTest()); + EXPECT_TRUE(FindNotification("Dpath")); - // Forcing the hide removes the balloon. - notifications_->ExecuteHide(); + // Running pending messges should remove the notification. ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(0u, collection_->GetActiveBalloons().size()); + EXPECT_EQ(0u, notifications_->GetNotificationCountForTest()); - // Immediate show then hide results in no balloons. + // Immediate show then hide results in no notification. notifications_->ShowNotification(FileBrowserNotifications::DEVICE_FAIL, "path"); notifications_->HideNotification(FileBrowserNotifications::DEVICE_FAIL, "path"); ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(0u, collection_->GetActiveBalloons().size()); + EXPECT_EQ(0u, notifications_->GetNotificationCountForTest()); // Delayed hide for a notification that doesn't exist does nothing. notifications_->HideNotificationDelayed(FileBrowserNotifications::DEVICE_FAIL, "path", - base::TimeDelta::FromSeconds(3)); - notifications_->ExecuteHide(); + base::TimeDelta::FromSeconds(0)); ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_EQ(0u, collection_->GetActiveBalloons().size()); + EXPECT_EQ(0u, notifications_->GetNotificationCountForTest()); } } // namespace chromeos. diff --git a/chrome/browser/chromeos/extensions/file_browser_notifications_unittest.cc b/chrome/browser/chromeos/extensions/file_browser_notifications_unittest.cc index 0569841..566df51 100644 --- a/chrome/browser/chromeos/extensions/file_browser_notifications_unittest.cc +++ b/chrome/browser/chromeos/extensions/file_browser_notifications_unittest.cc @@ -9,6 +9,7 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/chromeos/extensions/file_browser_notifications.h" +#include "chromeos/dbus/mock_dbus_thread_manager.h" // For SystemNotifications #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -20,6 +21,8 @@ using ::testing::AnyNumber; namespace chromeos { +namespace { + class MockFileBrowserNotificationsOnMount : public FileBrowserNotifications { public: explicit MockFileBrowserNotificationsOnMount(Profile* profile) @@ -37,6 +40,8 @@ MATCHER_P2(String16Equals, id, label, "") { return arg == l10n_util::GetStringFUTF16(id, ASCIIToUTF16(label)); } +} // namespace + TEST(FileBrowserMountNotificationsTest, GoodDevice) { MockFileBrowserNotificationsOnMount* mocked_notifications = new MockFileBrowserNotificationsOnMount(NULL); diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc index d8029f0..c7014a7 100644 --- a/chrome/browser/notifications/desktop_notification_service.cc +++ b/chrome/browser/notifications/desktop_notification_service.cc @@ -24,6 +24,7 @@ #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/tab_contents/tab_contents.h" +#include "chrome/browser/ui/webui/web_ui_util.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/content_settings.h" #include "chrome/common/content_settings_pattern.h" @@ -44,6 +45,19 @@ #include "ui/base/layout.h" #include "ui/base/resource/resource_bundle.h" +#if defined(USE_ASH) +#include "ash/ash_switches.h" +#include "base/command_line.h" + +namespace { + +bool IsAshNotifyEnabled() { + return CommandLine::ForCurrentProcess()->HasSwitch(ash::switches::kAshNotify); +} + +} // namespace +#endif + using content::BrowserThread; using content::RenderViewHost; using content::WebContents; @@ -218,6 +232,58 @@ string16 DesktopNotificationService::CreateDataUrl( net::EscapeQueryParamValue(data, false)); } +// static +std::string DesktopNotificationService::AddNotification( + const GURL& origin_url, + const string16& title, + const string16& message, + const GURL& icon_url, + NotificationDelegate* delegate, + Profile* profile) { +#if defined(USE_ASH) + if (IsAshNotifyEnabled()) { + // For Ash create a non-HTML notification with |icon_url|. + Notification notification(GURL(), icon_url, title, message, + WebKit::WebTextDirectionDefault, + string16(), string16(), delegate); + g_browser_process->notification_ui_manager()->Add(notification, profile); + return notification.notification_id(); + } +#endif + // Generate a data URL embedding the icon URL, title, and message. + GURL content_url(CreateDataUrl( + icon_url, title, message, WebKit::WebTextDirectionDefault)); + Notification notification( + GURL(), content_url, string16(), string16(), delegate); + g_browser_process->notification_ui_manager()->Add(notification, profile); + return notification.notification_id(); +} + +// static +std::string DesktopNotificationService::AddIconNotification( + const GURL& origin_url, + const string16& title, + const string16& message, + const gfx::ImageSkia& icon, + NotificationDelegate* delegate, + Profile* profile) { +#if defined(USE_ASH) + if (IsAshNotifyEnabled()) { + // For Ash create a non-HTML notification with |icon|. + Notification notification(GURL(), icon, title, message, + WebKit::WebTextDirectionDefault, + string16(), string16(), delegate); + g_browser_process->notification_ui_manager()->Add(notification, profile); + return notification.notification_id(); + } +#endif + GURL icon_url; + if (!icon.empty()) + icon_url = GURL(web_ui_util::GetImageDataUrl(icon)); + return AddNotification( + origin_url, title, message, icon_url, delegate, profile); +} + DesktopNotificationService::DesktopNotificationService(Profile* profile, NotificationUIManager* ui_manager) : profile_(profile), diff --git a/chrome/browser/notifications/desktop_notification_service.h b/chrome/browser/notifications/desktop_notification_service.h index 9a5fef7..ec51dcc 100644 --- a/chrome/browser/notifications/desktop_notification_service.h +++ b/chrome/browser/notifications/desktop_notification_service.h @@ -23,6 +23,7 @@ class ContentSettingsPattern; class Notification; +class NotificationDelegate; class NotificationUIManager; class Profile; @@ -31,6 +32,10 @@ class WebContents; struct ShowDesktopNotificationHostMsgParams; } +namespace gfx { +class ImageSkia; +} + // The DesktopNotificationService is an object, owned by the Profile, // which provides the creation of desktop "toasts" to web pages and workers. class DesktopNotificationService : public content::NotificationObserver, @@ -94,6 +99,24 @@ class DesktopNotificationService : public content::NotificationObserver, static string16 CreateDataUrl(int resource, const std::vector<std::string>& subst); + // Add a desktop notification. On non-Ash platforms this will generate a HTML + // notification from the input parameters. On Ash it will generate a normal + // ash notification. Returns the notification id. + static std::string AddNotification(const GURL& origin_url, + const string16& title, + const string16& message, + const GURL& icon_url, + NotificationDelegate* delegate, + Profile* profile); + + // Same as above, but takes a gfx::ImageSkia for the icon instead. + static std::string AddIconNotification(const GURL& origin_url, + const string16& title, + const string16& message, + const gfx::ImageSkia& icon, + NotificationDelegate* delegate, + Profile* profile); + // The default content setting determines how to handle origins that haven't // been allowed or denied yet. If |provider_id| is not NULL, the id of the // provider which provided the default setting is assigned to it. diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc index e171b00..15bc6d7 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc @@ -138,18 +138,14 @@ bool CloudPrintProxyService::ShowTokenExpiredNotification() { if (token_expired_delegate_.get()) return false; - // TODO(sanjeevr): Get icon for this notification. - GURL icon_url; - + // TODO(sanjeevr): Get icon for this notification. crbug.com/132848. string16 title = l10n_util::GetStringUTF16(IDS_GOOGLE_CLOUD_PRINT); string16 message = l10n_util::GetStringFUTF16(IDS_CLOUD_PRINT_TOKEN_EXPIRED_MESSAGE, title); - string16 content_url = DesktopNotificationService::CreateDataUrl( - icon_url, title, message, WebKit::WebTextDirectionDefault); token_expired_delegate_ = new TokenExpiredNotificationDelegate(this); - Notification notification(GURL(), GURL(content_url), string16(), string16(), - token_expired_delegate_.get()); - g_browser_process->notification_ui_manager()->Add(notification, profile_); + DesktopNotificationService::AddNotification( + GURL(), title, message, GURL(), + token_expired_delegate_.get(), profile_); // Keep the browser alive while we are showing the notification. browser::StartKeepAlive(); return true; diff --git a/chrome/browser/status_icons/desktop_notification_balloon.cc b/chrome/browser/status_icons/desktop_notification_balloon.cc index d49a8bb..fb22c91 100644 --- a/chrome/browser/status_icons/desktop_notification_balloon.cc +++ b/chrome/browser/status_icons/desktop_notification_balloon.cc @@ -67,28 +67,22 @@ DesktopNotificationBalloon::DesktopNotificationBalloon() { } DesktopNotificationBalloon::~DesktopNotificationBalloon() { - if (notification_.get()) - CloseBalloon(notification_->notification_id()); + if (!notification_id_.empty()) + CloseBalloon(notification_id_); } void DesktopNotificationBalloon::DisplayBalloon(const SkBitmap& icon, const string16& title, const string16& contents) { - GURL icon_url; - if (!icon.empty()) - icon_url = GURL(web_ui_util::GetImageDataUrl(gfx::ImageSkia(icon))); - - GURL content_url(DesktopNotificationService::CreateDataUrl( - icon_url, title, contents, WebKit::WebTextDirectionDefault)); - - notification_.reset(new Notification( - GURL(), content_url, string16(), string16(), - new DummyNotificationDelegate(base::IntToString(id_count_++)))); - // Allowing IO access is required here to cover the corner case where // there is no last used profile and the default one is loaded. // IO access won't be required for normal uses. - base::ThreadRestrictions::ScopedAllowIO allow_io; - g_browser_process->notification_ui_manager()->Add( - *notification_.get(), ProfileManager::GetLastUsedProfile()); + Profile* profile; + { + base::ThreadRestrictions::ScopedAllowIO allow_io; + profile = ProfileManager::GetLastUsedProfile(); + } + notification_id_ = DesktopNotificationService::AddIconNotification( + GURL(), title, contents, gfx::ImageSkia(icon), + new DummyNotificationDelegate(base::IntToString(id_count_++)), profile); } diff --git a/chrome/browser/status_icons/desktop_notification_balloon.h b/chrome/browser/status_icons/desktop_notification_balloon.h index 2a44f2b..f3b9195 100644 --- a/chrome/browser/status_icons/desktop_notification_balloon.h +++ b/chrome/browser/status_icons/desktop_notification_balloon.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -25,8 +25,7 @@ class DesktopNotificationBalloon { const string16& contents); private: - // Notification balloon. - scoped_ptr<Notification> notification_; + std::string notification_id_; // Counter to provide unique ids to notifications. static int id_count_; |