Retain popup bubble mouse status even through updates.

Retain popup bubble mouse status even through updates.

Currently rapid updates completely block the user from
interacting with the view even for completely static
content such as the close button.

BUG=368025
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269458

[dewittj, 2014-05-07 23:02:18]

PTAL, I hope to get this in for m36 if possible..

[dewittj, 2014-05-07 23:04:06]

+smckay for cc.

[Jun Mukai, 2014-05-07 23:15:11]

lgtm

It's great if some test can verify this, but the tests doesn't need to be in
M36.

https://codereview.chromium.org/271773002/diff/1/ui/message_center/views/noti...
File ui/message_center/views/notification_view.h (right):

https://codereview.chromium.org/271773002/diff/1/ui/message_center/views/noti...
ui/message_center/views/notification_view.h:74: void CreateViews(const
Notification& notification);
nit: this doesn't actually create views right?  It creates if missing, or update
the contents. What about CreateOrUpdateViews()? (it doesn't sound concise
though...)

[dewittj, 2014-05-07 23:49:00]

The CQ bit was checked by dewittj@chromium.org

[commit-bot: I haz the power, 2014-05-07 23:49:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dewittj@chromium.org/271773002/1

[dewittj, 2014-05-07 23:50:45]

The CQ bit was unchecked by dewittj@chromium.org

[dewittj, 2014-05-08 17:52:19]

The CQ bit was checked by dewittj@chromium.org

[commit-bot: I haz the power, 2014-05-08 18:00:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dewittj@chromium.org/271773002/40001

[commit-bot: I haz the power, 2014-05-09 09:24:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-09 09:24:59]

Failed to apply patch for ui/message_center/views/notification_view.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/message_center/views/notification_view.cc
  Hunk #3 FAILED at 339.
  Hunk #4 succeeded at 589 (offset 8 lines).
  Hunk #5 FAILED at 620.
  2 out of 5 hunks FAILED -- saving rejects to file
ui/message_center/views/notification_view.cc.rej

Patch:       ui/message_center/views/notification_view.cc
Index: ui/message_center/views/notification_view.cc
diff --git a/ui/message_center/views/notification_view.cc
b/ui/message_center/views/notification_view.cc
index
671b42045bb9d8202761c8b9ca07a2bdf43bc32d..ecde1d13ab98e7d3fe9a43e8d9294d6a6ba82f59
100644
--- a/ui/message_center/views/notification_view.cc
+++ b/ui/message_center/views/notification_view.cc
@@ -291,6 +291,29 @@ NotificationView*
NotificationView::Create(MessageCenterController* controller,
   return notification_view;
 }
 
+void NotificationView::CreateOrUpdateViews(const Notification& notification) {
+  CreateOrUpdateTitleView(notification);
+  CreateOrUpdateMessageView(notification);
+  CreateOrUpdateProgressBarView(notification);
+  CreateOrUpdateListItemViews(notification);
+  CreateOrUpdateIconView(notification);
+  CreateOrUpdateImageView(notification);
+  CreateOrUpdateActionButtonViews(notification);
+}
+
+void NotificationView::SetAccessibleName(const Notification& notification) {
+  std::vector<base::string16> accessible_lines;
+  accessible_lines.push_back(notification.title());
+  accessible_lines.push_back(notification.message());
+  accessible_lines.push_back(notification.context_message());
+  std::vector<NotificationItem> items = notification.items();
+  for (size_t i = 0; i < items.size() && i < kNotificationMaximumItems; ++i) {
+    accessible_lines.push_back(items[i].title + base::ASCIIToUTF16(" ") +
+                               items[i].message);
+  }
+  set_accessible_name(JoinString(accessible_lines, '\n'));
+}
+
 NotificationView::NotificationView(MessageCenterController* controller,
                                    const Notification& notification)
     : MessageView(this,
@@ -299,8 +322,15 @@ NotificationView::NotificationView(MessageCenterController*
controller,
                   notification.small_image().AsImageSkia(),
                   notification.display_source()),
       controller_(controller),
-      clickable_(notification.clickable()) {
-  std::vector<base::string16> accessible_lines;
+      clickable_(notification.clickable()),
+      top_view_(NULL),
+      title_view_(NULL),
+      message_view_(NULL),
+      context_message_view_(NULL),
+      icon_view_(NULL),
+      bottom_view_(NULL),
+      image_view_(NULL),
+      progress_bar_view_(NULL) {
   // 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.
@@ -309,152 +339,32 @@
NotificationView::NotificationView(MessageCenterController* controller,
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
   top_view_->SetBorder(
       MakeEmptyBorder(kTextTopPadding - 8, 0, kTextBottomPadding - 5, 0));
-
-  const gfx::FontList default_label_font_list = views::Label().font_list();
-
-  // Create the title view if appropriate.
-  title_view_ = NULL;
-  if (!notification.title().empty()) {
-    const gfx::FontList& font_list =
-        default_label_font_list.DeriveWithSizeDelta(2);
-    int padding = kTitleLineHeight - font_list.GetHeight();
-    int title_lines = notification.message().empty() ? kTitleNoMessageLineLimit
-                                                     : kTitleLineLimit;
-    int title_character_limit =
-        kNotificationWidth * title_lines / kMinPixelsPerTitleCharacter;
-    title_view_ = new BoundedLabel(
-        gfx::TruncateString(notification.title(), title_character_limit),
-        font_list);
-    title_view_->SetLineHeight(kTitleLineHeight);
-    title_view_->SetLineLimit(title_lines);
-    title_view_->SetColors(message_center::kRegularTextColor,
-                           kRegularTextBackgroundColor);
-    title_view_->SetBorder(MakeTextBorder(padding, 3, 0));
-    top_view_->AddChildView(title_view_);
-    accessible_lines.push_back(notification.title());
-  }
-
-  // Create the message view if appropriate.
-  message_view_ = NULL;
-  if (!notification.message().empty()) {
-    int padding = kMessageLineHeight - default_label_font_list.GetHeight();
-    message_view_ = new BoundedLabel(
-        gfx::TruncateString(notification.message(), kMessageCharacterLimit));
-    message_view_->SetLineHeight(kMessageLineHeight);
-    message_view_->SetVisible(!notification.items().size());
-    message_view_->SetColors(message_center::kRegularTextColor,
-                             kDimTextBackgroundColor);
-    message_view_->SetBorder(MakeTextBorder(padding, 4, 0));
-    top_view_->AddChildView(message_view_);
-    accessible_lines.push_back(notification.message());
-  }
-
-  // Create the context message view if appropriate.
-  context_message_view_ = NULL;
-  if (!notification.context_message().empty()) {
-    int padding = kMessageLineHeight - default_label_font_list.GetHeight();
-    context_message_view_ =
-        new BoundedLabel(gfx::TruncateString(notification.context_message(),
-                                            kContextMessageCharacterLimit),
-                         default_label_font_list);
-    context_message_view_->SetLineLimit(
-        message_center::kContextMessageLineLimit);
-    context_message_view_->SetLineHeight(kMessageLineHeight);
-    context_message_view_->SetColors(message_center::kDimTextColor,
-                                     kContextTextBackgroundColor);
-    context_message_view_->SetBorder(MakeTextBorder(padding, 4, 0));
-    top_view_->AddChildView(context_message_view_);
-    accessible_lines.push_back(notification.context_message());
-  }
-
-  // Create the progress bar view.
-  progress_bar_view_ = NULL;
-  if (notification.type() == NOTIFICATION_TYPE_PROGRESS) {
-    progress_bar_view_ = new NotificationProgressBar();
-    progress_bar_view_->SetBorder(MakeProgressBarBorder(
-        message_center::kProgressBarTopPadding, kProgressBarBottomPadding));
-    progress_bar_view_->SetValue(notification.progress() / 100.0);
-    top_view_->AddChildView(progress_bar_view_);
-  }
-
-  // Create the list item views (up to a maximum).
-  int padding = kMessageLineHeight - default_label_font_list.GetHeight();
-  std::vector<NotificationItem> items = notification.items();
-  for (size_t i = 0; i < items.size() && i < kNotificationMaximumItems; ++i) {
-    ItemView* item_view = new ItemView(items[i]);
-    item_view->SetBorder(MakeTextBorder(padding, i ? 0 : 4, 0));
-    item_views_.push_back(item_view);
-    top_view_->AddChildView(item_view);
-    accessible_lines.push_back(
-        items[i].title + base::ASCIIToUTF16(" ") + items[i].message);
-  }
-
-  // Create the notification icon view.
-  gfx::ImageSkia icon = notification.icon().AsImageSkia();
-  if (notification.type() == NOTIFICATION_TYPE_SIMPLE &&
-      (icon.width() != kIconSize ||
-       icon.height() != kIconSize ||
-       HasAlpha(icon, GetWidget()))) {
-    views::ImageView* icon_view = new views::ImageView();
-    icon_view->SetImage(icon);
-    icon_view->SetImageSize(gfx::Size(kLegacyIconSize, kLegacyIconSize));
-    icon_view->SetHorizontalAlignment(views::ImageView::CENTER);
-    icon_view->SetVerticalAlignment(views::ImageView::CENTER);
-    icon_view_ = icon_view;
-  } else {
-    icon_view_ =
-        new ProportionalImageView(icon, gfx::Size(kIconSize, kIconSize));
-  }
-
-  icon_view_->set_background(
-      views::Background::CreateSolidBackground(kIconBackgroundColor));
+  AddChildView(top_view_);
 
   // Create the bottom_view_, which collects into a vertical box all content
   // below the notification icon.
   bottom_view_ = new views::View();
   bottom_view_->SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
+  AddChildView(bottom_view_);
 
-  // Create the image view if appropriate.
-  image_view_ = NULL;
-  if (!notification.image().IsEmpty()) {
-    gfx::Size image_size(
-        kNotificationPreferredImageWidth, kNotificationPreferredImageHeight);
-    image_view_ = MakeNotificationImage(notification.image(), image_size);
-    bottom_view_->AddChildView(image_view_);
-  }
-
-  // Create action buttons if appropriate.
-  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));
-    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);
-  }
+  CreateOrUpdateViews(notification);
 
   // Put together the different content and control views. Layering those
allows
   // for proper layout logic and it also allows the close button and small
   // image to overlap the content as needed to provide large enough click and
   // touch areas (<http://crbug.com/168822> and <http://crbug.com/168856>).
-  AddChildView(top_view_);
-  AddChildView(icon_view_);
-  AddChildView(bottom_view_);
   AddChildView(small_image());
   AddChildView(close_button());
-  set_accessible_name(JoinString(accessible_lines, '\n'));
+  SetAccessibleName(notification);
 }
 
 NotificationView::~NotificationView() {
 }
 
 gfx::Size NotificationView::GetPreferredSize() {
-  int top_width = top_view_->GetPreferredSize().width();
+  int top_width = top_view_->GetPreferredSize().width() +
+                  icon_view_->GetPreferredSize().width();
   int bottom_width …
(message too large)

[dewittj, 2014-05-09 20:47:30]

The CQ bit was checked by dewittj@chromium.org

[commit-bot: I haz the power, 2014-05-09 20:49:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dewittj@chromium.org/271773002/60001

[commit-bot: I haz the power, 2014-05-09 23:15:23]

Change committed as 269458