diff options
Diffstat (limited to 'chrome')
22 files changed, 454 insertions, 100 deletions
diff --git a/chrome/browser/chromeos/power/peripheral_battery_observer_browsertest.cc b/chrome/browser/chromeos/power/peripheral_battery_observer_browsertest.cc index ba280d7..852d11c 100644 --- a/chrome/browser/chromeos/power/peripheral_battery_observer_browsertest.cc +++ b/chrome/browser/chromeos/power/peripheral_battery_observer_browsertest.cc @@ -82,7 +82,7 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, Basic) { EXPECT_EQ(info.name, kTestDeviceName); EXPECT_EQ(info.level, 50); EXPECT_EQ(info.last_notification_timestamp, base::TimeTicks()); - EXPECT_FALSE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_FALSE(notification_manager->FindById(kTestBatteryAddress) != NULL); // Level 5 at time 110, low-battery notification. clock.Advance(base::TimeDelta::FromSeconds(10)); @@ -90,7 +90,7 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, Basic) { kTestDeviceName, 5); EXPECT_EQ(info.level, 5); EXPECT_EQ(info.last_notification_timestamp, clock.NowTicks()); - EXPECT_TRUE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_TRUE(notification_manager->FindById(kTestBatteryAddress) != NULL); // Level -1 at time 115, cancel previous notification clock.Advance(base::TimeDelta::FromSeconds(5)); @@ -99,7 +99,7 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, Basic) { EXPECT_EQ(info.level, 5); EXPECT_EQ(info.last_notification_timestamp, clock.NowTicks() - base::TimeDelta::FromSeconds(5)); - EXPECT_FALSE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_FALSE(notification_manager->FindById(kTestBatteryAddress) != NULL); // Level 50 at time 120, no low-battery notification. clock.Advance(base::TimeDelta::FromSeconds(5)); @@ -108,7 +108,7 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, Basic) { EXPECT_EQ(info.level, 50); EXPECT_EQ(info.last_notification_timestamp, clock.NowTicks() - base::TimeDelta::FromSeconds(10)); - EXPECT_FALSE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_FALSE(notification_manager->FindById(kTestBatteryAddress) != NULL); // Level 5 at time 130, no low-battery notification (throttling). clock.Advance(base::TimeDelta::FromSeconds(10)); @@ -117,7 +117,7 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, Basic) { EXPECT_EQ(info.level, 5); EXPECT_EQ(info.last_notification_timestamp, clock.NowTicks() - base::TimeDelta::FromSeconds(20)); - EXPECT_FALSE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_FALSE(notification_manager->FindById(kTestBatteryAddress) != NULL); } IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, InvalidBatteryInfo) { @@ -149,10 +149,10 @@ IN_PROC_BROWSER_TEST_F(PeripheralBatteryObserverTest, DeviceRemove) { observer_->PeripheralBatteryStatusReceived(kTestBatteryPath, kTestDeviceName, 5); EXPECT_EQ(observer_->batteries_.count(kTestBatteryAddress), 1u); - EXPECT_TRUE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_TRUE(notification_manager->FindById(kTestBatteryAddress) != NULL); observer_->RemoveBattery(kTestBatteryAddress); - EXPECT_FALSE(notification_manager->DoesIdExist(kTestBatteryAddress)); + EXPECT_FALSE(notification_manager->FindById(kTestBatteryAddress) != NULL); } } // namespace chromeos diff --git a/chrome/browser/extensions/api/notifications/notifications_api.cc b/chrome/browser/extensions/api/notifications/notifications_api.cc index cd8aff5..7e9477e 100644 --- a/chrome/browser/extensions/api/notifications/notifications_api.cc +++ b/chrome/browser/extensions/api/notifications/notifications_api.cc @@ -35,6 +35,9 @@ namespace extensions { namespace { const char kResultKey[] = "result"; +const char kMissingRequiredPropertiesForCreateNotification[] = + "Some of the required properties are missing: type, iconUrl, title and " + "message."; const char kUnexpectedProgressValueForNonProgressType[] = "The progress value should not be specified for non-progress notification"; const char kInvalidProgressValue[] = @@ -231,12 +234,21 @@ NotificationsApiFunction::~NotificationsApiFunction() { bool NotificationsApiFunction::CreateNotification( const std::string& id, api::notifications::NotificationOptions* options) { + // First, make sure the required fields exist: type, title, message, icon. + // These fields are defined as optional in IDL such that they can be used as + // optional for notification updates. But for notification creations, they + // should be present. + if (options->type == api::notifications::TEMPLATE_TYPE_NONE || + !options->icon_url || !options->title || !options->message) { + SetError(kMissingRequiredPropertiesForCreateNotification); + return false; + } - // First, extract required fields: type, title, message, and icon. + // Extract required fields: type, title, message, and icon. message_center::NotificationType type = MapApiTemplateTypeToType(options->type); - const string16 title(UTF8ToUTF16(options->title)); - const string16 message(UTF8ToUTF16(options->message)); + const string16 title(UTF8ToUTF16(*options->title)); + const string16 message(UTF8ToUTF16(*options->message)); gfx::Image icon; // TODO(dewittj): Return error if this fails. @@ -326,6 +338,96 @@ bool NotificationsApiFunction::CreateNotification( return true; } +bool NotificationsApiFunction::UpdateNotification( + const std::string& id, + api::notifications::NotificationOptions* options, + Notification* notification) { + // Update optional fields if provided. + if (options->type != api::notifications::TEMPLATE_TYPE_NONE) + notification->set_type(MapApiTemplateTypeToType(options->type)); + if (options->title) + notification->set_title(UTF8ToUTF16(*options->title)); + if (options->message) + notification->set_message(UTF8ToUTF16(*options->message)); + + // TODO(dewittj): Return error if this fails. + if (options->icon_bitmap) { + gfx::Image icon; + NotificationBitmapToGfxImage(options->icon_bitmap.get(), &icon); + notification->set_icon(icon); + } + + message_center::RichNotificationData optional_fields; + if (message_center::IsRichNotificationEnabled()) { + if (options->priority) + notification->set_priority(*options->priority); + + if (options->event_time) + notification->set_timestamp(base::Time::FromJsTime(*options->event_time)); + + if (options->buttons) { + // Currently we allow up to 2 buttons. + size_t number_of_buttons = options->buttons->size(); + number_of_buttons = number_of_buttons > 2 ? 2 : number_of_buttons; + + for (size_t i = 0; i < number_of_buttons; i++) { + message_center::ButtonInfo info( + UTF8ToUTF16((*options->buttons)[i]->title)); + NotificationBitmapToGfxImage((*options->buttons)[i]->icon_bitmap.get(), + &info.icon); + optional_fields.buttons.push_back(info); + } + } + + if (options->expanded_message) { + notification->set_expanded_message( + UTF8ToUTF16(*options->expanded_message)); + } + + gfx::Image image; + if (NotificationBitmapToGfxImage(options->image_bitmap.get(), &image)) { + // We should have an image if and only if the type is an image type. + if (notification->type() != message_center::NOTIFICATION_TYPE_IMAGE) + return false; + notification->set_image(image); + } + + if (options->progress) { + // We should have progress if and only if the type is a progress type. + if (notification->type() != message_center::NOTIFICATION_TYPE_PROGRESS) { + SetError(kUnexpectedProgressValueForNonProgressType); + return false; + } + int progress = *options->progress; + // Progress value should range from 0 to 100. + if (progress < 0 || progress > 100) { + SetError(kInvalidProgressValue); + return false; + } + notification->set_progress(progress); + } + + if (options->items.get() && options->items->size() > 0) { + // We should have list items if and only if the type is a multiple type. + if (notification->type() != message_center::NOTIFICATION_TYPE_MULTIPLE) + return false; + + std::vector< message_center::NotificationItem> items; + using api::notifications::NotificationItem; + std::vector<linked_ptr<NotificationItem> >::iterator i; + for (i = options->items->begin(); i != options->items->end(); ++i) { + message_center::NotificationItem item(UTF8ToUTF16(i->get()->title), + UTF8ToUTF16(i->get()->message)); + items.push_back(item); + } + notification->set_items(items); + } + } + + g_browser_process->notification_ui_manager()->Add(*notification, profile()); + return true; +} + bool NotificationsApiFunction::IsNotificationsApiEnabled() { DesktopNotificationService* service = DesktopNotificationServiceFactory::GetForProfile(profile()); @@ -411,21 +513,24 @@ bool NotificationsUpdateFunction::RunNotificationsApi() { // We are in update. If the ID doesn't exist, succeed but call the callback // with "false". - if (!g_browser_process->notification_ui_manager()->DoesIdExist( - CreateScopedIdentifier(extension_->id(), params_->notification_id))) { + const Notification* matched_notification = + g_browser_process->notification_ui_manager()->FindById( + CreateScopedIdentifier(extension_->id(), params_->notification_id)); + if (!matched_notification) { SetResult(Value::CreateBooleanValue(false)); SendResponse(true); return true; } - // If we have trouble creating the notification (could be improper use of API + // If we have trouble updating the notification (could be improper use of API // or some other reason), mark the function as failed, calling the callback // with false. // TODO(dewittj): Add more human-readable error strings if this fails. - bool could_create_notification = - CreateNotification(params_->notification_id, ¶ms_->options); - SetResult(Value::CreateBooleanValue(could_create_notification)); - if (!could_create_notification) + Notification notification = *matched_notification; + bool could_update_notification = UpdateNotification( + params_->notification_id, ¶ms_->options, ¬ification); + SetResult(Value::CreateBooleanValue(could_update_notification)); + if (!could_update_notification) return false; // No trouble, created the notification, send true to the callback and diff --git a/chrome/browser/extensions/api/notifications/notifications_api.h b/chrome/browser/extensions/api/notifications/notifications_api.h index 6949905..a465604 100644 --- a/chrome/browser/extensions/api/notifications/notifications_api.h +++ b/chrome/browser/extensions/api/notifications/notifications_api.h @@ -13,6 +13,8 @@ #include "chrome/common/extensions/api/notifications.h" #include "ui/message_center/notification_types.h" +class Notification; + namespace extensions { class NotificationsApiFunction : public ApiFunction { @@ -27,6 +29,9 @@ class NotificationsApiFunction : public ApiFunction { bool CreateNotification(const std::string& id, api::notifications::NotificationOptions* options); + bool UpdateNotification(const std::string& id, + api::notifications::NotificationOptions* options, + Notification* notification); bool IsNotificationsApiEnabled(); diff --git a/chrome/browser/extensions/api/notifications/notifications_apitest.cc b/chrome/browser/extensions/api/notifications/notifications_apitest.cc index 5ea2a9f..4fae518 100644 --- a/chrome/browser/extensions/api/notifications/notifications_apitest.cc +++ b/chrome/browser/extensions/api/notifications/notifications_apitest.cc @@ -191,43 +191,160 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestIdUsage) { } IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestBaseFormatNotification) { - scoped_refptr<extensions::NotificationsCreateFunction> - notification_create_function( - new extensions::NotificationsCreateFunction()); scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension()); - notification_create_function->set_extension(empty_extension.get()); - notification_create_function->set_has_callback(true); + // Create a new notification with the minimum required properties. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); - scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( - notification_create_function.get(), - "[\"\", " - "{" - "\"type\": \"basic\"," - "\"iconUrl\": \"an/image/that/does/not/exist.png\"," - "\"title\": \"Attention!\"," - "\"message\": \"Check out Cirque du Soleil\"," - "\"priority\": 1," - "\"eventTime\": 1234567890.12345678," - "\"buttons\": [" - " {" - " \"title\": \"Up\"," - " \"iconUrl\":\"http://www.google.com/logos/2012/\"" - " }," - " {" - " \"title\": \"Down\"" // note: no iconUrl - " }" - "]," - "\"expandedMessage\": \"This is a longer expanded message.\"," - "\"imageUrl\": \"http://www.google.com/logos/2012/election12-hp.jpg\"" - "}]", - browser(), - utils::NONE)); + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_create_function.get(), + "[\"\", " + "{" + "\"type\": \"basic\"," + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"title\": \"Attention!\"," + "\"message\": \"Check out Cirque du Soleil\"" + "}]", + browser(), + utils::NONE)); - std::string notification_id; - ASSERT_EQ(base::Value::TYPE_STRING, result->GetType()); - ASSERT_TRUE(result->GetAsString(¬ification_id)); - ASSERT_TRUE(notification_id.length() > 0); + std::string notification_id; + ASSERT_EQ(base::Value::TYPE_STRING, result->GetType()); + ASSERT_TRUE(result->GetAsString(¬ification_id)); + ASSERT_TRUE(notification_id.length() > 0); + } + + // Create another new notification with more than the required properties. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); + + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_create_function.get(), + "[\"\", " + "{" + "\"type\": \"basic\"," + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"title\": \"Attention!\"," + "\"message\": \"Check out Cirque du Soleil\"," + "\"priority\": 1," + "\"eventTime\": 1234567890.12345678," + "\"buttons\": [" + " {" + " \"title\": \"Up\"," + " \"iconUrl\":\"http://www.google.com/logos/2012/\"" + " }," + " {" + " \"title\": \"Down\"" // note: no iconUrl + " }" + "]," + "\"expandedMessage\": \"This is a longer expanded message.\"," + "\"imageUrl\": \"http://www.google.com/logos/2012/election12-hp.jpg\"" + "}]", + browser(), + utils::NONE)); + + std::string notification_id; + ASSERT_EQ(base::Value::TYPE_STRING, result->GetType()); + ASSERT_TRUE(result->GetAsString(¬ification_id)); + ASSERT_TRUE(notification_id.length() > 0); + } + + // Error case: missing type property. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); + + utils::RunFunction( + notification_create_function.get(), + "[\"\", " + "{" + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"title\": \"Attention!\"," + "\"message\": \"Check out Cirque du Soleil\"" + "}]", + browser(), + utils::NONE); + + EXPECT_FALSE(notification_create_function->GetError().empty()); + } + + // Error case: missing iconUrl property. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); + + utils::RunFunction( + notification_create_function.get(), + "[\"\", " + "{" + "\"type\": \"basic\"," + "\"title\": \"Attention!\"," + "\"message\": \"Check out Cirque du Soleil\"" + "}]", + browser(), + utils::NONE); + + EXPECT_FALSE(notification_create_function->GetError().empty()); + } + + // Error case: missing title property. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); + + utils::RunFunction( + notification_create_function.get(), + "[\"\", " + "{" + "\"type\": \"basic\"," + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"message\": \"Check out Cirque du Soleil\"" + "}]", + browser(), + utils::NONE); + + EXPECT_FALSE(notification_create_function->GetError().empty()); + } + + // Error case: missing message property. + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_create_function( + new extensions::NotificationsCreateFunction()); + notification_create_function->set_extension(empty_extension.get()); + notification_create_function->set_has_callback(true); + + utils::RunFunction( + notification_create_function.get(), + "[\"\", " + "{" + "\"type\": \"basic\"," + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"title\": \"Attention!\"" + "}]", + browser(), + utils::NONE); + + EXPECT_FALSE(notification_create_function->GetError().empty()); + } } IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestMultipleItemNotification) { @@ -399,6 +516,8 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, MAYBE_TestByUser) { IN_PROC_BROWSER_TEST_F(NotificationsApiTest, MAYBE_TestProgressNotification) { scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension()); + // Create a new progress notification. + std::string notification_id; { scoped_refptr<extensions::NotificationsCreateFunction> notification_create_function( @@ -421,12 +540,35 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, MAYBE_TestProgressNotification) { browser(), utils::NONE)); - std::string notification_id; EXPECT_EQ(base::Value::TYPE_STRING, result->GetType()); EXPECT_TRUE(result->GetAsString(¬ification_id)); EXPECT_TRUE(notification_id.length() > 0); } + // Update the progress property only. + { + scoped_refptr<extensions::NotificationsUpdateFunction> + notification_function( + new extensions::NotificationsUpdateFunction()); + notification_function->set_extension(empty_extension.get()); + notification_function->set_has_callback(true); + + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_function.get(), + "[\"" + notification_id + + "\", " + "{" + "\"progress\": 60" + "}]", + browser(), + utils::NONE)); + + EXPECT_EQ(base::Value::TYPE_BOOLEAN, result->GetType()); + bool copy_bool_value = false; + EXPECT_TRUE(result->GetAsBoolean(©_bool_value)); + EXPECT_TRUE(copy_bool_value); + } + // Error case: progress value provided for non-progress type. { scoped_refptr<extensions::NotificationsCreateFunction> @@ -502,3 +644,82 @@ IN_PROC_BROWSER_TEST_F(NotificationsApiTest, MAYBE_TestProgressNotification) { EXPECT_FALSE(notification_create_function->GetError().empty()); } } + +IN_PROC_BROWSER_TEST_F(NotificationsApiTest, TestPartialUpdate) { + scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension()); + + // Create a new notification. + std::string notification_id; + { + scoped_refptr<extensions::NotificationsCreateFunction> + notification_function( + new extensions::NotificationsCreateFunction()); + notification_function->set_extension(empty_extension.get()); + notification_function->set_has_callback(true); + + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_function.get(), + "[\"\", " // Empty string: ask API to generate ID + "{" + "\"type\": \"basic\"," + "\"iconUrl\": \"an/image/that/does/not/exist.png\"," + "\"title\": \"Attention!\"," + "\"message\": \"Check out Cirque du Soleil\"" + "}]", + browser(), + utils::NONE)); + + ASSERT_EQ(base::Value::TYPE_STRING, result->GetType()); + ASSERT_TRUE(result->GetAsString(¬ification_id)); + ASSERT_TRUE(notification_id.length() > 0); + } + + // Update a few properties in the existing notification. + { + scoped_refptr<extensions::NotificationsUpdateFunction> + notification_function( + new extensions::NotificationsUpdateFunction()); + notification_function->set_extension(empty_extension.get()); + notification_function->set_has_callback(true); + + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_function.get(), + "[\"" + notification_id + + "\", " + "{" + "\"title\": \"Changed!\"," + "\"message\": \"Too late! The show ended yesterday\"" + "}]", + browser(), + utils::NONE)); + + ASSERT_EQ(base::Value::TYPE_BOOLEAN, result->GetType()); + bool copy_bool_value = false; + ASSERT_TRUE(result->GetAsBoolean(©_bool_value)); + ASSERT_TRUE(copy_bool_value); + } + + // Update another property in the existing notification. + { + scoped_refptr<extensions::NotificationsUpdateFunction> + notification_function( + new extensions::NotificationsUpdateFunction()); + notification_function->set_extension(empty_extension.get()); + notification_function->set_has_callback(true); + + scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult( + notification_function.get(), + "[\"" + notification_id + + "\", " + "{" + "\"priority\": 2" + "}]", + browser(), + utils::NONE)); + + ASSERT_EQ(base::Value::TYPE_BOOLEAN, result->GetType()); + bool copy_bool_value = false; + ASSERT_TRUE(result->GetAsBoolean(©_bool_value)); + ASSERT_TRUE(copy_bool_value); + } +} diff --git a/chrome/browser/notifications/balloon_collection.h b/chrome/browser/notifications/balloon_collection.h index cd17d1a..5d54b7d 100644 --- a/chrome/browser/notifications/balloon_collection.h +++ b/chrome/browser/notifications/balloon_collection.h @@ -55,7 +55,7 @@ class BalloonCollection { Profile* profile) = 0; // Returns true if any balloon has this notification id. - virtual bool DoesIdExist(const std::string& id) = 0; + virtual const Notification* FindById(const std::string& id) const = 0; // Removes any balloons that have this notification id. Returns // true if anything was removed. diff --git a/chrome/browser/notifications/balloon_collection_base.cc b/chrome/browser/notifications/balloon_collection_base.cc index c748739..b7f8669 100644 --- a/chrome/browser/notifications/balloon_collection_base.cc +++ b/chrome/browser/notifications/balloon_collection_base.cc @@ -35,13 +35,15 @@ void BalloonCollectionBase::Remove(Balloon* balloon) { } } -bool BalloonCollectionBase::DoesIdExist(const std::string& id) { - Balloons::iterator iter; +const Notification* BalloonCollectionBase::FindById( + const std::string& id) const { + Balloons::const_iterator iter; for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { - if ((*iter)->notification().notification_id() == id) - return true; + if ((*iter)->notification().notification_id() == id) { + return &((*iter)->notification()); + } } - return false; + return NULL; } bool BalloonCollectionBase::CloseById(const std::string& id) { diff --git a/chrome/browser/notifications/balloon_collection_base.h b/chrome/browser/notifications/balloon_collection_base.h index 8c4e442..89f3341 100644 --- a/chrome/browser/notifications/balloon_collection_base.h +++ b/chrome/browser/notifications/balloon_collection_base.h @@ -34,7 +34,7 @@ class BalloonCollectionBase { virtual void Remove(Balloon* balloon); // Returns true if any balloon matches the given notification id. - virtual bool DoesIdExist(const std::string& id); + virtual const Notification* FindById(const std::string& id) const; // Finds any balloon matching the given notification id, and // calls CloseByScript on it. Returns true if anything was diff --git a/chrome/browser/notifications/balloon_collection_impl.cc b/chrome/browser/notifications/balloon_collection_impl.cc index c7d0861..3ee0c05 100644 --- a/chrome/browser/notifications/balloon_collection_impl.cc +++ b/chrome/browser/notifications/balloon_collection_impl.cc @@ -92,8 +92,9 @@ void BalloonCollectionImpl::Add(const Notification& notification, AddImpl(notification, profile, false); } -bool BalloonCollectionImpl::DoesIdExist(const std::string& id) { - return base_.DoesIdExist(id); +const Notification* BalloonCollectionImpl::FindById( + const std::string& id) const { + return base_.FindById(id); } bool BalloonCollectionImpl::RemoveById(const std::string& id) { diff --git a/chrome/browser/notifications/balloon_collection_impl.h b/chrome/browser/notifications/balloon_collection_impl.h index 45b26d0..4a006fb 100644 --- a/chrome/browser/notifications/balloon_collection_impl.h +++ b/chrome/browser/notifications/balloon_collection_impl.h @@ -46,7 +46,7 @@ class BalloonCollectionImpl : public BalloonCollection, // BalloonCollection interface. virtual void Add(const Notification& notification, Profile* profile) OVERRIDE; - virtual bool DoesIdExist(const std::string& id) OVERRIDE; + virtual const Notification* FindById(const std::string& id) const OVERRIDE; virtual bool RemoveById(const std::string& id) OVERRIDE; virtual bool RemoveBySourceOrigin(const GURL& source_origin) OVERRIDE; virtual bool RemoveByProfile(Profile* profile) OVERRIDE; diff --git a/chrome/browser/notifications/balloon_notification_ui_manager.cc b/chrome/browser/notifications/balloon_notification_ui_manager.cc index d6ea75e..e19b4f6 100644 --- a/chrome/browser/notifications/balloon_notification_ui_manager.cc +++ b/chrome/browser/notifications/balloon_notification_ui_manager.cc @@ -44,10 +44,12 @@ void BalloonNotificationUIManager::SetBalloonCollection( balloon_collection_->set_space_change_listener(this); } -bool BalloonNotificationUIManager::DoesIdExist(const std::string& id) { - if (NotificationUIManagerImpl::DoesIdExist(id)) - return true; - return balloon_collection_->DoesIdExist(id); +const Notification* BalloonNotificationUIManager::FindById( + const std::string& id) const { + const Notification* notification = NotificationUIManagerImpl::FindById(id); + if (notification) + return notification; + return balloon_collection_->FindById(id); } bool BalloonNotificationUIManager::CancelById(const std::string& id) { diff --git a/chrome/browser/notifications/balloon_notification_ui_manager.h b/chrome/browser/notifications/balloon_notification_ui_manager.h index 953891e..31be3fa 100644 --- a/chrome/browser/notifications/balloon_notification_ui_manager.h +++ b/chrome/browser/notifications/balloon_notification_ui_manager.h @@ -34,7 +34,8 @@ class BalloonNotificationUIManager void SetBalloonCollection(BalloonCollection* balloon_collection); // NotificationUIManager: - virtual bool DoesIdExist(const std::string& notification_id) OVERRIDE; + virtual const Notification* FindById( + const std::string& notification_id) const OVERRIDE; virtual bool CancelById(const std::string& notification_id) OVERRIDE; virtual std::set<std::string> GetAllIdsByProfileAndSourceOrigin( Profile* profile, diff --git a/chrome/browser/notifications/message_center_notification_manager.cc b/chrome/browser/notifications/message_center_notification_manager.cc index d0cbb7e..76458b4 100644 --- a/chrome/browser/notifications/message_center_notification_manager.cc +++ b/chrome/browser/notifications/message_center_notification_manager.cc @@ -64,13 +64,15 @@ MessageCenterNotificationManager::~MessageCenterNotificationManager() { //////////////////////////////////////////////////////////////////////////////// // NotificationUIManager -bool MessageCenterNotificationManager::DoesIdExist(const std::string& id) { - if (NotificationUIManagerImpl::DoesIdExist(id)) - return true; - NotificationMap::iterator iter = profile_notifications_.find(id); +const Notification* MessageCenterNotificationManager::FindById( + const std::string& id) const { + const Notification* notification = NotificationUIManagerImpl::FindById(id); + if (notification) + return notification; + NotificationMap::const_iterator iter = profile_notifications_.find(id); if (iter == profile_notifications_.end()) - return false; - return true; + return NULL; + return &(iter->second->notification()); } bool MessageCenterNotificationManager::CancelById(const std::string& id) { diff --git a/chrome/browser/notifications/message_center_notification_manager.h b/chrome/browser/notifications/message_center_notification_manager.h index 8ae5ee5..89493a2 100644 --- a/chrome/browser/notifications/message_center_notification_manager.h +++ b/chrome/browser/notifications/message_center_notification_manager.h @@ -38,7 +38,8 @@ class MessageCenterNotificationManager virtual ~MessageCenterNotificationManager(); // NotificationUIManager - virtual bool DoesIdExist(const std::string& notification_id) OVERRIDE; + virtual const Notification* FindById( + const std::string& notification_id) const OVERRIDE; virtual bool CancelById(const std::string& notification_id) OVERRIDE; virtual std::set<std::string> GetAllIdsByProfileAndSourceOrigin( Profile* profile, diff --git a/chrome/browser/notifications/notification_ui_manager.h b/chrome/browser/notifications/notification_ui_manager.h index 6257939..a86f1d2 100644 --- a/chrome/browser/notifications/notification_ui_manager.h +++ b/chrome/browser/notifications/notification_ui_manager.h @@ -29,9 +29,10 @@ class NotificationUIManager { virtual void Add(const Notification& notification, Profile* profile) = 0; - // Returns true if any notifications match the supplied ID, either currently - // displayed or in the queue. - virtual bool DoesIdExist(const std::string& notification_id) = 0; + // Returns the pointer to a notification if it match the supplied ID, either + // currently displayed or in the queue. + virtual const Notification* FindById( + const std::string& notification_id) const = 0; // Removes any notifications matching the supplied ID, either currently // displayed or in the queue. Returns true if anything was removed. diff --git a/chrome/browser/notifications/notification_ui_manager_impl.cc b/chrome/browser/notifications/notification_ui_manager_impl.cc index 6e15592..d2b08d0 100644 --- a/chrome/browser/notifications/notification_ui_manager_impl.cc +++ b/chrome/browser/notifications/notification_ui_manager_impl.cc @@ -74,13 +74,15 @@ void NotificationUIManagerImpl::Add(const Notification& notification, CheckAndShowNotifications(); } -bool NotificationUIManagerImpl::DoesIdExist(const std::string& id) { - for (NotificationDeque::iterator iter = show_queue_.begin(); +const Notification* NotificationUIManagerImpl::FindById( + const std::string& id) const { + for (NotificationDeque::const_iterator iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) { - if ((*iter)->notification().notification_id() == id) - return true; + if ((*iter)->notification().notification_id() == id) { + return &((*iter)->notification()); + } } - return false; + return NULL; } bool NotificationUIManagerImpl::CancelById(const std::string& id) { diff --git a/chrome/browser/notifications/notification_ui_manager_impl.h b/chrome/browser/notifications/notification_ui_manager_impl.h index f8544ea..5ced721 100644 --- a/chrome/browser/notifications/notification_ui_manager_impl.h +++ b/chrome/browser/notifications/notification_ui_manager_impl.h @@ -34,7 +34,8 @@ class NotificationUIManagerImpl // NotificationUIManager: virtual void Add(const Notification& notification, Profile* profile) OVERRIDE; - virtual bool DoesIdExist(const std::string& notification_id) OVERRIDE; + virtual const Notification* FindById( + const std::string& notification_id) const OVERRIDE; virtual bool CancelById(const std::string& notification_id) OVERRIDE; virtual std::set<std::string> GetAllIdsByProfileAndSourceOrigin( Profile* profile, diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc index caedb24..2906a37 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc @@ -65,8 +65,8 @@ class StubNotificationUIManager : public NotificationUIManager { // Returns true if any notifications match the supplied ID, either currently // displayed or in the queue. - virtual bool DoesIdExist(const std::string& id) OVERRIDE { - return true; + virtual const Notification* FindById(const std::string& id) const OVERRIDE { + return (notification_.id() == id) ? ¬ification_ : NULL; } // Removes any notifications matching the supplied ID, either currently diff --git a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc index ccd6b4f..38794aa 100644 --- a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc +++ b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc @@ -58,8 +58,8 @@ class StubNotificationUIManager : public NotificationUIManager { // Returns true if any notifications match the supplied ID, either currently // displayed or in the queue. - virtual bool DoesIdExist(const std::string& id) OVERRIDE { - return true; + virtual const Notification* FindById(const std::string& id) const OVERRIDE { + return (notification_.id() == id) ? ¬ification_ : NULL; } // Removes any notifications matching the supplied ID, either currently diff --git a/chrome/browser/ui/ash/screenshot_taker_unittest.cc b/chrome/browser/ui/ash/screenshot_taker_unittest.cc index ed8ffd1..439a6a1 100644 --- a/chrome/browser/ui/ash/screenshot_taker_unittest.cc +++ b/chrome/browser/ui/ash/screenshot_taker_unittest.cc @@ -118,8 +118,8 @@ TEST_F(ScreenshotTakerTest, TakeScreenshot) { #if defined(OS_CHROMEOS) // Screenshot notifications on Windows not yet turned on. - EXPECT_TRUE(g_browser_process->notification_ui_manager()->DoesIdExist( - std::string("screenshot"))); + EXPECT_TRUE(g_browser_process->notification_ui_manager()->FindById( + std::string("screenshot")) != NULL); g_browser_process->notification_ui_manager()->CancelAll(); #endif diff --git a/chrome/browser/ui/cocoa/notifications/balloon_controller_unittest.mm b/chrome/browser/ui/cocoa/notifications/balloon_controller_unittest.mm index 58c279f..b8f8137 100644 --- a/chrome/browser/ui/cocoa/notifications/balloon_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/notifications/balloon_controller_unittest.mm @@ -31,7 +31,9 @@ namespace { class MockBalloonCollection : public BalloonCollection { virtual void Add(const Notification& notification, Profile* profile) OVERRIDE {} - virtual bool DoesIdExist(const std::string& id) OVERRIDE { return false; } + virtual const Notification* FindById(const std::string& id) const OVERRIDE { + return NULL; + } virtual bool RemoveById(const std::string& id) OVERRIDE { return false; } virtual bool RemoveBySourceOrigin(const GURL& origin) OVERRIDE { return false; diff --git a/chrome/common/extensions/api/notifications.idl b/chrome/common/extensions/api/notifications.idl index 5e24848..5808bc4 100644 --- a/chrome/common/extensions/api/notifications.idl +++ b/chrome/common/extensions/api/notifications.idl @@ -41,17 +41,17 @@ namespace notifications { dictionary NotificationOptions { // Which type of notification to display. - TemplateType type; + TemplateType? type; // Sender's avatar, app icon, or a thumbnail for image notifications. - DOMString iconUrl; + DOMString? iconUrl; [nodoc] NotificationBitmap? iconBitmap; // Title of the notification (e.g. sender name for email). - DOMString title; + DOMString? title; // Main notification content. - DOMString message; + DOMString? message; // Priority ranges from -2 to 2. -2 is lowest priority. 2 is highest. Zero // is default. diff --git a/chrome/renderer/resources/extensions/notifications_custom_bindings.js b/chrome/renderer/resources/extensions/notifications_custom_bindings.js index 5150a4f..915439e 100644 --- a/chrome/renderer/resources/extensions/notifications_custom_bindings.js +++ b/chrome/renderer/resources/extensions/notifications_custom_bindings.js @@ -26,13 +26,16 @@ function replaceNotificationOptionURLs(notification_details, callback) { // field in the output of create. // TODO(dewittj): Try to remove hard-coding of image sizes. - // |iconUrl| is required. - var url_specs = [{ - path: notification_details.iconUrl, - width: 80, - height: 80, - callback: image_data_setter(notification_details, 'iconBitmap') - }]; + // |iconUrl| might be optional for notification updates. + var url_specs = []; + if (notification_details.iconUrl) { + $Array.push(url_specs, { + path: notification_details.iconUrl, + width: 80, + height: 80, + callback: image_data_setter(notification_details, 'iconBitmap') + }); + } // |imageUrl| is optional. if (notification_details.imageUrl) { @@ -60,6 +63,11 @@ function replaceNotificationOptionURLs(notification_details, callback) { } } + if (!url_specs.length) { + callback(true); + return; + } + var errors = 0; imageUtil.loadAllImages(url_specs, { |