diff options
Diffstat (limited to 'chromium/ui/message_center')
42 files changed, 441 insertions, 539 deletions
diff --git a/chromium/ui/message_center/BUILD.gn b/chromium/ui/message_center/BUILD.gn index ed816055fb4..e7b7e35f858 100644 --- a/chromium/ui/message_center/BUILD.gn +++ b/chromium/ui/message_center/BUILD.gn @@ -13,12 +13,10 @@ aggregate_vector_icons("message_center_vector_icons") { icon_directory = "vector_icons" icons = [ - "notification_close_button.1x.icon", "notification_close_button.icon", "notification_expand_less.icon", "notification_expand_more.icon", "notification_inline_reply.icon", - "notification_settings_button.1x.icon", "notification_settings_button.icon", "product.icon", ] @@ -49,6 +47,7 @@ jumbo_component("message_center") { "//ui/accessibility", "//ui/display", "//ui/events", + "//ui/events:gesture_detection", "//ui/gfx", "//ui/gfx/geometry", "//ui/native_theme", @@ -209,7 +208,7 @@ if (enable_message_center) { ":test_support", "//base", "//base/test:test_support", - "//mojo/edk/system", + "//mojo/edk", "//skia", "//testing/gmock", "//testing/gtest", @@ -236,7 +235,7 @@ if (enable_message_center) { if (is_chromeos) { sources += [ "public/mojo/struct_traits_unittest.cc" ] deps += [ - "//mojo/edk/system", + "//mojo/edk", "//ui/message_center/public/mojo:test_interfaces", ] } diff --git a/chromium/ui/message_center/message_center_impl.cc b/chromium/ui/message_center/message_center_impl.cc index 53485f966a1..b5d9e1385b6 100644 --- a/chromium/ui/message_center/message_center_impl.cc +++ b/chromium/ui/message_center/message_center_impl.cc @@ -12,7 +12,6 @@ #include "base/auto_reset.h" #include "base/command_line.h" #include "base/macros.h" -#include "base/memory/ptr_util.h" #include "base/observer_list.h" #include "base/stl_util.h" #include "base/strings/string_util.h" @@ -76,15 +75,6 @@ void MessageCenterImpl::OnBlockingStateChanged(NotificationBlocker* blocker) { NotificationList::PopupNotifications popups = notification_list_->GetPopupNotifications(blockers_, &blocked); - // Close already displayed pop-ups that are blocked now. - for (const std::string& notification_id : blocked) { - // Do not call MessageCenterImpl::MarkSinglePopupAsShown() directly here - // just for performance reason. MessageCenterImpl::MarkSinglePopupAsShown() - // calls NotificationList::MarkSinglePopupAsShown(), but the whole cache - // will be recreated below. - if (FindVisibleNotificationById(notification_id)->IsRead()) - notification_list_->MarkSinglePopupAsShown(notification_id, true); - } visible_notifications_ = notification_list_->GetVisibleNotifications(blockers_); @@ -319,9 +309,9 @@ void MessageCenterImpl::ClickOnNotification(const std::string& id) { scoped_refptr<NotificationDelegate> delegate = notification_list_->GetNotificationDelegate(id); for (auto& observer : observer_list_) - observer.OnNotificationClicked(id); - if (delegate.get()) - delegate->Click(); + observer.OnNotificationClicked(id, base::nullopt, base::nullopt); + if (delegate) + delegate->Click(base::nullopt, base::nullopt); } void MessageCenterImpl::ClickOnNotificationButton(const std::string& id, @@ -336,9 +326,9 @@ void MessageCenterImpl::ClickOnNotificationButton(const std::string& id, scoped_refptr<NotificationDelegate> delegate = notification_list_->GetNotificationDelegate(id); for (auto& observer : observer_list_) - observer.OnNotificationButtonClicked(id, button_index); - if (delegate.get()) - delegate->ButtonClick(button_index); + observer.OnNotificationClicked(id, button_index, base::nullopt); + if (delegate) + delegate->Click(button_index, base::nullopt); } void MessageCenterImpl::ClickOnNotificationButtonWithReply( @@ -355,9 +345,9 @@ void MessageCenterImpl::ClickOnNotificationButtonWithReply( scoped_refptr<NotificationDelegate> delegate = notification_list_->GetNotificationDelegate(id); for (auto& observer : observer_list_) - observer.OnNotificationButtonClickedWithReply(id, button_index, reply); - if (delegate.get()) - delegate->ButtonClickWithReply(button_index, reply); + observer.OnNotificationClicked(id, button_index, reply); + if (delegate) + delegate->Click(button_index, reply); } void MessageCenterImpl::ClickOnSettingsButton(const std::string& id) { diff --git a/chromium/ui/message_center/message_center_observer.h b/chromium/ui/message_center/message_center_observer.h index 9ef0ec65c84..c88c1166571 100644 --- a/chromium/ui/message_center/message_center_observer.h +++ b/chromium/ui/message_center/message_center_observer.h @@ -7,6 +7,7 @@ #include <string> +#include "base/optional.h" #include "ui/message_center/message_center_export.h" #include "ui/message_center/message_center_types.h" @@ -34,20 +35,13 @@ class MESSAGE_CENTER_EXPORT MessageCenterObserver { virtual void OnNotificationUpdated(const std::string& notification_id) {} // Called when a click event happens on the notification associated with - // |notification_id|. - virtual void OnNotificationClicked(const std::string& notification_id) {} - - // Called when a click event happens on a button indexed by |button_index| - // of the notification associated with |notification_id|. - virtual void OnNotificationButtonClicked(const std::string& notification_id, - int button_index) {} - - // Called when a click event happens on a button with an input indexed by - // |button_index| of the notification associated with |notification_id|. - virtual void OnNotificationButtonClickedWithReply( + // |notification_id|. |button_index| will be nullopt if the click occurred on + // the body of the notification. |reply| will be filled in only if there was + // an input field associated with the button. + virtual void OnNotificationClicked( const std::string& notification_id, - int button_index, - const base::string16& reply) {} + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) {} // Called when notification settings button is clicked. The |handled| argument // indicates whether the notification delegate already handled the operation. diff --git a/chromium/ui/message_center/message_center_stats_collector.cc b/chromium/ui/message_center/message_center_stats_collector.cc index 210e0bf85f5..c6503a3a9ed 100644 --- a/chromium/ui/message_center/message_center_stats_collector.cc +++ b/chromium/ui/message_center/message_center_stats_collector.cc @@ -92,24 +92,17 @@ void MessageCenterStatsCollector::OnNotificationUpdated( } void MessageCenterStatsCollector::OnNotificationClicked( - const std::string& notification_id) { - StatsCollection::iterator iter = stats_.find(notification_id); - if (iter == stats_.end()) - return; - NotificationStats& notification_stat = iter->second; - - notification_stat.CollectAction(NOTIFICATION_ACTION_CLICK); -} - -void MessageCenterStatsCollector::OnNotificationButtonClicked( const std::string& notification_id, - int button_index) { + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) { StatsCollection::iterator iter = stats_.find(notification_id); if (iter == stats_.end()) return; NotificationStats& notification_stat = iter->second; - notification_stat.CollectAction(NOTIFICATION_ACTION_BUTTON_CLICK); + notification_stat.CollectAction(button_index + ? NOTIFICATION_ACTION_BUTTON_CLICK + : NOTIFICATION_ACTION_CLICK); } void MessageCenterStatsCollector::OnNotificationSettingsClicked(bool handled) { diff --git a/chromium/ui/message_center/message_center_stats_collector.h b/chromium/ui/message_center/message_center_stats_collector.h index 6906f65d42f..c2e212c3111 100644 --- a/chromium/ui/message_center/message_center_stats_collector.h +++ b/chromium/ui/message_center/message_center_stats_collector.h @@ -66,9 +66,10 @@ class MessageCenterStatsCollector : public MessageCenterObserver { void OnNotificationRemoved(const std::string& notification_id, bool by_user) override; void OnNotificationUpdated(const std::string& notification_id) override; - void OnNotificationClicked(const std::string& notification_id) override; - void OnNotificationButtonClicked(const std::string& notification_id, - int button_index) override; + void OnNotificationClicked( + const std::string& notification_id, + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) override; void OnNotificationSettingsClicked(bool handled) override; void OnNotificationDisplayed(const std::string& notification_id, const DisplaySource source) override; diff --git a/chromium/ui/message_center/notification_list.cc b/chromium/ui/message_center/notification_list.cc index 03882443b45..a461bbff53f 100644 --- a/chromium/ui/message_center/notification_list.cc +++ b/chromium/ui/message_center/notification_list.cc @@ -34,7 +34,7 @@ bool ShouldShowNotificationAsPopup( } // namespace bool ComparePriorityTimestampSerial::operator()(Notification* n1, - Notification* n2) { + Notification* n2) const { if (n1->priority() > n2->priority()) // Higher pri go first. return true; if (n1->priority() < n2->priority()) @@ -42,7 +42,8 @@ bool ComparePriorityTimestampSerial::operator()(Notification* n1, return CompareTimestampSerial()(n1, n2); } -bool CompareTimestampSerial::operator()(Notification* n1, Notification* n2) { +bool CompareTimestampSerial::operator()(Notification* n1, + Notification* n2) const { if (n1->timestamp() > n2->timestamp()) // Newer come first. return true; if (n1->timestamp() < n2->timestamp()) @@ -54,6 +55,11 @@ bool CompareTimestampSerial::operator()(Notification* n1, Notification* n2) { return false; } +bool NotificationList::NotificationState::operator!=( + const NotificationState& other) const { + return shown_as_popup != other.shown_as_popup || is_read != other.is_read; +} + NotificationList::NotificationList(MessageCenter* message_center) : message_center_(message_center), quiet_mode_(false) { @@ -67,14 +73,13 @@ void NotificationList::SetNotificationsShown( std::set<std::string>* updated_ids) { Notifications notifications = GetVisibleNotifications(blockers); - for (auto iter = notifications.begin(); iter != notifications.end(); ++iter) { - Notification* notification = *iter; - bool was_popup = notification->shown_as_popup(); - bool was_read = notification->IsRead(); + for (Notification* notification : notifications) { + NotificationState* state = &GetNotification(notification->id())->second; + const NotificationState original_state = *state; if (notification->priority() < SYSTEM_PRIORITY) - notification->set_shown_as_popup(true); - notification->set_is_read(true); - if (updated_ids && !(was_popup && was_read)) + state->shown_as_popup = true; + state->is_read = true; + if (updated_ids && (original_state != *state)) updated_ids->insert(notification->id()); } } @@ -91,17 +96,17 @@ void NotificationList::UpdateNotificationMessage( if (iter == notifications_.end()) return; - new_notification->CopyState(iter->get()); + Notification* notification = iter->first.get(); + NotificationState state = iter->second; // Handles priority promotion. If the notification is already dismissed but // the updated notification has higher priority, it should re-appear as a // toast. Notifications coming from websites through the Web Notification API // will always re-appear on update. - if (((*iter)->priority() < new_notification->priority() || + if ((notification->priority() < new_notification->priority() || new_notification->notifier_id().type == NotifierId::WEB_PAGE) && !quiet_mode_) { - new_notification->set_is_read(false); - new_notification->set_shown_as_popup(false); + state = NotificationState(); } // Do not use EraseNotification and PushNotification, since we don't want to @@ -110,7 +115,7 @@ void NotificationList::UpdateNotificationMessage( // We really don't want duplicate IDs. DCHECK(GetNotification(new_notification->id()) == notifications_.end()); - notifications_.insert(std::move(new_notification)); + notifications_.emplace(std::move(new_notification), state); } void NotificationList::RemoveNotification(const std::string& id) { @@ -120,9 +125,10 @@ void NotificationList::RemoveNotification(const std::string& id) { NotificationList::Notifications NotificationList::GetNotificationsByNotifierId( const NotifierId& notifier_id) { Notifications notifications; - for (const auto& notification : notifications_) { + for (const auto& tuple : notifications_) { + Notification* notification = tuple.first.get(); if (notification->notifier_id() == notifier_id) - notifications.insert(notification.get()); + notifications.insert(notification); } return notifications; } @@ -132,7 +138,7 @@ bool NotificationList::SetNotificationIcon(const std::string& notification_id, auto iter = GetNotification(notification_id); if (iter == notifications_.end()) return false; - (*iter)->set_icon(image); + iter->first->set_icon(image); return true; } @@ -141,7 +147,7 @@ bool NotificationList::SetNotificationImage(const std::string& notification_id, auto iter = GetNotification(notification_id); if (iter == notifications_.end()) return false; - (*iter)->set_image(image); + iter->first->set_image(image); return true; } @@ -151,7 +157,7 @@ bool NotificationList::SetNotificationButtonIcon( auto iter = GetNotification(notification_id); if (iter == notifications_.end()) return false; - (*iter)->SetButtonIcon(button_index, image); + iter->first->SetButtonIcon(button_index, image); return true; } @@ -161,16 +167,16 @@ bool NotificationList::HasNotificationOfType(const std::string& id, if (iter == notifications_.end()) return false; - return (*iter)->type() == type; + return iter->first->type() == type; } bool NotificationList::HasPopupNotifications( const NotificationBlockers& blockers) { - for (const auto& notification : notifications_) { - if (notification->priority() < DEFAULT_PRIORITY) + for (const auto& tuple : notifications_) { + if (tuple.first->priority() < DEFAULT_PRIORITY) break; - if (!notification->shown_as_popup() && - ShouldShowNotificationAsPopup(*notification.get(), blockers)) { + if (!tuple.second.shown_as_popup && + ShouldShowNotificationAsPopup(*tuple.first, blockers)) { return true; } } @@ -186,8 +192,9 @@ NotificationList::GetPopupNotifications(const NotificationBlockers& blockers, // Collect notifications that should be shown as popups. Start from oldest. for (auto iter = notifications_.rbegin(); iter != notifications_.rend(); iter++) { - Notification* notification = iter->get(); - if (notification->shown_as_popup()) + NotificationState* state = &iter->second; + Notification* notification = iter->first.get(); + if (state->shown_as_popup) continue; // No popups for LOW/MIN priority. @@ -195,6 +202,8 @@ NotificationList::GetPopupNotifications(const NotificationBlockers& blockers, continue; if (!ShouldShowNotificationAsPopup(*notification, blockers)) { + if (state->is_read) + state->shown_as_popup = true; if (blocked) blocked->push_back(notification->id()); continue; @@ -218,17 +227,20 @@ void NotificationList::MarkSinglePopupAsShown( auto iter = GetNotification(id); DCHECK(iter != notifications_.end()); - if ((*iter)->shown_as_popup()) + NotificationState* state = &iter->second; + const Notification& notification = *iter->first; + + if (iter->second.shown_as_popup) return; // System notification is marked as shown only when marked as read. - if ((*iter)->priority() != SYSTEM_PRIORITY || mark_notification_as_read) - (*iter)->set_shown_as_popup(true); + if (notification.priority() != SYSTEM_PRIORITY || mark_notification_as_read) + state->shown_as_popup = true; // The popup notification is already marked as read when it's displayed. - // Set the is_read() back to false if necessary. + // Set the is_read back to false if necessary. if (!mark_notification_as_read) - (*iter)->set_is_read(false); + state->is_read = false; } void NotificationList::MarkSinglePopupAsDisplayed(const std::string& id) { @@ -236,11 +248,12 @@ void NotificationList::MarkSinglePopupAsDisplayed(const std::string& id) { if (iter == notifications_.end()) return; - if ((*iter)->shown_as_popup()) + NotificationState* state = &iter->second; + + if (state->shown_as_popup) return; - if (!(*iter)->IsRead()) - (*iter)->set_is_read(true); + state->is_read = true; } NotificationDelegate* NotificationList::GetNotificationDelegate( @@ -248,37 +261,37 @@ NotificationDelegate* NotificationList::GetNotificationDelegate( auto iter = GetNotification(id); if (iter == notifications_.end()) return nullptr; - return (*iter)->delegate(); + return iter->first->delegate(); } void NotificationList::SetQuietMode(bool quiet_mode) { quiet_mode_ = quiet_mode; if (quiet_mode_) { - for (auto& notification : notifications_) - notification->set_shown_as_popup(true); + for (auto& tuple : notifications_) + tuple.second.shown_as_popup = true; } } Notification* NotificationList::GetNotificationById(const std::string& id) { auto iter = GetNotification(id); if (iter != notifications_.end()) - return iter->get(); + return iter->first.get(); return nullptr; } NotificationList::Notifications NotificationList::GetVisibleNotifications( const NotificationBlockers& blockers) const { Notifications result; - for (const auto& notification : notifications_) { + for (const auto& tuple : notifications_) { bool should_show = true; for (size_t i = 0; i < blockers.size(); ++i) { - if (!blockers[i]->ShouldShowNotification(*notification.get())) { + if (!blockers[i]->ShouldShowNotification(*tuple.first)) { should_show = false; break; } } if (should_show) - result.insert(notification.get()); + result.insert(tuple.first.get()); } return result; @@ -293,7 +306,7 @@ NotificationList::OwnedNotifications::iterator NotificationList::GetNotification(const std::string& id) { for (auto iter = notifications_.begin(); iter != notifications_.end(); ++iter) { - if ((*iter)->id() == id) + if (iter->first->id() == id) return iter; } return notifications_.end(); @@ -308,24 +321,19 @@ void NotificationList::PushNotification( // Ensure that notification.id is unique by erasing any existing // notification with the same id (shouldn't normally happen). auto iter = GetNotification(notification->id()); - bool state_inherited = false; + NotificationState state; if (iter != notifications_.end()) { - notification->CopyState(iter->get()); - state_inherited = true; + state = iter->second; EraseNotification(iter); - } - // Add the notification to the the list and mark it unread and unshown. - if (!state_inherited) { + } else { // TODO(mukai): needs to distinguish if a notification is dismissed by // the quiet mode or user operation. - notification->set_is_read(false); - notification->set_shown_as_popup(message_center_->IsMessageCenterVisible() - || quiet_mode_ - || notification->shown_as_popup()); + state.shown_as_popup = + message_center_->IsMessageCenterVisible() || quiet_mode_; } - // Take ownership. The notification can only be removed from the list - // in EraseNotification(), which will delete it. - notifications_.insert(std::move(notification)); + if (notification->priority() == MIN_PRIORITY) + state.is_read = true; + notifications_.emplace(std::move(notification), state); } } // namespace message_center diff --git a/chromium/ui/message_center/notification_list.h b/chromium/ui/message_center/notification_list.h index c5a7935e93f..21a07abfc37 100644 --- a/chromium/ui/message_center/notification_list.h +++ b/chromium/ui/message_center/notification_list.h @@ -8,6 +8,7 @@ #include <stddef.h> #include <list> +#include <map> #include <set> #include <string> @@ -37,18 +38,19 @@ struct NotifierId; // Comparers used to auto-sort the lists of Notifications. struct MESSAGE_CENTER_EXPORT ComparePriorityTimestampSerial { - bool operator()(Notification* n1, Notification* n2); + bool operator()(Notification* n1, Notification* n2) const; }; struct MESSAGE_CENTER_EXPORT CompareTimestampSerial { - bool operator()(Notification* n1, Notification* n2); + bool operator()(Notification* n1, Notification* n2) const; }; // An adapter to allow use of the comparers above with std::unique_ptr. template <typename PlainCompare> struct UniquePtrCompare { template <typename T> - bool operator()(const std::unique_ptr<T>& n1, const std::unique_ptr<T>& n2) { + bool operator()(const std::unique_ptr<T>& n1, + const std::unique_ptr<T>& n2) const { return PlainCompare()(n1.get(), n2.get()); } }; @@ -56,11 +58,19 @@ struct UniquePtrCompare { // A helper class to manage the list of notifications. class MESSAGE_CENTER_EXPORT NotificationList { public: + struct NotificationState { + bool operator!=(const NotificationState& other) const; + + bool shown_as_popup = false; + bool is_read = false; + }; + // Auto-sorted set. Matches the order in which Notifications are shown in // Notification Center. using Notifications = std::set<Notification*, ComparePriorityTimestampSerial>; using OwnedNotifications = - std::set<std::unique_ptr<Notification>, + std::map<std::unique_ptr<Notification>, + NotificationState, UniquePtrCompare<ComparePriorityTimestampSerial>>; // Auto-sorted set used to return the Notifications to be shown as popup @@ -139,6 +149,8 @@ class MESSAGE_CENTER_EXPORT NotificationList { // NULL. Notification instance is owned by this list. Notification* GetNotificationById(const std::string& id); + void PopupBlocked(const std::string& id); + // Returns all visible notifications, in a (priority-timestamp) order. // Suitable for rendering notifications in a MessageCenter. Notifications GetVisibleNotifications( diff --git a/chromium/ui/message_center/notification_list_unittest.cc b/chromium/ui/message_center/notification_list_unittest.cc index 995358660d6..98ee4f841b2 100644 --- a/chromium/ui/message_center/notification_list_unittest.cc +++ b/chromium/ui/message_center/notification_list_unittest.cc @@ -24,6 +24,8 @@ using base::UTF8ToUTF16; namespace message_center { +using NotificationState = NotificationList::NotificationState; + class NotificationListTest : public testing::Test { public: NotificationListTest() {} @@ -90,7 +92,13 @@ class NotificationListTest : public testing::Test { auto iter = notification_list()->GetNotification(id); if (iter == notification_list()->notifications_.end()) return NULL; - return iter->get(); + return iter->first.get(); + } + + NotificationState GetNotificationState(const std::string& id) { + auto iter = notification_list()->GetNotification(id); + EXPECT_FALSE(iter == notification_list()->notifications_.end()); + return iter->second; } NotificationList* notification_list() { return notification_list_.get(); } @@ -191,8 +199,8 @@ TEST_F(NotificationListTest, UpdateNotificationWithPriority) { std::string old_id; auto old_notification = MakeNotification(&old_id); old_notification->set_priority(DEFAULT_PRIORITY); - old_notification->set_shown_as_popup(true); notification_list()->AddNotification(std::move(old_notification)); + notification_list()->MarkSinglePopupAsShown(old_id, true); EXPECT_EQ(1u, notification_list()->NotificationCount(blockers())); std::string new_id; @@ -210,8 +218,14 @@ TEST_F(NotificationListTest, UpdateNotificationWithPriority) { // again. // In quiet mode, |shown_as_popup| should not be reset , as popup should not // be shown even though the priority is promoted. - EXPECT_EQ(static_cast<bool>(quiet_mode), - (*notifications.begin())->shown_as_popup()); + const NotificationList::PopupNotifications popup_notifications = + notification_list()->GetPopupNotifications(blockers(), nullptr); + if (quiet_mode) { + ASSERT_EQ(0U, popup_notifications.size()); + } else { + ASSERT_EQ(1U, popup_notifications.size()); + EXPECT_EQ(new_id, (*popup_notifications.begin())->id()); + } } } @@ -576,15 +590,15 @@ TEST_F(NotificationListTest, UpdateAfterMarkedAsShown) { EXPECT_EQ(2u, GetPopupCounts()); - const Notification* n1 = GetNotification(id1); - EXPECT_FALSE(n1->shown_as_popup()); - EXPECT_TRUE(n1->IsRead()); + NotificationState n1_state = GetNotificationState(id1); + EXPECT_FALSE(n1_state.shown_as_popup); + EXPECT_TRUE(n1_state.is_read); notification_list()->MarkSinglePopupAsShown(id1, true); - n1 = GetNotification(id1); - EXPECT_TRUE(n1->shown_as_popup()); - EXPECT_TRUE(n1->IsRead()); + n1_state = GetNotificationState(id1); + EXPECT_TRUE(n1_state.shown_as_popup); + EXPECT_TRUE(n1_state.is_read); const std::string replaced("test-replaced-id"); std::unique_ptr<Notification> notification(new Notification( @@ -593,11 +607,11 @@ TEST_F(NotificationListTest, UpdateAfterMarkedAsShown) { NotifierId(NotifierId::APPLICATION, kExtensionId), RichNotificationData(), NULL)); notification_list()->UpdateNotificationMessage(id1, std::move(notification)); - n1 = GetNotification(id1); + Notification* n1 = GetNotification(id1); EXPECT_TRUE(n1 == NULL); - const Notification* nr = GetNotification(replaced); - EXPECT_TRUE(nr->shown_as_popup()); - EXPECT_TRUE(nr->IsRead()); + const NotificationState nr_state = GetNotificationState(replaced); + EXPECT_TRUE(nr_state.shown_as_popup); + EXPECT_TRUE(nr_state.is_read); } TEST_F(NotificationListTest, QuietMode) { @@ -616,19 +630,6 @@ TEST_F(NotificationListTest, QuietMode) { // TODO(mukai): Add test of quiet mode with expiration. } -TEST_F(NotificationListTest, TestPushingShownNotification) { - // Create a notification and mark it as shown. - std::string id1; - std::unique_ptr<Notification> notification(MakeNotification(&id1)); - notification->set_shown_as_popup(true); - - // Call PushNotification on this notification. - notification_list()->PushNotification(std::move(notification)); - - // Ensure it is still marked as shown. - EXPECT_TRUE(GetNotification(id1)->shown_as_popup()); -} - TEST_F(NotificationListTest, TestHasNotificationOfType) { std::string id = AddNotification(); diff --git a/chromium/ui/message_center/public/cpp/notification.cc b/chromium/ui/message_center/public/cpp/notification.cc index dd06417bce3..dce03cfdb17 100644 --- a/chromium/ui/message_center/public/cpp/notification.cc +++ b/chromium/ui/message_center/public/cpp/notification.cc @@ -61,7 +61,7 @@ RichNotificationData::RichNotificationData(const RichNotificationData& other) = RichNotificationData::~RichNotificationData() = default; -Notification::Notification() = default; +Notification::Notification() : serial_number_(g_next_serial_number++) {} Notification::Notification(NotificationType type, const std::string& id, @@ -83,8 +83,6 @@ Notification::Notification(NotificationType type, notifier_id_(notifier_id), optional_fields_(optional_fields), serial_number_(g_next_serial_number++), - shown_as_popup_(false), - is_read_(false), delegate_(std::move(delegate)) {} Notification::Notification(const std::string& id, const Notification& other) @@ -121,18 +119,6 @@ std::unique_ptr<Notification> Notification::DeepCopy( return notification_copy; } -bool Notification::IsRead() const { - return is_read_ || optional_fields_.priority == MIN_PRIORITY; -} - -void Notification::CopyState(Notification* base) { - shown_as_popup_ = base->shown_as_popup(); - is_read_ = base->is_read_; - if (!delegate_.get()) - delegate_ = base->delegate(); - optional_fields_.never_timeout = base->never_timeout(); -} - void Notification::SetButtonIcon(size_t index, const gfx::Image& icon) { if (index >= optional_fields_.buttons.size()) return; @@ -185,7 +171,6 @@ std::unique_ptr<Notification> Notification::CreateSystemNotification( RichNotificationData(), new HandleNotificationClickDelegate(click_callback), gfx::kNoneIcon, SystemNotificationWarningLevel::CRITICAL_WARNING); - notification->set_clickable(true); notification->SetSystemPriority(); return notification; } @@ -227,10 +212,12 @@ std::unique_ptr<Notification> Notification::CreateSystemNotification( } // static -void RegisterVectorIcon(const gfx::VectorIcon& vector_icon) { - g_vector_icon_registry.Get().insert( - std::pair<std::string, const gfx::VectorIcon&>(vector_icon.name, - vector_icon)); +void RegisterVectorIcons( + const std::vector<const gfx::VectorIcon*>& vector_icons) { + for (const gfx::VectorIcon* icon : vector_icons) { + g_vector_icon_registry.Get().insert( + std::pair<std::string, const gfx::VectorIcon&>(icon->name, *icon)); + } } // static diff --git a/chromium/ui/message_center/public/cpp/notification.h b/chromium/ui/message_center/public/cpp/notification.h index c91d0dac645..bbe62a47f33 100644 --- a/chromium/ui/message_center/public/cpp/notification.h +++ b/chromium/ui/message_center/public/cpp/notification.h @@ -145,9 +145,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT RichNotificationData { // depending on visual assistance systems. bool should_make_spoken_feedback_for_popup_updates = true; - // Whether it should be possible for the user to click on the notification. - bool clickable = false; - #if defined(OS_CHROMEOS) // Flag if the notification is pinned. If true, the notification is pinned // and the user can't remove it. @@ -240,10 +237,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { bool include_small_image, bool include_icon_images); - // Copies the internal on-memory state from |base|, i.e. shown_as_popup, - // is_read and never_timeout. - void CopyState(Notification* base); - NotificationType type() const { return type_; } void set_type(NotificationType type) { type_ = type; } @@ -372,15 +365,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { } void SetButtonIcon(size_t index, const gfx::Image& icon); - bool shown_as_popup() const { return shown_as_popup_; } - void set_shown_as_popup(bool shown_as_popup) { - shown_as_popup_ = shown_as_popup; - } - - // Read status in the message center. - bool IsRead() const; - void set_is_read(bool read) { is_read_ = read; } - // Used to keep the order of notifications with the same timestamp. // The notification with lesser serial_number is considered 'older'. unsigned serial_number() { return serial_number_; } @@ -391,9 +375,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { optional_fields_.never_timeout = never_timeout; } - bool clickable() const { return optional_fields_.clickable; } - void set_clickable(bool clickable) { optional_fields_.clickable = clickable; } - bool pinned() const { #if defined(OS_CHROMEOS) return optional_fields_.pinned; @@ -442,11 +423,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { // method explicitly, to avoid setting it accidentally. void SetSystemPriority(); - // Delegate actions. - void Click() const { delegate()->Click(); } - void ButtonClick(int index) const { delegate()->ButtonClick(index); } - void Close(bool by_user) const { delegate()->Close(by_user); } - // Helper method to create a simple system notification. |click_callback| // will be invoked when the notification is clicked. // @@ -512,8 +488,6 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { // TODO(estade): these book-keeping fields should be moved into // NotificationList. unsigned serial_number_; - bool shown_as_popup_; // True if this has been shown as a popup. - bool is_read_; // True if this has been seen in the message center. // A proxy object that allows access back to the JavaScript object that // represents the notification, for firing events. @@ -525,7 +499,8 @@ class MESSAGE_CENTER_PUBLIC_EXPORT Notification { // of icons it expects to see, this allows for icon lookup without // serialization/deserialization. MESSAGE_CENTER_PUBLIC_EXPORT -void RegisterVectorIcon(const gfx::VectorIcon& vector_icon); +void RegisterVectorIcons( + const std::vector<const gfx::VectorIcon*>& vector_icon); MESSAGE_CENTER_PUBLIC_EXPORT const gfx::VectorIcon* GetRegisteredVectorIcon(const std::string& id); diff --git a/chromium/ui/message_center/public/cpp/notification_delegate.cc b/chromium/ui/message_center/public/cpp/notification_delegate.cc index 42b536b97ac..267b2155a7e 100644 --- a/chromium/ui/message_center/public/cpp/notification_delegate.cc +++ b/chromium/ui/message_center/public/cpp/notification_delegate.cc @@ -20,21 +20,11 @@ void ThunkNotificationDelegate::Close(bool by_user) { impl_->Close(by_user); } -void ThunkNotificationDelegate::Click() { +void ThunkNotificationDelegate::Click( + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) { if (impl_) - impl_->Click(); -} - -void ThunkNotificationDelegate::ButtonClick(int button_index) { - if (impl_) - impl_->ButtonClick(button_index); -} - -void ThunkNotificationDelegate::ButtonClickWithReply( - int button_index, - const base::string16& reply) { - if (impl_) - impl_->ButtonClickWithReply(button_index, reply); + impl_->Click(button_index, reply); } void ThunkNotificationDelegate::SettingsClick() { @@ -72,12 +62,9 @@ HandleNotificationClickDelegate::HandleNotificationClickDelegate( HandleNotificationClickDelegate::~HandleNotificationClickDelegate() {} -void HandleNotificationClickDelegate::Click() { - if (!callback_.is_null()) - callback_.Run(base::nullopt); -} - -void HandleNotificationClickDelegate::ButtonClick(int button_index) { +void HandleNotificationClickDelegate::Click( + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) { if (!callback_.is_null()) callback_.Run(button_index); } diff --git a/chromium/ui/message_center/public/cpp/notification_delegate.h b/chromium/ui/message_center/public/cpp/notification_delegate.h index 8f5a5321969..1c2e62c4668 100644 --- a/chromium/ui/message_center/public/cpp/notification_delegate.h +++ b/chromium/ui/message_center/public/cpp/notification_delegate.h @@ -21,22 +21,19 @@ namespace message_center { // Handles actions performed on a notification. class MESSAGE_CENTER_PUBLIC_EXPORT NotificationObserver { public: - // To be called when the desktop notification is closed. If closed by a - // user explicitly (as opposed to timeout/script), |by_user| should be true. + // Called when the desktop notification is closed. If closed by a user + // explicitly (as opposed to timeout/script), |by_user| should be true. virtual void Close(bool by_user) {} - // To be called when a desktop notification is clicked. - virtual void Click() {} + // Called when a desktop notification is clicked. |button_index| is filled in + // if a button was clicked (as opposed to the body of the notification) while + // |reply| is filled in if there was an input field associated with the + // button. + virtual void Click(const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) {} - // To be called when the user clicks a button in a notification. - virtual void ButtonClick(int button_index) {} - - // To be called when the user types a reply to a notification. - virtual void ButtonClickWithReply(int button_index, - const base::string16& reply) {} - - // To be called when the user clicks the settings button in a notification - // which has a DELEGATE settings button action. + // Called when the user clicks the settings button in a notification which has + // a DELEGATE settings button action. virtual void SettingsClick() {} // Called when the user attempts to disable the notification. @@ -58,8 +55,7 @@ class MESSAGE_CENTER_PUBLIC_EXPORT NotificationDelegate // A pass-through which converts the RefCounted requirement to a WeakPtr // requirement. This class replaces the need for individual delegates that pass // through to an actual controller class, and which only exist because the -// actual controller has a strong ownership model. TODO(estade): replace many -// existing NotificationDelegate overrides with an instance of this class. +// actual controller has a strong ownership model. class MESSAGE_CENTER_PUBLIC_EXPORT ThunkNotificationDelegate : public NotificationDelegate { public: @@ -67,10 +63,8 @@ class MESSAGE_CENTER_PUBLIC_EXPORT ThunkNotificationDelegate // NotificationDelegate: void Close(bool by_user) override; - void Click() override; - void ButtonClick(int button_index) override; - void ButtonClickWithReply(int button_index, - const base::string16& reply) override; + void Click(const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) override; void SettingsClick() override; void DisableNotification() override; @@ -102,8 +96,8 @@ class MESSAGE_CENTER_PUBLIC_EXPORT HandleNotificationClickDelegate const base::RepeatingClosure& closure); // NotificationDelegate overrides: - void Click() override; - void ButtonClick(int button_index) override; + void Click(const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) override; protected: ~HandleNotificationClickDelegate() override; diff --git a/chromium/ui/message_center/public/cpp/notification_delegate_unittest.cc b/chromium/ui/message_center/public/cpp/notification_delegate_unittest.cc index 09874d1ceeb..bd600a7ee64 100644 --- a/chromium/ui/message_center/public/cpp/notification_delegate_unittest.cc +++ b/chromium/ui/message_center/public/cpp/notification_delegate_unittest.cc @@ -37,7 +37,7 @@ TEST_F(NotificationDelegateTest, ClickDelegate) { base::Bind(&NotificationDelegateTest::BodyClickCallback, base::Unretained(this))); - delegate->Click(); + delegate->Click(base::nullopt, base::nullopt); EXPECT_EQ(1, callback_count_); } @@ -45,7 +45,7 @@ TEST_F(NotificationDelegateTest, NullClickDelegate) { auto delegate = base::MakeRefCounted<HandleNotificationClickDelegate>(base::Closure()); - delegate->Click(); + delegate->Click(base::nullopt, base::nullopt); EXPECT_EQ(0, callback_count_); } @@ -54,11 +54,11 @@ TEST_F(NotificationDelegateTest, ButtonClickDelegate) { base::Bind(&NotificationDelegateTest::ButtonClickCallback, base::Unretained(this))); - delegate->Click(); + delegate->Click(base::nullopt, base::nullopt); EXPECT_EQ(1, callback_count_); EXPECT_EQ(base::nullopt, last_button_index_); - delegate->ButtonClick(3); + delegate->Click(3, base::nullopt); EXPECT_EQ(2, callback_count_); EXPECT_EQ(3, *last_button_index_); } diff --git a/chromium/ui/message_center/public/mojo/BUILD.gn b/chromium/ui/message_center/public/mojo/BUILD.gn index d6aaf8314fe..59d1d2bf31f 100644 --- a/chromium/ui/message_center/public/mojo/BUILD.gn +++ b/chromium/ui/message_center/public/mojo/BUILD.gn @@ -12,6 +12,7 @@ mojom("mojo") { public_deps = [ "//mojo/common:common_custom_types", + "//mojo/public/mojom/base", "//ui/gfx/image/mojo:interfaces", "//url/mojom:url_mojom_gurl", ] diff --git a/chromium/ui/message_center/public/mojo/notification.mojom b/chromium/ui/message_center/public/mojo/notification.mojom index 1b1b4ba3cd2..823517c3a43 100644 --- a/chromium/ui/message_center/public/mojo/notification.mojom +++ b/chromium/ui/message_center/public/mojo/notification.mojom @@ -4,7 +4,7 @@ module message_center.mojom; -import "mojo/common/time.mojom"; +import "mojo/public/mojom/base/time.mojom"; import "mojo/public/mojom/base/string16.mojom"; import "ui/gfx/image/mojo/image.mojom"; import "ui/message_center/public/mojo/notifier_id.mojom"; @@ -51,7 +51,7 @@ struct ButtonInfo { struct RichNotificationData { int32 priority; bool never_time_out; - mojo.common.mojom.Time timestamp; + mojo_base.mojom.Time timestamp; // |context_message| intentionally omitted. See https://crbug.com/797084 gfx.mojom.ImageSkia? image; gfx.mojom.ImageSkia? small_image; @@ -60,7 +60,6 @@ struct RichNotificationData { mojo_base.mojom.String16 progress_status; array<ButtonInfo> buttons; bool should_make_spoken_feedback_for_popup_updates; - bool clickable; bool pinned; // |vibration_pattern| intentionally omitted // |renotify| intentionally omitted diff --git a/chromium/ui/message_center/public/mojo/notification_struct_traits.cc b/chromium/ui/message_center/public/mojo/notification_struct_traits.cc index 35b84fdf5e0..0a51cb6fa9d 100644 --- a/chromium/ui/message_center/public/mojo/notification_struct_traits.cc +++ b/chromium/ui/message_center/public/mojo/notification_struct_traits.cc @@ -4,8 +4,8 @@ #include "ui/message_center/public/mojo/notification_struct_traits.h" -#include "mojo/common/time_struct_traits.h" #include "mojo/public/cpp/base/string16_mojom_traits.h" +#include "mojo/public/cpp/base/time_mojom_traits.h" #include "ui/gfx/image/mojo/image_skia_struct_traits.h" #include "ui/message_center/public/mojo/notifier_id_struct_traits.h" #include "url/mojom/url_gurl_mojom_traits.h" @@ -112,12 +112,6 @@ bool RichNotificationDataStructTraits:: } // static -bool RichNotificationDataStructTraits::clickable( - const RichNotificationData& r) { - return r.clickable; -} - -// static bool RichNotificationDataStructTraits::pinned(const RichNotificationData& r) { return r.pinned; } @@ -171,7 +165,6 @@ bool RichNotificationDataStructTraits::Read(RichNotificationDataDataView data, out->progress = data.progress(); out->should_make_spoken_feedback_for_popup_updates = data.should_make_spoken_feedback_for_popup_updates(); - out->clickable = data.clickable(); out->pinned = data.pinned(); // Look up the vector icon by ID. This will only work if RegisterVectorIcon @@ -179,8 +172,10 @@ bool RichNotificationDataStructTraits::Read(RichNotificationDataDataView data, std::string icon_id; if (data.ReadVectorSmallImageId(&icon_id) && !icon_id.empty()) { out->vector_small_image = message_center::GetRegisteredVectorIcon(icon_id); - if (!out->vector_small_image) + if (!out->vector_small_image) { LOG(ERROR) << "Couldn't find icon: " + icon_id; + return false; + } } out->accent_color = data.accent_color(); diff --git a/chromium/ui/message_center/public/mojo/notification_struct_traits.h b/chromium/ui/message_center/public/mojo/notification_struct_traits.h index e97e4a6c504..4c35d5f716e 100644 --- a/chromium/ui/message_center/public/mojo/notification_struct_traits.h +++ b/chromium/ui/message_center/public/mojo/notification_struct_traits.h @@ -175,7 +175,6 @@ struct StructTraits<message_center::mojom::RichNotificationDataDataView, const message_center::RichNotificationData& r); static bool should_make_spoken_feedback_for_popup_updates( const message_center::RichNotificationData& r); - static bool clickable(const message_center::RichNotificationData& r); static bool pinned(const message_center::RichNotificationData& r); static const base::string16& accessible_name( const message_center::RichNotificationData& r); diff --git a/chromium/ui/message_center/public/mojo/notifier_id_struct_traits.cc b/chromium/ui/message_center/public/mojo/notifier_id_struct_traits.cc index d2b9f6b3e64..016424fedc8 100644 --- a/chromium/ui/message_center/public/mojo/notifier_id_struct_traits.cc +++ b/chromium/ui/message_center/public/mojo/notifier_id_struct_traits.cc @@ -3,8 +3,6 @@ // found in the LICENSE file. #include "ui/message_center/public/mojo/notifier_id_struct_traits.h" - -#include "mojo/common/common_custom_types_struct_traits.h" #include "url/mojom/url_gurl_mojom_traits.h" namespace mojo { diff --git a/chromium/ui/message_center/public/mojo/struct_traits_unittest.cc b/chromium/ui/message_center/public/mojo/struct_traits_unittest.cc index 59f0263524b..39e8dcae9c2 100644 --- a/chromium/ui/message_center/public/mojo/struct_traits_unittest.cc +++ b/chromium/ui/message_center/public/mojo/struct_traits_unittest.cc @@ -49,7 +49,6 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService { .should_make_spoken_feedback_for_popup_updates, output.rich_notification_data() .should_make_spoken_feedback_for_popup_updates); - EXPECT_EQ(input.clickable(), output.clickable()); EXPECT_EQ(input.accessible_name(), output.accessible_name()); EXPECT_EQ(input.accent_color(), output.accent_color()); EXPECT_EQ(input.should_show_settings_button(), @@ -97,7 +96,6 @@ TEST_F(StructTraitsTest, Notification) { input.set_type(NotificationType::NOTIFICATION_TYPE_PROGRESS); input.set_progress(50); input.set_progress_status(base::ASCIIToUTF16("progress text")); - input.set_clickable(!input.clickable()); input.set_image(gfx::test::CreateImage(48, 48)); input.set_small_image(gfx::test::CreateImage(16, 16)); input.set_accent_color(SK_ColorMAGENTA); diff --git a/chromium/ui/message_center/ui_controller.cc b/chromium/ui/message_center/ui_controller.cc index 997b24750f9..1572243177f 100644 --- a/chromium/ui/message_center/ui_controller.cc +++ b/chromium/ui/message_center/ui_controller.cc @@ -130,14 +130,10 @@ void UiController::OnNotificationUpdated(const std::string& notification_id) { OnMessageCenterChanged(); } -void UiController::OnNotificationClicked(const std::string& notification_id) { - if (popups_visible_) - OnMessageCenterChanged(); -} - -void UiController::OnNotificationButtonClicked( +void UiController::OnNotificationClicked( const std::string& notification_id, - int button_index) { + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) { if (popups_visible_) OnMessageCenterChanged(); } diff --git a/chromium/ui/message_center/ui_controller.h b/chromium/ui/message_center/ui_controller.h index 582ad520d4e..5b489e06bae 100644 --- a/chromium/ui/message_center/ui_controller.h +++ b/chromium/ui/message_center/ui_controller.h @@ -60,9 +60,10 @@ class MESSAGE_CENTER_EXPORT UiController : public MessageCenterObserver { void OnNotificationRemoved(const std::string& notification_id, bool by_user) override; void OnNotificationUpdated(const std::string& notification_id) override; - void OnNotificationClicked(const std::string& notification_id) override; - void OnNotificationButtonClicked(const std::string& notification_id, - int button_index) override; + void OnNotificationClicked( + const std::string& notification_id, + const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) override; void OnNotificationSettingsClicked(bool handled) override; void OnNotificationDisplayed(const std::string& notification_id, const DisplaySource source) override; diff --git a/chromium/ui/message_center/vector_icons/notification_close_button.1x.icon b/chromium/ui/message_center/vector_icons/notification_close_button.1x.icon deleted file mode 100644 index 2bea75dea16..00000000000 --- a/chromium/ui/message_center/vector_icons/notification_close_button.1x.icon +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2017 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. - -CANVAS_DIMENSIONS, 12, -MOVE_TO, 10.5f, 2.36f, -LINE_TO, 9.64f, 1.5f, -LINE_TO, 6, 5.14f, -LINE_TO, 2.36f, 1.5f, -LINE_TO, 1.5f, 2.36f, -LINE_TO, 5.14f, 6, -LINE_TO, 1.5f, 9.64f, -LINE_TO, 2.36f, 10.5f, -LINE_TO, 6, 6.86f, -LINE_TO, 9.64f, 10.5f, -LINE_TO, 10.5f, 9.64f, -LINE_TO, 6.86f, 6, -CLOSE, -END diff --git a/chromium/ui/message_center/vector_icons/notification_close_button.icon b/chromium/ui/message_center/vector_icons/notification_close_button.icon index 5ce70e49153..ced7c26b683 100644 --- a/chromium/ui/message_center/vector_icons/notification_close_button.icon +++ b/chromium/ui/message_center/vector_icons/notification_close_button.icon @@ -15,5 +15,19 @@ LINE_TO, 12, 13.73f, LINE_TO, 18.27f, 20, LINE_TO, 20, 18.27f, LINE_TO, 13.73f, 12, -CLOSE, -END +CLOSE + +CANVAS_DIMENSIONS, 12, +MOVE_TO, 10.5f, 2.36f, +LINE_TO, 9.64f, 1.5f, +LINE_TO, 6, 5.14f, +LINE_TO, 2.36f, 1.5f, +LINE_TO, 1.5f, 2.36f, +LINE_TO, 5.14f, 6, +LINE_TO, 1.5f, 9.64f, +LINE_TO, 2.36f, 10.5f, +LINE_TO, 6, 6.86f, +LINE_TO, 9.64f, 10.5f, +LINE_TO, 10.5f, 9.64f, +LINE_TO, 6.86f, 6, +CLOSE diff --git a/chromium/ui/message_center/vector_icons/notification_expand_less.icon b/chromium/ui/message_center/vector_icons/notification_expand_less.icon index b30ee4211b5..d15fce2af57 100644 --- a/chromium/ui/message_center/vector_icons/notification_expand_less.icon +++ b/chromium/ui/message_center/vector_icons/notification_expand_less.icon @@ -9,5 +9,4 @@ LINE_TO, 13.33f, 13, LINE_TO, 15, 11.31f, R_LINE_TO, -7, -6.89f, R_LINE_TO, -7, 6.89f, -CLOSE, -END +CLOSE diff --git a/chromium/ui/message_center/vector_icons/notification_expand_more.icon b/chromium/ui/message_center/vector_icons/notification_expand_more.icon index b779f5f8e5c..155f6b58871 100644 --- a/chromium/ui/message_center/vector_icons/notification_expand_more.icon +++ b/chromium/ui/message_center/vector_icons/notification_expand_more.icon @@ -9,5 +9,4 @@ LINE_TO, 13.33f, 3, LINE_TO, 15, 4.69f, R_LINE_TO, -7, 6.89f, R_LINE_TO, -7, -6.89f, -CLOSE, -END +CLOSE diff --git a/chromium/ui/message_center/vector_icons/notification_inline_reply.icon b/chromium/ui/message_center/vector_icons/notification_inline_reply.icon index e12869934bf..abb71d49ff5 100644 --- a/chromium/ui/message_center/vector_icons/notification_inline_reply.icon +++ b/chromium/ui/message_center/vector_icons/notification_inline_reply.icon @@ -9,5 +9,4 @@ LINE_TO, 10.04f, 14, LINE_TO, 10, 40.44f, LINE_TO, 64.29f, 48, LINE_TO, 10, 55.56f, -CLOSE, -END
\ No newline at end of file +CLOSE diff --git a/chromium/ui/message_center/vector_icons/notification_settings_button.1x.icon b/chromium/ui/message_center/vector_icons/notification_settings_button.1x.icon deleted file mode 100644 index d9996915b9b..00000000000 --- a/chromium/ui/message_center/vector_icons/notification_settings_button.1x.icon +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2017 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. - -CANVAS_DIMENSIONS, 12, -MOVE_TO, 9.82f, 6.49f, -CUBIC_TO, 9.84f, 6.33f, 9.85f, 6.17f, 9.85f, 6, -CUBIC_TO, 9.85f, 5.84f, 9.84f, 5.67f, 9.82f, 5.51f, -LINE_TO, 10.9f, 4.68f, -CUBIC_TO, 11, 4.61f, 11.03f, 4.47f, 10.97f, 4.37f, -LINE_TO, 9.94f, 2.63f, -CUBIC_TO, 9.87f, 2.53f, 9.74f, 2.48f, 9.62f, 2.53f, -LINE_TO, 8.34f, 3.03f, -CUBIC_TO, 8.08f, 2.83f, 7.79f, 2.66f, 7.48f, 2.54f, -LINE_TO, 7.28f, 1.21f, -CUBIC_TO, 7.26f, 1.09f, 7.15f, 1, 7.03f, 1, -LINE_TO, 4.97f, 1, -CUBIC_TO, 4.84f, 1, 4.74f, 1.09f, 4.72f, 1.21f, -LINE_TO, 4.52f, 2.54f, -CUBIC_TO, 4.21f, 2.66f, 3.92f, 2.83f, 3.66f, 3.03f, -LINE_TO, 2.38f, 2.53f, -CUBIC_TO, 2.26f, 2.48f, 2.13f, 2.53f, 2.06f, 2.63f, -LINE_TO, 1.03f, 4.37f, -CUBIC_TO, 0.97f, 4.47f, 1, 4.61f, 1.1f, 4.68f, -LINE_TO, 2.18f, 5.51f, -CUBIC_TO, 2.16f, 5.67f, 2.14f, 5.84f, 2.14f, 6, -CUBIC_TO, 2.14f, 6.17f, 2.16f, 6.33f, 2.18f, 6.49f, -LINE_TO, 1.1f, 7.32f, -CUBIC_TO, 1, 7.39f, 0.97f, 7.53f, 1.03f, 7.64f, -LINE_TO, 2.06f, 9.37f, -CUBIC_TO, 2.13f, 9.48f, 2.26f, 9.52f, 2.38f, 9.48f, -LINE_TO, 3.66f, 8.97f, -CUBIC_TO, 3.92f, 9.17f, 4.21f, 9.34f, 4.52f, 9.47f, -LINE_TO, 4.72f, 10.79f, -CUBIC_TO, 4.74f, 10.91f, 4.84f, 11, 4.97f, 11, -LINE_TO, 7.03f, 11, -CUBIC_TO, 7.15f, 11, 7.26f, 10.91f, 7.28f, 10.79f, -LINE_TO, 7.47f, 9.47f, -CUBIC_TO, 7.79f, 9.34f, 8.08f, 9.17f, 8.34f, 8.97f, -LINE_TO, 9.62f, 9.48f, -CUBIC_TO, 9.74f, 9.52f, 9.87f, 9.48f, 9.94f, 9.37f, -LINE_TO, 10.96f, 7.64f, -CUBIC_TO, 11.03f, 7.53f, 11, 7.39f, 10.9f, 7.32f, -LINE_TO, 9.82f, 6.49f, -LINE_TO, 9.82f, 6.49f, -CLOSE, -MOVE_TO, 6, 7.5f, -CUBIC_TO, 5.17f, 7.5f, 4.5f, 6.83f, 4.5f, 6, -CUBIC_TO, 4.5f, 5.17f, 5.17f, 4.5f, 6, 4.5f, -CUBIC_TO, 6.83f, 4.5f, 7.5f, 5.17f, 7.5f, 6, -CUBIC_TO, 7.5f, 6.83f, 6.83f, 7.5f, 6, 7.5f, -LINE_TO, 6, 7.5f, -CLOSE, -END diff --git a/chromium/ui/message_center/vector_icons/notification_settings_button.icon b/chromium/ui/message_center/vector_icons/notification_settings_button.icon index ae7e26af6fa..3a0a1d9735a 100644 --- a/chromium/ui/message_center/vector_icons/notification_settings_button.icon +++ b/chromium/ui/message_center/vector_icons/notification_settings_button.icon @@ -50,5 +50,54 @@ CUBIC_TO, 9, 10.34f, 10.34f, 9, 12, 9, CUBIC_TO, 13.66f, 9, 15, 10.34f, 15, 12, CUBIC_TO, 15, 13.66f, 13.66f, 15, 12, 15, LINE_TO, 12, 15, +CLOSE + +CANVAS_DIMENSIONS, 12, +MOVE_TO, 9.82f, 6.49f, +CUBIC_TO, 9.84f, 6.33f, 9.85f, 6.17f, 9.85f, 6, +CUBIC_TO, 9.85f, 5.84f, 9.84f, 5.67f, 9.82f, 5.51f, +LINE_TO, 10.9f, 4.68f, +CUBIC_TO, 11, 4.61f, 11.03f, 4.47f, 10.97f, 4.37f, +LINE_TO, 9.94f, 2.63f, +CUBIC_TO, 9.87f, 2.53f, 9.74f, 2.48f, 9.62f, 2.53f, +LINE_TO, 8.34f, 3.03f, +CUBIC_TO, 8.08f, 2.83f, 7.79f, 2.66f, 7.48f, 2.54f, +LINE_TO, 7.28f, 1.21f, +CUBIC_TO, 7.26f, 1.09f, 7.15f, 1, 7.03f, 1, +LINE_TO, 4.97f, 1, +CUBIC_TO, 4.84f, 1, 4.74f, 1.09f, 4.72f, 1.21f, +LINE_TO, 4.52f, 2.54f, +CUBIC_TO, 4.21f, 2.66f, 3.92f, 2.83f, 3.66f, 3.03f, +LINE_TO, 2.38f, 2.53f, +CUBIC_TO, 2.26f, 2.48f, 2.13f, 2.53f, 2.06f, 2.63f, +LINE_TO, 1.03f, 4.37f, +CUBIC_TO, 0.97f, 4.47f, 1, 4.61f, 1.1f, 4.68f, +LINE_TO, 2.18f, 5.51f, +CUBIC_TO, 2.16f, 5.67f, 2.14f, 5.84f, 2.14f, 6, +CUBIC_TO, 2.14f, 6.17f, 2.16f, 6.33f, 2.18f, 6.49f, +LINE_TO, 1.1f, 7.32f, +CUBIC_TO, 1, 7.39f, 0.97f, 7.53f, 1.03f, 7.64f, +LINE_TO, 2.06f, 9.37f, +CUBIC_TO, 2.13f, 9.48f, 2.26f, 9.52f, 2.38f, 9.48f, +LINE_TO, 3.66f, 8.97f, +CUBIC_TO, 3.92f, 9.17f, 4.21f, 9.34f, 4.52f, 9.47f, +LINE_TO, 4.72f, 10.79f, +CUBIC_TO, 4.74f, 10.91f, 4.84f, 11, 4.97f, 11, +LINE_TO, 7.03f, 11, +CUBIC_TO, 7.15f, 11, 7.26f, 10.91f, 7.28f, 10.79f, +LINE_TO, 7.47f, 9.47f, +CUBIC_TO, 7.79f, 9.34f, 8.08f, 9.17f, 8.34f, 8.97f, +LINE_TO, 9.62f, 9.48f, +CUBIC_TO, 9.74f, 9.52f, 9.87f, 9.48f, 9.94f, 9.37f, +LINE_TO, 10.96f, 7.64f, +CUBIC_TO, 11.03f, 7.53f, 11, 7.39f, 10.9f, 7.32f, +LINE_TO, 9.82f, 6.49f, +LINE_TO, 9.82f, 6.49f, CLOSE, -END +MOVE_TO, 6, 7.5f, +CUBIC_TO, 5.17f, 7.5f, 4.5f, 6.83f, 4.5f, 6, +CUBIC_TO, 4.5f, 5.17f, 5.17f, 4.5f, 6, 4.5f, +CUBIC_TO, 6.83f, 4.5f, 7.5f, 5.17f, 7.5f, 6, +CUBIC_TO, 7.5f, 6.83f, 6.83f, 7.5f, 6, 7.5f, +LINE_TO, 6, 7.5f, +CLOSE diff --git a/chromium/ui/message_center/vector_icons/product.icon b/chromium/ui/message_center/vector_icons/product.icon index 97f8448b152..7068cb20b97 100644 --- a/chromium/ui/message_center/vector_icons/product.icon +++ b/chromium/ui/message_center/vector_icons/product.icon @@ -32,5 +32,4 @@ R_CUBIC_TO, -7.73f, 0, -14, -6.27f, -14, -14, R_CUBIC_TO, 0, -7.73f, 6.27f, -14, 14, -14, R_CUBIC_TO, 7.73f, 0, 14, 6.27f, 14, 14, R_CUBIC_TO, 0, 7.73f, -6.27f, 14, -14, 14, -CLOSE, -END +CLOSE diff --git a/chromium/ui/message_center/views/message_popup_collection.cc b/chromium/ui/message_center/views/message_popup_collection.cc index edd1bbdf3ad..e28f4563676 100644 --- a/chromium/ui/message_center/views/message_popup_collection.cc +++ b/chromium/ui/message_center/views/message_popup_collection.cc @@ -172,7 +172,8 @@ void MessagePopupCollection::UpdateWidgets() { // Create top-level notification. MessageView* view = MessageViewFactory::Create(notification, true); observed_views_.Add(view); - view->SetExpanded(true); + if (!view->IsManuallyExpandedOrCollapsed()) + view->SetExpanded(view->IsAutoExpandingAllowed()); #if !defined(OS_CHROMEOS) view->set_context_menu_controller(context_menu_controller_.get()); @@ -190,8 +191,14 @@ void MessagePopupCollection::UpdateWidgets() { ToastContentsView* toast = new ToastContentsView( notification.id(), alignment_delegate_, weak_factory_.GetWeakPtr()); + + const RichNotificationData& optional_fields = + notification.rich_notification_data(); + bool a11y_feedback_for_updates = + optional_fields.should_make_spoken_feedback_for_popup_updates; + // There will be no contents already since this is a new ToastContentsView. - toast->SetContents(view, /*a11y_feedback_for_updates=*/false); + toast->SetContents(view, a11y_feedback_for_updates); toasts_.push_back(toast); gfx::Size preferred_size = toast->GetPreferredSize(); @@ -343,8 +350,7 @@ void MessagePopupCollection::OnNotificationRemoved( RemoveToast(*iter, /*mark_as_shown=*/true); - if (by_user) - RepositionWidgets(); + DoUpdate(); } void MessagePopupCollection::OnNotificationUpdated( diff --git a/chromium/ui/message_center/views/message_view.cc b/chromium/ui/message_center/views/message_view.cc index 41912b341f7..7c8b678de4a 100644 --- a/chromium/ui/message_center/views/message_view.cc +++ b/chromium/ui/message_center/views/message_view.cc @@ -6,6 +6,7 @@ #include "base/feature_list.h" #include "base/strings/utf_string_conversions.h" +#include "ui/accessibility/ax_enums.mojom.h" #include "ui/accessibility/ax_node_data.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/models/simple_menu_model.h" @@ -17,6 +18,7 @@ #include "ui/message_center/message_center.h" #include "ui/message_center/public/cpp/features.h" #include "ui/message_center/public/cpp/message_center_constants.h" +#include "ui/message_center/views/notification_control_buttons_view.h" #include "ui/strings/grit/ui_strings.h" #include "ui/views/background.h" #include "ui/views/border.h" @@ -94,7 +96,11 @@ MessageView::~MessageView() {} void MessageView::UpdateWithNotification(const Notification& notification) { pinned_ = notification.pinned(); - accessible_name_ = CreateAccessibleName(notification); + base::string16 new_accessible_name = CreateAccessibleName(notification); + if (new_accessible_name != accessible_name_) { + accessible_name_ = new_accessible_name; + NotifyAccessibilityEvent(ax::mojom::Event::kTextChanged, true); + } slide_out_controller_.set_enabled(!GetPinned()); } @@ -124,6 +130,21 @@ void MessageView::SetIsNested() { } } +bool MessageView::IsCloseButtonFocused() const { + auto* control_buttons_view = GetControlButtonsView(); + return control_buttons_view ? control_buttons_view->IsCloseButtonFocused() + : false; +} + +void MessageView::RequestFocusOnCloseButton() { + auto* control_buttons_view = GetControlButtonsView(); + if (!control_buttons_view) + return; + + control_buttons_view->RequestFocusOnCloseButton(); + UpdateControlButtonsVisibility(); +} + void MessageView::SetExpanded(bool expanded) { // Not implemented by default. } @@ -133,6 +154,11 @@ bool MessageView::IsExpanded() const { return false; } +bool MessageView::IsAutoExpandingAllowed() const { + // Allowed by default. + return true; +} + bool MessageView::IsManuallyExpandedOrCollapsed() const { // Not implemented by default. return false; @@ -160,11 +186,18 @@ void MessageView::GetAccessibleNodeData(ui::AXNodeData* node_data) { } bool MessageView::OnMousePressed(const ui::MouseEvent& event) { + return true; +} + +bool MessageView::OnMouseDragged(const ui::MouseEvent& event) { + return true; +} + +void MessageView::OnMouseReleased(const ui::MouseEvent& event) { if (!event.IsOnlyLeftMouseButton()) - return false; + return; MessageCenter::Get()->ClickOnNotification(notification_id_); - return true; } bool MessageView::OnKeyPressed(const ui::KeyEvent& event) { @@ -271,8 +304,17 @@ ui::Layer* MessageView::GetSlideOutLayer() { void MessageView::OnSlideChanged() {} void MessageView::OnSlideOut() { - MessageCenter::Get()->RemoveNotification(notification_id_, - true /* by_user */); + // As a workaround for a MessagePopupCollection bug https://crbug.com/805208, + // pass false to by_user although it is triggered by user. + // TODO(tetsui): Rewrite MessagePopupCollection and remove this hack. + if (pinned_) { + // Also a workaround to not break notification pinning. + MessageCenter::Get()->MarkSinglePopupAsShown( + notification_id_, true /* mark_notification_as_read */); + } else { + MessageCenter::Get()->RemoveNotification(notification_id_, + true /* by_user */); + } } bool MessageView::GetPinned() const { diff --git a/chromium/ui/message_center/views/message_view.h b/chromium/ui/message_center/views/message_view.h index 0cbbaf18602..74df97012b8 100644 --- a/chromium/ui/message_center/views/message_view.h +++ b/chromium/ui/message_center/views/message_view.h @@ -28,14 +28,17 @@ class ScrollView; namespace message_center { +namespace test { +class MessagePopupCollectionTest; +} + class Notification; class NotificationControlButtonsView; // An base class for a notification entry. Contains background and other // elements shared by derived notification views. -class MESSAGE_CENTER_EXPORT MessageView - : public views::InkDropHostView, - public views::SlideOutController::Delegate { +class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView, + public SlideOutController::Delegate { public: static const char kViewClassName[]; @@ -55,13 +58,16 @@ class MESSAGE_CENTER_EXPORT MessageView // Creates a shadow around the notification and changes slide-out behavior. void SetIsNested(); + bool IsCloseButtonFocused() const; + void RequestFocusOnCloseButton(); + virtual NotificationControlButtonsView* GetControlButtonsView() const = 0; - virtual bool IsCloseButtonFocused() const = 0; - virtual void RequestFocusOnCloseButton() = 0; virtual void UpdateControlButtonsVisibility() = 0; + virtual const char* GetMessageViewSubClassName() const = 0; virtual void SetExpanded(bool expanded); virtual bool IsExpanded() const; + virtual bool IsAutoExpandingAllowed() const; virtual bool IsManuallyExpandedOrCollapsed() const; virtual void SetManuallyExpandedOrCollapsed(bool value); @@ -78,16 +84,20 @@ class MESSAGE_CENTER_EXPORT MessageView // views::View void GetAccessibleNodeData(ui::AXNodeData* node_data) override; bool OnMousePressed(const ui::MouseEvent& event) override; + bool OnMouseDragged(const ui::MouseEvent& event) override; + void OnMouseReleased(const ui::MouseEvent& event) override; bool OnKeyPressed(const ui::KeyEvent& event) override; bool OnKeyReleased(const ui::KeyEvent& event) override; void OnPaint(gfx::Canvas* canvas) override; void OnFocus() override; void OnBlur() override; void Layout() override; - const char* GetClassName() const override; void OnGestureEvent(ui::GestureEvent* event) override; + // Subclasses of MessageView shouldn't use this method to distinguish classes. + // Instead, use GetMessageViewSubClassName(). + const char* GetClassName() const final; - // views::SlideOutController::Delegate + // message_center::SlideOutController::Delegate ui::Layer* GetSlideOutLayer() override; void OnSlideChanged() override; void OnSlideOut() override; @@ -113,6 +123,8 @@ class MESSAGE_CENTER_EXPORT MessageView bool is_nested() const { return is_nested_; } private: + friend class test::MessagePopupCollectionTest; + std::string notification_id_; views::View* background_view_ = nullptr; // Owned by views hierarchy. views::ScrollView* scroller_ = nullptr; @@ -124,7 +136,7 @@ class MESSAGE_CENTER_EXPORT MessageView std::unique_ptr<views::Painter> focus_painter_; - views::SlideOutController slide_out_controller_; + message_center::SlideOutController slide_out_controller_; // True if |this| is embedded in another view. Equivalent to |!top_level| in // MessageViewFactory parlance. diff --git a/chromium/ui/message_center/views/notification_header_view.cc b/chromium/ui/message_center/views/notification_header_view.cc index 731494c65f6..b9b83d8e7c3 100644 --- a/chromium/ui/message_center/views/notification_header_view.cc +++ b/chromium/ui/message_center/views/notification_header_view.cc @@ -333,6 +333,17 @@ void NotificationHeaderView::ClearOverflowIndicator() { UpdateSummaryTextVisibility(); } +void NotificationHeaderView::GetAccessibleNodeData(ui::AXNodeData* node_data) { + Button::GetAccessibleNodeData(node_data); + + node_data->SetName(app_name_view_->text()); + node_data->SetDescription(summary_text_view_->text() + + base::ASCIIToUTF16(" ") + timestamp_view_->text()); + + if (is_expanded_) + node_data->AddState(ax::mojom::State::kExpanded); +} + void NotificationHeaderView::SetTimestamp(base::Time past) { timestamp_view_->SetText(FormatToRelativeTime(past)); has_timestamp_ = true; @@ -361,6 +372,7 @@ void NotificationHeaderView::SetExpanded(bool expanded) { expand_button_->SetTooltipText(l10n_util::GetStringUTF16( expanded ? IDS_MESSAGE_CENTER_COLLAPSE_NOTIFICATION : IDS_MESSAGE_CENTER_EXPAND_NOTIFICATION)); + NotifyAccessibilityEvent(ax::mojom::Event::kStateChanged, true); } void NotificationHeaderView::SetAccentColor(SkColor color) { diff --git a/chromium/ui/message_center/views/notification_header_view.h b/chromium/ui/message_center/views/notification_header_view.h index dd40f3f5ebf..b1f14665034 100644 --- a/chromium/ui/message_center/views/notification_header_view.h +++ b/chromium/ui/message_center/views/notification_header_view.h @@ -42,6 +42,9 @@ class NotificationHeaderView : public views::Button { bool IsExpandButtonEnabled(); void SetSubpixelRenderingEnabled(bool enabled); + // views::View: + void GetAccessibleNodeData(ui::AXNodeData* node_data) override; + // Button override: std::unique_ptr<views::InkDrop> CreateInkDrop() override; diff --git a/chromium/ui/message_center/views/notification_view.cc b/chromium/ui/message_center/views/notification_view.cc index 7dc5d5d0346..01604e6e8e7 100644 --- a/chromium/ui/message_center/views/notification_view.cc +++ b/chromium/ui/message_center/views/notification_view.cc @@ -43,7 +43,6 @@ #include "ui/views/layout/fill_layout.h" #include "ui/views/native_cursor.h" #include "ui/views/painter.h" -#include "ui/views/view_targeter.h" #include "ui/views/widget/widget.h" #include "url/gurl.h" @@ -146,33 +145,7 @@ void NotificationItemView::SetVisible(bool visible) { // NotificationView //////////////////////////////////////////////////////////// -views::View* NotificationView::TargetForRect(views::View* root, - const gfx::Rect& rect) { - CHECK_EQ(root, this); - - // TODO(tdanderson): Modify this function to support rect-based event - // targeting. Using the center point of |rect| preserves this function's - // expected behavior for the time being. - gfx::Point point = rect.CenterPoint(); - - // Want to return this for underlying views, otherwise GetCursor is not - // called. But buttons are exceptions, they'll have their own event handlings. - std::vector<views::View*> buttons(action_buttons_.begin(), - action_buttons_.end()); - if (control_buttons_view_->settings_button()) - buttons.push_back(control_buttons_view_->settings_button()); - if (control_buttons_view_->close_button()) - buttons.push_back(control_buttons_view_->close_button()); - - for (size_t i = 0; i < buttons.size(); ++i) { - gfx::Point point_in_child = point; - ConvertPointToTarget(this, buttons[i], &point_in_child); - if (buttons[i]->HitTestPoint(point_in_child)) - return buttons[i]->GetEventHandlerForPoint(point_in_child); - } - - return root; -} +const char NotificationView::kMessageViewSubClassName[] = "NotificationView"; void NotificationView::CreateOrUpdateViews(const Notification& notification) { CreateOrUpdateTitleView(notification); @@ -187,7 +160,7 @@ void NotificationView::CreateOrUpdateViews(const Notification& notification) { } NotificationView::NotificationView(const Notification& notification) - : MessageView(notification), clickable_(notification.clickable()) { + : MessageView(notification) { // Create the top_view_, which collects into a vertical box all content // at the top of the notification (to the right of the icon) except for the // close button. @@ -221,8 +194,6 @@ NotificationView::NotificationView(const Notification& notification) AddChildView(small_image_view_.get()); UpdateControlButtonsVisibilityWithNotification(notification); - SetEventTargeter( - std::unique_ptr<views::ViewTargeter>(new views::ViewTargeter(this))); set_notify_enter_exit_on_child(true); } @@ -336,12 +307,6 @@ void NotificationView::ScrollRectToVisible(const gfx::Rect& rect) { views::View::ScrollRectToVisible(GetLocalBounds()); } -gfx::NativeCursor NotificationView::GetCursor(const ui::MouseEvent& event) { - return clickable_ ? views::GetNativeHandCursor() - : views::View::GetCursor(event); -} - - void NotificationView::OnMouseEntered(const ui::MouseEvent& event) { MessageView::OnMouseEntered(event); UpdateControlButtonsVisibility(); @@ -380,14 +345,6 @@ void NotificationView::ButtonPressed(views::Button* sender, NOTREACHED(); } -bool NotificationView::IsCloseButtonFocused() const { - return control_buttons_view_->IsCloseButtonFocused(); -} - -void NotificationView::RequestFocusOnCloseButton() { - control_buttons_view_->RequestFocusOnCloseButton(); -} - void NotificationView::CreateOrUpdateTitleView( const Notification& notification) { if (notification.title().empty()) { @@ -672,6 +629,10 @@ NotificationControlButtonsView* NotificationView::GetControlButtonsView() return control_buttons_view_; } +const char* NotificationView::GetMessageViewSubClassName() const { + return kMessageViewSubClassName; +} + int NotificationView::GetMessageLineLimit(int title_lines, int width) const { // Image notifications require that the image must be kept flush against // their icons, but we can allow more text if no image. diff --git a/chromium/ui/message_center/views/notification_view.h b/chromium/ui/message_center/views/notification_view.h index f1f0e2fbdef..0f1f8ced86f 100644 --- a/chromium/ui/message_center/views/notification_view.h +++ b/chromium/ui/message_center/views/notification_view.h @@ -13,7 +13,6 @@ #include "ui/message_center/views/message_view.h" #include "ui/views/controls/button/button.h" #include "ui/views/controls/button/image_button.h" -#include "ui/views/view_targeter_delegate.h" namespace views { class ImageView; @@ -31,11 +30,11 @@ class ProportionalImageView; // list) except the custom notification. Future notification types may be // handled by other classes, in which case instances of those classes would be // returned by the Create() factory method below. -class MESSAGE_CENTER_EXPORT NotificationView - : public MessageView, - public views::ButtonListener, - public views::ViewTargeterDelegate { +class MESSAGE_CENTER_EXPORT NotificationView : public MessageView, + public views::ButtonListener { public: + static const char kMessageViewSubClassName[]; + explicit NotificationView(const Notification& notification); ~NotificationView() override; @@ -45,17 +44,15 @@ class MESSAGE_CENTER_EXPORT NotificationView void Layout() override; void OnFocus() override; void ScrollRectToVisible(const gfx::Rect& rect) override; - gfx::NativeCursor GetCursor(const ui::MouseEvent& event) override; void OnMouseEntered(const ui::MouseEvent& event) override; void OnMouseExited(const ui::MouseEvent& event) override; // Overridden from MessageView: void UpdateWithNotification(const Notification& notification) override; void ButtonPressed(views::Button* sender, const ui::Event& event) override; - bool IsCloseButtonFocused() const override; - void RequestFocusOnCloseButton() override; void UpdateControlButtonsVisibility() override; NotificationControlButtonsView* GetControlButtonsView() const override; + const char* GetMessageViewSubClassName() const final; private: FRIEND_TEST_ALL_PREFIXES(NotificationViewTest, CreateOrUpdateTest); @@ -71,9 +68,6 @@ class MESSAGE_CENTER_EXPORT NotificationView friend class NotificationViewTest; - // views::ViewTargeterDelegate: - views::View* TargetForRect(views::View* root, const gfx::Rect& rect) override; - void CreateOrUpdateViews(const Notification& notification); void CreateOrUpdateTitleView(const Notification& notification); @@ -101,9 +95,6 @@ class MESSAGE_CENTER_EXPORT NotificationView // Shrink the topmost label not to be covered by the control button. void ShrinkTopmostLabel(); - // Describes whether the view should display a hand pointer or not. - bool clickable_; - // Weak references to NotificationView descendants owned by their parents. views::View* top_view_ = nullptr; BoundedLabel* title_view_ = nullptr; diff --git a/chromium/ui/message_center/views/notification_view_md.cc b/chromium/ui/message_center/views/notification_view_md.cc index eaf1c4852b6..8d26e265a09 100644 --- a/chromium/ui/message_center/views/notification_view_md.cc +++ b/chromium/ui/message_center/views/notification_view_md.cc @@ -12,6 +12,8 @@ #include "components/url_formatter/elide_url.h" #include "ui/base/cursor/cursor.h" #include "ui/base/l10n/l10n_util.h" +#include "ui/events/base_event_utils.h" +#include "ui/events/gesture_detection/gesture_provider_config_helper.h" #include "ui/gfx/canvas.h" #include "ui/gfx/color_palette.h" #include "ui/gfx/geometry/size.h" @@ -42,7 +44,6 @@ #include "ui/views/layout/box_layout.h" #include "ui/views/layout/fill_layout.h" #include "ui/views/native_cursor.h" -#include "ui/views/view_targeter.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" @@ -169,6 +170,10 @@ class ClickActivator : public ui::EventHandler { } // anonymous namespace +// static +const char NotificationViewMD::kMessageViewSubClassName[] = + "NotificationViewMD"; + // ItemView //////////////////////////////////////////////////////////////////// ItemView::ItemView(const NotificationItem& item) { @@ -533,44 +538,6 @@ class InlineSettingsRadioButton : public views::RadioButton { // NotificationViewMD // //////////////////////////////////////////////////////////// -views::View* NotificationViewMD::TargetForRect(views::View* root, - const gfx::Rect& rect) { - CHECK_EQ(root, this); - - // TODO(tetsui): Modify this function to support rect-based event - // targeting. Using the center point of |rect| preserves this function's - // expected behavior for the time being. - gfx::Point point = rect.CenterPoint(); - - // Want to return this for underlying views, otherwise GetCursor is not - // called. But buttons are exceptions, they'll have their own event handlings. - std::vector<views::View*> buttons; - if (header_row_->expand_button()) - buttons.push_back(header_row_->expand_button()); - buttons.push_back(header_row_); - - if (action_buttons_row_->visible()) { - buttons.insert(buttons.end(), action_buttons_.begin(), - action_buttons_.end()); - } - if (inline_reply_->visible()) - buttons.push_back(inline_reply_); - if (settings_row_) { - buttons.push_back(block_all_button_); - buttons.push_back(dont_block_button_); - buttons.push_back(settings_done_button_); - } - - for (size_t i = 0; i < buttons.size(); ++i) { - gfx::Point point_in_child = point; - ConvertPointToTarget(this, buttons[i], &point_in_child); - if (buttons[i]->HitTestPoint(point_in_child)) - return buttons[i]->GetEventHandlerForPoint(point_in_child); - } - - return root; -} - void NotificationViewMD::CreateOrUpdateViews(const Notification& notification) { CreateOrUpdateContextTitleView(notification); CreateOrUpdateTitleView(notification); @@ -591,8 +558,7 @@ void NotificationViewMD::CreateOrUpdateViews(const Notification& notification) { NotificationViewMD::NotificationViewMD(const Notification& notification) : MessageView(notification), - ink_drop_container_(new views::InkDropContainerView()), - clickable_(notification.clickable()) { + ink_drop_container_(new views::InkDropContainerView()) { SetLayoutManager(std::make_unique<views::BoxLayout>( views::BoxLayout::kVertical, gfx::Insets(), 0)); @@ -657,8 +623,6 @@ NotificationViewMD::NotificationViewMD(const Notification& notification) CreateOrUpdateViews(notification); UpdateControlButtonsVisibilityWithNotification(notification); - SetEventTargeter( - std::unique_ptr<views::ViewTargeter>(new views::ViewTargeter(this))); set_notify_enter_exit_on_child(true); click_activator_ = std::make_unique<ClickActivator>(this); @@ -721,30 +685,28 @@ void NotificationViewMD::ScrollRectToVisible(const gfx::Rect& rect) { views::View::ScrollRectToVisible(GetLocalBounds()); } -gfx::NativeCursor NotificationViewMD::GetCursor(const ui::MouseEvent& event) { - // Do not change the cursor on a notification that isn't clickable. - if (!clickable_) - return views::View::GetCursor(event); - - // Do not change the cursor on the actions row. - if (expanded_) { - DCHECK(actions_row_); - gfx::Point point_in_child = event.location(); - ConvertPointToTarget(this, actions_row_, &point_in_child); - if (actions_row_->HitTestPoint(point_in_child)) - return views::View::GetCursor(event); - } - - // Do not change the cursor when inline settings is shown. - if (settings_row_ && settings_row_->visible()) - return views::View::GetCursor(event); +bool NotificationViewMD::OnMousePressed(const ui::MouseEvent& event) { + last_mouse_pressed_timestamp_ = base::TimeTicks(event.time_stamp()); + return true; +} - return views::GetNativeHandCursor(); +bool NotificationViewMD::OnMouseDragged(const ui::MouseEvent& event) { + return true; } -bool NotificationViewMD::OnMousePressed(const ui::MouseEvent& event) { +void NotificationViewMD::OnMouseReleased(const ui::MouseEvent& event) { if (!event.IsOnlyLeftMouseButton()) - return false; + return; + + // The mouse has been clicked for a long time. + if (ui::EventTimeStampToSeconds(event.time_stamp()) - + ui::EventTimeStampToSeconds(last_mouse_pressed_timestamp_) > + ui::GetGestureProviderConfig( + ui::GestureProviderConfigType::CURRENT_PLATFORM) + .gesture_detector_config.longpress_timeout.InSecondsF()) { + ToggleInlineSettings(event); + return; + } // Ignore click of actions row outside action buttons. if (expanded_) { @@ -752,14 +714,14 @@ bool NotificationViewMD::OnMousePressed(const ui::MouseEvent& event) { gfx::Point point_in_child = event.location(); ConvertPointToTarget(this, actions_row_, &point_in_child); if (actions_row_->HitTestPoint(point_in_child)) - return true; + return; } // Ignore clicks of outside region when inline settings is shown. if (settings_row_ && settings_row_->visible()) - return true; + return; - return MessageView::OnMousePressed(event); + MessageView::OnMouseReleased(event); } void NotificationViewMD::OnMouseEvent(ui::MouseEvent* event) { @@ -776,6 +738,14 @@ void NotificationViewMD::OnMouseEvent(ui::MouseEvent* event) { View::OnMouseEvent(event); } +void NotificationViewMD::OnGestureEvent(ui::GestureEvent* event) { + if (event->type() == ui::ET_GESTURE_LONG_TAP) { + ToggleInlineSettings(*event); + return; + } + MessageView::OnGestureEvent(event); +} + void NotificationViewMD::UpdateWithNotification( const Notification& notification) { MessageView::UpdateWithNotification(notification); @@ -848,14 +818,6 @@ void NotificationViewMD::OnNotificationInputSubmit(size_t index, index, text); } -bool NotificationViewMD::IsCloseButtonFocused() const { - return control_buttons_view_->IsCloseButtonFocused(); -} - -void NotificationViewMD::RequestFocusOnCloseButton() { - control_buttons_view_->RequestFocusOnCloseButton(); -} - void NotificationViewMD::CreateOrUpdateContextTitleView( const Notification& notification) { header_row_->SetAccentColor(notification.accent_color() == SK_ColorTRANSPARENT @@ -1274,7 +1236,8 @@ void NotificationViewMD::UpdateViewForExpandedState(bool expanded) { } void NotificationViewMD::ToggleInlineSettings(const ui::Event& event) { - DCHECK(settings_row_); + if (!settings_row_) + return; bool inline_settings_visible = !settings_row_->visible(); @@ -1303,9 +1266,10 @@ void NotificationViewMD::ToggleInlineSettings(const ui::Event& event) { // among NotificationView and ArcNotificationView. void NotificationViewMD::UpdateControlButtonsVisibility() { const bool target_visibility = - AlwaysShowControlButtons() || IsMouseHovered() || - control_buttons_view_->IsCloseButtonFocused() || - control_buttons_view_->IsSettingsButtonFocused(); + (AlwaysShowControlButtons() || IsMouseHovered() || + control_buttons_view_->IsCloseButtonFocused() || + control_buttons_view_->IsSettingsButtonFocused()) && + !GetPinned(); control_buttons_view_->SetVisible(target_visibility); } @@ -1344,13 +1308,17 @@ void NotificationViewMD::OnSettingsButtonPressed(const ui::Event& event) { MessageView::OnSettingsButtonPressed(event); } +const char* NotificationViewMD::GetMessageViewSubClassName() const { + return kMessageViewSubClassName; +} + void NotificationViewMD::Activate() { GetWidget()->widget_delegate()->set_can_activate(true); GetWidget()->Activate(); } void NotificationViewMD::AddBackgroundAnimation(const ui::Event& event) { - SetInkDropMode(InkDropMode::ON); + SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER); // In case the animation is triggered from keyboard operation. if (!event.IsLocatedEvent()) { AnimateInkDrop(views::InkDropState::ACTION_PENDING, nullptr); diff --git a/chromium/ui/message_center/views/notification_view_md.h b/chromium/ui/message_center/views/notification_view_md.h index d9ed79d902b..d4404224f49 100644 --- a/chromium/ui/message_center/views/notification_view_md.h +++ b/chromium/ui/message_center/views/notification_view_md.h @@ -10,6 +10,7 @@ #include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/optional.h" +#include "base/time/time.h" #include "ui/message_center/message_center_export.h" #include "ui/message_center/views/message_view.h" #include "ui/views/animation/ink_drop_observer.h" @@ -18,7 +19,6 @@ #include "ui/views/controls/button/label_button.h" #include "ui/views/controls/textfield/textfield.h" #include "ui/views/controls/textfield/textfield_controller.h" -#include "ui/views/view_targeter_delegate.h" namespace views { class ImageButton; @@ -218,9 +218,10 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD : public MessageView, public views::InkDropObserver, public NotificationInputDelegate, - public views::ButtonListener, - public views::ViewTargeterDelegate { + public views::ButtonListener { public: + static const char kMessageViewSubClassName[]; + explicit NotificationViewMD(const Notification& notification); ~NotificationViewMD() override; @@ -233,9 +234,11 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD void Layout() override; void OnFocus() override; void ScrollRectToVisible(const gfx::Rect& rect) override; - gfx::NativeCursor GetCursor(const ui::MouseEvent& event) override; bool OnMousePressed(const ui::MouseEvent& event) override; + bool OnMouseDragged(const ui::MouseEvent& event) override; + void OnMouseReleased(const ui::MouseEvent& event) override; void OnMouseEvent(ui::MouseEvent* event) override; + void OnGestureEvent(ui::GestureEvent* event) override; // Overridden from views::InkDropHostView: void AddInkDropLayer(ui::Layer* ink_drop_layer) override; @@ -246,16 +249,14 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD // Overridden from MessageView: void UpdateWithNotification(const Notification& notification) override; void ButtonPressed(views::Button* sender, const ui::Event& event) override; - bool IsCloseButtonFocused() const override; - void RequestFocusOnCloseButton() override; void UpdateControlButtonsVisibility() override; NotificationControlButtonsView* GetControlButtonsView() const override; bool IsExpanded() const override; void SetExpanded(bool expanded) override; bool IsManuallyExpandedOrCollapsed() const override; void SetManuallyExpandedOrCollapsed(bool value) override; - void OnSettingsButtonPressed(const ui::Event& event) override; + const char* GetMessageViewSubClassName() const final; // views::InkDropObserver: void InkDropAnimationStarted() override; @@ -265,9 +266,6 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD void OnNotificationInputSubmit(size_t index, const base::string16& text) override; - // views::ViewTargeterDelegate: - views::View* TargetForRect(views::View* root, const gfx::Rect& rect) override; - private: FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, CreateOrUpdateTest); FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, TestIconSizing); @@ -357,6 +355,8 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD std::unique_ptr<ui::EventHandler> click_activator_; + base::TimeTicks last_mouse_pressed_timestamp_; + DISALLOW_COPY_AND_ASSIGN(NotificationViewMD); }; diff --git a/chromium/ui/message_center/views/notification_view_md_unittest.cc b/chromium/ui/message_center/views/notification_view_md_unittest.cc index f8574e2975f..ae5a86196de 100644 --- a/chromium/ui/message_center/views/notification_view_md_unittest.cc +++ b/chromium/ui/message_center/views/notification_view_md_unittest.cc @@ -37,26 +37,23 @@ class NotificationTestDelegate : public NotificationDelegate { public: NotificationTestDelegate() = default; - void ButtonClick(int button_index) override { - if (!expecting_button_click_) - ADD_FAILURE() << "ClickOnNotificationButton should not be invoked."; - clicked_button_index_ = button_index; - } - - void ButtonClickWithReply(int button_index, - const base::string16& reply) override { - if (!expecting_reply_submission_) { - ADD_FAILURE() - << "ClickOnNotificationButtonWithReply should not be invoked."; - } - - clicked_button_index_ = button_index; - submitted_reply_string_ = reply; + void Click(const base::Optional<int>& button_index, + const base::Optional<base::string16>& reply) override { + if (!button_index) + return; + + if (!reply && !expecting_button_click_) + ADD_FAILURE() << "Click should not be invoked with a button index."; + if (reply && !expecting_reply_submission_) + ADD_FAILURE() << "Click should not be invoked with a reply."; + + clicked_button_index_ = *button_index; + submitted_reply_string_ = reply.value_or(base::string16()); } void Reset() { clicked_button_index_ = -1; - submitted_reply_string_ = base::EmptyString16(); + submitted_reply_string_.clear(); } void DisableNotification() override { disable_notification_called_ = true; } @@ -791,7 +788,7 @@ TEST_F(NotificationViewMDTest, InlineSettings) { EXPECT_FALSE(notification_view()->settings_row_->visible()); gfx::Point settings_cursor_location(1, 1); views::View::ConvertPointToScreen( - notification_view()->control_buttons_view_.get()->settings_button(), + notification_view()->control_buttons_view_->settings_button(), &settings_cursor_location); ui::test::EventGenerator generator(widget()->GetNativeWindow()); generator.MoveMouseTo(settings_cursor_location); diff --git a/chromium/ui/message_center/views/slide_out_controller.cc b/chromium/ui/message_center/views/slide_out_controller.cc index efcf8e027d1..6a863de1b87 100644 --- a/chromium/ui/message_center/views/slide_out_controller.cc +++ b/chromium/ui/message_center/views/slide_out_controller.cc @@ -8,7 +8,7 @@ #include "ui/compositor/scoped_layer_animation_settings.h" #include "ui/gfx/transform.h" -namespace views { +namespace message_center { SlideOutController::SlideOutController(ui::EventTarget* target, Delegate* delegate) @@ -108,4 +108,4 @@ void SlideOutController::OnImplicitAnimationsCompleted() { delegate_->OnSlideOut(); } -} // namespace views +} // namespace message_center diff --git a/chromium/ui/message_center/views/slide_out_controller.h b/chromium/ui/message_center/views/slide_out_controller.h index 5b01df2d98e..c3f134eb270 100644 --- a/chromium/ui/message_center/views/slide_out_controller.h +++ b/chromium/ui/message_center/views/slide_out_controller.h @@ -8,15 +8,16 @@ #include "base/macros.h" #include "ui/compositor/layer_animation_observer.h" #include "ui/events/scoped_target_handler.h" +#include "ui/message_center/message_center_export.h" #include "ui/views/view.h" -#include "ui/views/views_export.h" -namespace views { +namespace message_center { // This class contains logic to control sliding out of a layer in response to // swipes, i.e. gesture scroll events. -class SlideOutController : public ui::EventHandler, - public ui::ImplicitAnimationObserver { +class MESSAGE_CENTER_EXPORT SlideOutController + : public ui::EventHandler, + public ui::ImplicitAnimationObserver { public: class Delegate { public: @@ -59,6 +60,6 @@ class SlideOutController : public ui::EventHandler, DISALLOW_COPY_AND_ASSIGN(SlideOutController); }; -} // namespace views +} // namespace message_center #endif // UI_MESSAGE_CENTER_VIEWS_SLIDE_OUT_CONTROLLER_H_ diff --git a/chromium/ui/message_center/views/toast_contents_view.cc b/chromium/ui/message_center/views/toast_contents_view.cc index 285ca0b30a2..23c84cf6d59 100644 --- a/chromium/ui/message_center/views/toast_contents_view.cc +++ b/chromium/ui/message_center/views/toast_contents_view.cc @@ -11,6 +11,7 @@ #include "base/memory/weak_ptr.h" #include "base/time/time.h" #include "build/build_config.h" +#include "ui/accessibility/ax_enums.mojom.h" #include "ui/accessibility/ax_node_data.h" #include "ui/aura/window.h" #include "ui/aura/window_targeter.h" @@ -84,17 +85,11 @@ ToastContentsView::~ToastContentsView() { void ToastContentsView::SetContents(MessageView* view, bool a11y_feedback_for_updates) { message_view_ = view; - bool already_has_contents = child_count() > 0; RemoveAllChildViews(true); AddChildView(view); UpdatePreferredSize(); - - // If it has the contents already, this invocation means an update of the - // popup toast, and the new contents should be read through a11y feature. - // The notification type should be ALERT, otherwise the accessibility message - // won't be read for this view which returns ROLE_WINDOW. - if (already_has_contents && a11y_feedback_for_updates) - NotifyAccessibilityEvent(ax::mojom::Event::kAlert, false); + if (a11y_feedback_for_updates) + NotifyAccessibilityEvent(ax::mojom::Event::kAlert, true); } void ToastContentsView::UpdateContents(const Notification& notification, @@ -104,7 +99,7 @@ void ToastContentsView::UpdateContents(const Notification& notification, message_view->UpdateWithNotification(notification); UpdatePreferredSize(); if (a11y_feedback_for_updates) - NotifyAccessibilityEvent(ax::mojom::Event::kAlert, false); + NotifyAccessibilityEvent(ax::mojom::Event::kAlert, true); } void ToastContentsView::RevealWithAnimation(gfx::Point origin) { @@ -316,7 +311,7 @@ void ToastContentsView::UpdatePreferredSize() { void ToastContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) { if (child_count() > 0) child_at(0)->GetAccessibleNodeData(node_data); - node_data->role = ax::mojom::Role::kWindow; + node_data->role = ax::mojom::Role::kAlertDialog; } const char* ToastContentsView::GetClassName() const { |