Notifications: Retain button hover state during content updates.

Rapidly updating notifications (like progressbar indicators) can
be hard to interact with if the state resets with each update.

BUG=368025

Review URL: https://codereview.chromium.org/273173003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272609 0039d316-1c4b-4281-b951-d872f2087c98

ui/message_center/views/notification_button.cc

@@ -98,6 +98,14 @@ void NotificationButton::OnBlur() {
   SchedulePaint();
 }
 
+void NotificationButton::ViewHierarchyChanged(
+    const ViewHierarchyChangedDetails& details) {
+  // We disable view hierarchy change detection in the parent
+  // because it resets the hoverstate, which we do not want
+  // when we update the view to contain a new label or image.
+  views::View::ViewHierarchyChanged(details);
+}
+
 void NotificationButton::StateChanged() {
   if (state() == STATE_HOVERED || state() == STATE_PRESSED) {
     set_background(views::Background::CreateSolidBackground(

ui/message_center/views/notification_button.h

@@ -32,6 +32,8 @@ class NotificationButton : public views::CustomButton {
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
   virtual void OnFocus() OVERRIDE;
   virtual void OnBlur() OVERRIDE;
+  virtual void ViewHierarchyChanged(
+      const ViewHierarchyChangedDetails& details) OVERRIDE;
 
   // Overridden from views::CustomButton:
   virtual void StateChanged() OVERRIDE;

ui/message_center/views/notification_view.cc

@@ -5,6 +5,7 @@
 #include "ui/message_center/views/notification_view.h"
 
 #include "base/command_line.h"
+#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "grit/ui_resources.h"
@@ -447,7 +448,8 @@ views::View* NotificationView::GetEventHandlerForRect(const gfx::Rect& rect) {
 
   // 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_);
+  std::vector<views::View*> buttons(action_buttons_.begin(),
+                                    action_buttons_.end());
   buttons.push_back(close_button());
 
   for (size_t i = 0; i < buttons.size(); ++i) {
@@ -698,29 +700,45 @@ void NotificationView::CreateOrUpdateImageView(
 
 void NotificationView::CreateOrUpdateActionButtonViews(
     const Notification& notification) {
-  for (size_t i = 0; i < separators_.size(); ++i)
-    delete separators_[i];
-  separators_.clear();
+  std::vector<ButtonInfo> buttons = notification.buttons();
+  bool new_buttons = action_buttons_.size() != buttons.size();
 
-  for (size_t i = 0; i < action_buttons_.size(); ++i)
-    delete action_buttons_[i];
-  action_buttons_.clear();
+  if (new_buttons || buttons.size() == 0) {
+    // STLDeleteElements also clears the container.
+    STLDeleteElements(&separators_);
+    STLDeleteElements(&action_buttons_);
+  }
 
   DCHECK(bottom_view_);
   DCHECK_EQ(this, bottom_view_->parent());
 
-  std::vector<ButtonInfo> buttons = notification.buttons();
   for (size_t i = 0; i < buttons.size(); ++i) {
-    views::View* separator = new views::ImageView();
-    separator->SetBorder(MakeSeparatorBorder(1, 0, kButtonSeparatorColor));
-    separators_.push_back(separator);
-    bottom_view_->AddChildView(separator);
-    NotificationButton* button = new NotificationButton(this);
     ButtonInfo button_info = buttons[i];
-    button->SetTitle(button_info.title);
-    button->SetIcon(button_info.icon.AsImageSkia());
-    action_buttons_.push_back(button);
-    bottom_view_->AddChildView(button);
+    if (new_buttons) {
+      views::View* separator = new views::ImageView();
+      separator->SetBorder(MakeSeparatorBorder(1, 0, kButtonSeparatorColor));
+      separators_.push_back(separator);
+      bottom_view_->AddChildView(separator);
+      NotificationButton* button = new NotificationButton(this);
+      button->SetTitle(button_info.title);
+      button->SetIcon(button_info.icon.AsImageSkia());
+      action_buttons_.push_back(button);
+      bottom_view_->AddChildView(button);
+    } else {
+      action_buttons_[i]->SetTitle(button_info.title);
+      action_buttons_[i]->SetIcon(button_info.icon.AsImageSkia());
+      action_buttons_[i]->SchedulePaint();
+      action_buttons_[i]->Layout();
+    }
+  }
+
+  if (new_buttons) {
+    Layout();
+    views::Widget* widget = GetWidget();
+    if (widget != NULL) {
+      widget->SetSize(widget->GetContentsView()->GetPreferredSize());
+      GetWidget()->SynthesizeMouseMoveEvent();
+    }
   }
 }
 

ui/message_center/views/notification_view.h

@@ -19,6 +19,7 @@ namespace message_center {
 class BoundedLabel;
 class MessageCenter;
 class MessageCenterController;
+class NotificationButton;
 class NotificationView;
 class PaddedButton;
 
@@ -72,6 +73,8 @@ class MESSAGE_CENTER_EXPORT NotificationView : public MessageView,
  private:
   FRIEND_TEST_ALL_PREFIXES(NotificationViewTest, CreateOrUpdateTest);
   FRIEND_TEST_ALL_PREFIXES(NotificationViewTest, TestLineLimits);
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewTest, UpdateButtonsStateTest);
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewTest, UpdateButtonCountTest);
 
   void CreateOrUpdateViews(const Notification& notification);
   void SetAccessibleName(const Notification& notification);
@@ -103,7 +106,7 @@ class MESSAGE_CENTER_EXPORT NotificationView : public MessageView,
   views::View* bottom_view_;
   views::View* image_view_;
   views::ProgressBar* progress_bar_view_;
-  std::vector<views::View*> action_buttons_;
+  std::vector<NotificationButton*> action_buttons_;
   std::vector<views::View*> separators_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationView);

ui/message_center/views/notification_view_unittest.cc

@@ -13,12 +13,16 @@
 #include "ui/message_center/notification_list.h"
 #include "ui/message_center/notification_types.h"
 #include "ui/message_center/views/message_center_controller.h"
+#include "ui/message_center/views/notification_button.h"
+#include "ui/views/layout/fill_layout.h"
+#include "ui/views/test/views_test_base.h"
+#include "ui/views/widget/widget_delegate.h"
 
 namespace message_center {
 
 /* Test fixture ***************************************************************/
 
-class NotificationViewTest : public testing::Test,
+class NotificationViewTest : public views::ViewsTestBase,
                              public MessageCenterController {
  public:
   NotificationViewTest();
@@ -27,6 +31,7 @@ class NotificationViewTest : public testing::Test,
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
+  views::Widget* widget() { return notification_view_->GetWidget(); }
   NotificationView* notification_view() { return notification_view_.get(); }
   Notification* notification() { return notification_.get(); }
   RichNotificationData* data() { return data_.get(); }
@@ -55,6 +60,12 @@ class NotificationViewTest : public testing::Test,
     return bitmap;
   }
 
+  std::vector<ButtonInfo> CreateButtons(int number) {
+    ButtonInfo info(base::ASCIIToUTF16("Test button title."));
+    info.icon = CreateTestImage(4, 4);
+    return std::vector<ButtonInfo>(number, info);
+  }
+
  private:
   scoped_ptr<RichNotificationData> data_;
   scoped_ptr<Notification> notification_;
@@ -70,6 +81,7 @@ NotificationViewTest::~NotificationViewTest() {
 }
 
 void NotificationViewTest::SetUp() {
+  views::ViewsTestBase::SetUp();
   // Create a dummy notification.
   SkBitmap bitmap;
   data_.reset(new RichNotificationData());
@@ -89,10 +101,20 @@ void NotificationViewTest::SetUp() {
   // Then create a new NotificationView with that single notification.
   notification_view_.reset(
       NotificationView::Create(this, *notification_, true));
+  notification_view_->set_owned_by_client();
+
+  views::Widget::InitParams init_params(
+      CreateParams(views::Widget::InitParams::TYPE_POPUP));
+  views::Widget* widget = new views::Widget();
+  widget->Init(init_params);
+  widget->SetContentsView(notification_view_.get());
+  widget->SetSize(notification_view_->GetPreferredSize());
 }
 
 void NotificationViewTest::TearDown() {
+  widget()->Close();
   notification_view_.reset();
+  views::ViewsTestBase::TearDown();
 }
 
 void NotificationViewTest::ClickOnNotification(
@@ -175,4 +197,84 @@ TEST_F(NotificationViewTest, TestLineLimits) {
   EXPECT_EQ(0, notification_view()->GetMessageLineLimit(2, 360));
 }
 
+TEST_F(NotificationViewTest, UpdateButtonsStateTest) {
+  notification()->set_buttons(CreateButtons(2));
+  notification_view()->CreateOrUpdateViews(*notification());
+  widget()->Show();
+
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[0]->background());
+
+  // Now construct a mouse move event 1 pixel inside the boundary of the action
+  // button.
+  gfx::Point cursor_location(1, 1);
+  views::View::ConvertPointToWidget(notification_view()->action_buttons_[0],
+                                    &cursor_location);
+  ui::MouseEvent move(ui::ET_MOUSE_MOVED,
+                      cursor_location,
+                      cursor_location,
+                      ui::EF_NONE,
+                      ui::EF_NONE);
+  widget()->OnMouseEvent(&move);
+
+  EXPECT_TRUE(NULL != notification_view()->action_buttons_[0]->background());
+
+  notification_view()->CreateOrUpdateViews(*notification());
+
+  EXPECT_TRUE(NULL != notification_view()->action_buttons_[0]->background());
+
+  // Now construct a mouse move event 1 pixel outside the boundary of the
+  // widget.
+  cursor_location = gfx::Point(-1, -1);
+  move = ui::MouseEvent(ui::ET_MOUSE_MOVED,
+                        cursor_location,
+                        cursor_location,
+                        ui::EF_NONE,
+                        ui::EF_NONE);
+  widget()->OnMouseEvent(&move);
+
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[0]->background());
+}
+
+TEST_F(NotificationViewTest, UpdateButtonCountTest) {
+  notification()->set_buttons(CreateButtons(2));
+  notification_view()->CreateOrUpdateViews(*notification());
+  widget()->Show();
+
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[0]->background());
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[1]->background());
+
+  // Now construct a mouse move event 1 pixel inside the boundary of the action
+  // button.
+  gfx::Point cursor_location(1, 1);
+  views::View::ConvertPointToWidget(notification_view()->action_buttons_[0],
+                                    &cursor_location);
+  ui::MouseEvent move(ui::ET_MOUSE_MOVED,
+                      cursor_location,
+                      cursor_location,
+                      ui::EF_NONE,
+                      ui::EF_NONE);
+  widget()->OnMouseEvent(&move);
+
+  EXPECT_TRUE(NULL != notification_view()->action_buttons_[0]->background());
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[1]->background());
+
+  notification()->set_buttons(CreateButtons(1));
+  notification_view()->CreateOrUpdateViews(*notification());
+
+  EXPECT_TRUE(NULL != notification_view()->action_buttons_[0]->background());
+  EXPECT_EQ(1u, notification_view()->action_buttons_.size());
+
+  // Now construct a mouse move event 1 pixel outside the boundary of the
+  // widget.
+  cursor_location = gfx::Point(-1, -1);
+  move = ui::MouseEvent(ui::ET_MOUSE_MOVED,
+                        cursor_location,
+                        cursor_location,
+                        ui::EF_NONE,
+                        ui::EF_NONE);
+  widget()->OnMouseEvent(&move);
+
+  EXPECT_TRUE(NULL == notification_view()->action_buttons_[0]->background());
+}
+
 }  // namespace message_center