diff options
author | jianli@chromium.org <jianli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-26 13:40:47 +0000 |
---|---|---|
committer | jianli@chromium.org <jianli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-26 13:40:47 +0000 |
commit | 12d9b9eaa1cb43034b4991539f4b44cc349cc2fe (patch) | |
tree | 3bafa8e97ee8926cceb0c0cf6ff37a4dc53ac93c | |
parent | c47449bbe221667396e9b90e0d6cefed765b597f (diff) | |
download | chromium_src-12d9b9eaa1cb43034b4991539f4b44cc349cc2fe.zip chromium_src-12d9b9eaa1cb43034b4991539f4b44cc349cc2fe.tar.gz chromium_src-12d9b9eaa1cb43034b4991539f4b44cc349cc2fe.tar.bz2 |
Allow partial update for notification update API
With this change, the user only needs to specify the properties to be changed. This patch does not change the way that the toast is updated by recreating the views. It will be addressed in next patch.
BUG=170924
TEST=new tests added
TBR=sky@chromium.org
Review URL: https://chromiumcodereview.appspot.com/20136004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@213891 0039d316-1c4b-4281-b951-d872f2087c98
23 files changed, 475 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, { diff --git a/ui/message_center/notification.h b/ui/message_center/notification.h index 4217ac4..bda6900 100644 --- a/ui/message_center/notification.h +++ b/ui/message_center/notification.h @@ -69,9 +69,15 @@ class MESSAGE_CENTER_EXPORT Notification { void CopyState(Notification* base); NotificationType type() const { return type_; } + void set_type(NotificationType type) { type_ = type; } + const std::string& id() const { return id_; } + const string16& title() const { return title_; } + void set_title(const string16& title) { title_ = title; } + const string16& message() const { return message_; } + void set_message(const string16& message) { message_ = message; } // A display string for the source of the notification. const string16& display_source() const { return display_source_; } @@ -82,14 +88,29 @@ class MESSAGE_CENTER_EXPORT Notification { // Begin unpacked values from optional_fields. int priority() const { return optional_fields_.priority; } + void set_priority(int priority) { optional_fields_.priority = priority; } + base::Time timestamp() const { return optional_fields_.timestamp; } + void set_timestamp(const base::Time& timestamp) { + optional_fields_.timestamp = timestamp; + } + const string16& expanded_message() const { return optional_fields_.expanded_message; } + void set_expanded_message(const string16& expanded_message) { + optional_fields_.expanded_message = expanded_message; + } + const std::vector<NotificationItem>& items() const { return optional_fields_.items; } + void set_items(const std::vector<NotificationItem>& items) { + optional_fields_.items = items; + } + int progress() const { return optional_fields_.progress; } + void set_progress(int progress) { optional_fields_.progress = progress; } // End unpacked values. // Images fetched asynchronously. |