Fix Ash notification updates

ash/system/web_notification/web_notification_tray.cc

 // Copyright (c) 2012 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.
 
 #include "ash/system/web_notification/web_notification_tray.h"
 
 #include "ash/system/status_area_widget.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/tray_bubble_view.h"
 #include "ash/system/tray/tray_constants.h"
@@ skipping 120 matching lines @@
     }
     notification.id = id;
     notification.title = title;
     notification.message = message;
     notification.display_source = display_source;
     notification.extension_id = extension_id;
     notification.is_read = false;
     PushNotification(notification);
   }
 
-  void UpdateNotificationMessage(const std::string& id,
+  void UpdateNotificationMessage(const std::string& old_id,
+                                 const std::string& new_id,
                                  const string16& title,
                                  const string16& message) {
-    Notifications::iterator iter = GetNotification(id);
+    Notifications::iterator iter = GetNotification(old_id);
     if (iter == notifications_.end())
       return;
     // Copy and update notification, then move it to the front of the list.
     WebNotification notification(*iter);
+    notification.id = new_id;

[sadrul, 2012/08/09 18:14:45]

In AddNotification, any existing notifications with the same id is removed.
Should the same be done for |new_id| here?

[stevenjb, 2012/08/09 19:05:06]

Good catch. Moved the duplicate check to PushNotification. (In practice this
shouldn't ever happen, but it is good to be robust and consistent).

     notification.title = title;
     notification.message = message;
     notification.is_read = false;
     EraseNotification(iter);
     PushNotification(notification);
   }
 
   bool RemoveNotification(const std::string& id) {
     Notifications::iterator iter = GetNotification(id);
     if (iter == notifications_.end())
@@ skipping 273 matching lines @@
       const views::GestureEvent& event) OVERRIDE {
     if (event.type() != ui::ET_GESTURE_TAP)
       return ui::GESTURE_STATUS_UNKNOWN;
     tray_->OnClicked(notification_.id);
     return ui::GESTURE_STATUS_CONSUMED;
   }
 
   // Overridden from ButtonListener.
   virtual void ButtonPressed(views::Button* sender,
                              const views::Event& event) OVERRIDE {
-    if (sender == close_button_) {
-      tray_->RemoveNotification(notification_.id);
-      tray_->HideMessageCenterBubbleIfEmpty();
-    }
+    if (sender == close_button_)
+      tray_->SendRemoveNotification(notification_.id);
   }
 
   // Overridden from MenuButtonListener.
   virtual void OnMenuButtonClicked(
       View* source, const gfx::Point& point) OVERRIDE {
     if (source != menu_button_)
       return;
     WebNotificationMenuModel menu_model(tray_, notification_);
     views::MenuModelAdapter menu_model_adapter(&menu_model);
     views::MenuRunner menu_runner(menu_model_adapter.CreateMenu());
@@ skipping 52 matching lines @@
   virtual ~WebNotificationButtonView() {
   }
 
   void SetCloseAllVisible(bool visible) {
     close_all_button_->SetVisible(visible);
   }
 
   // Overridden from ButtonListener.
   virtual void ButtonPressed(views::Button* sender,
                              const views::Event& event) OVERRIDE {
-    if (sender == close_all_button_) {
-      tray_->RemoveAllNotifications();
-      tray_->HideMessageCenterBubbleIfEmpty();
-    }
+    if (sender == close_all_button_)
+      tray_->SendRemoveAllNotifications();
   }
 
  private:
   WebNotificationTray* tray_;
   TrayPopupTextButton* close_all_button_;
 
   DISALLOW_COPY_AND_ASSIGN(WebNotificationButtonView);
 };
 
 class WebContentsView : public views::View {
@@ skipping 53 matching lines @@
         new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 1));
     set_background(views::Background::CreateSolidBackground(kBackgroundColor));
 
     scroll_content_ = new ScrollContentView;
     scroller_ = new internal::FixedSizedScrollView;
     scroller_->SetContentsView(scroll_content_);
     AddChildView(scroller_);
 
     button_view_ = new internal::WebNotificationButtonView(tray);
     AddChildView(button_view_);
+
+    // Build initial view with no notifications.
+    Update(WebNotificationList::Notifications());
   }
 
   void Update(const WebNotificationList::Notifications& notifications) {
     scroll_content_->RemoveAllChildViews(true);
+    scroll_content_->set_preferred_size(gfx::Size());
     int num_children = 0;
     for (WebNotificationList::Notifications::const_iterator iter =
              notifications.begin(); iter != notifications.end(); ++iter) {
       WebNotificationView* view = new WebNotificationView(tray_, *iter);
       scroll_content_->AddChildView(view);
       if (++num_children >= kMaxVisibleNotifications)
         break;
     }
     if (num_children == 0) {
       views::Label* label = new views::Label(l10n_util::GetStringUTF16(
           IDS_ASH_WEB_NOTFICATION_TRAY_NO_MESSAGES));
       label->SetFont(label->font().DeriveFont(2));
       label->SetHorizontalAlignment(views::Label::ALIGN_CENTER);
       scroll_content_->AddChildView(label);
       button_view_->SetCloseAllVisible(false);
     } else {
       button_view_->SetCloseAllVisible(true);
     }
     SizeScrollContent();
     Layout();
-    GetWidget()->GetRootView()->SchedulePaint();
+    if (GetWidget())
+      GetWidget()->GetRootView()->SchedulePaint();
   }
 
  private:
   void SizeScrollContent() {
     gfx::Size scroll_size = scroll_content_->GetPreferredSize();
     const int button_height = button_view_->GetPreferredSize().height();
     const int min_height = kWebNotificationBubbleMinHeight - button_height;
     const int max_height = kWebNotificationBubbleMaxHeight - button_height;
     int scroll_height = std::min(std::max(
         scroll_size.height(), min_height), max_height);
@@ skipping 18 matching lines @@
   explicit WebNotificationContentsView(WebNotificationTray* tray)
       : WebContentsView(tray) {
     SetLayoutManager(
         new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 1));
     set_background(views::Background::CreateSolidBackground(kBackgroundColor));
 
     content_ = new views::View;
     content_->SetLayoutManager(
         new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 1));
     AddChildView(content_);
+
+    // Build initial view with no notification.
+    Update(WebNotificationList::Notifications());
   }
 
   void Update(const WebNotificationList::Notifications& notifications) {
     content_->RemoveAllChildViews(true);
-    WebNotificationList::Notifications::const_iterator iter =
-        notifications.begin();
-    WebNotificationView* view = new WebNotificationView(tray_, *iter);
+    const WebNotification& notification = (notifications.size() > 0) ?
+        notifications.front() : WebNotification();
+    WebNotificationView* view = new WebNotificationView(tray_, notification);
     content_->AddChildView(view);
+    content_->SizeToPreferredSize();
     Layout();
-    GetWidget()->GetRootView()->SchedulePaint();
+    if (GetWidget())
+      GetWidget()->GetRootView()->SchedulePaint();
   }
 
  private:
   views::View* content_;
 
   DISALLOW_COPY_AND_ASSIGN(WebNotificationContentsView);
 };
 
 }  // namespace internal
 
@@ skipping 152 matching lines @@
 }
 
 WebNotificationTray::~WebNotificationTray() {
 }
 
 void WebNotificationTray::SetDelegate(Delegate* delegate) {
   DCHECK(!delegate_);
   delegate_ = delegate;
 }
 
+// Add/Update/RemoveNotification are called by the client code, i.e the
+// Delegate implementation or its proxy.
+
 void WebNotificationTray::AddNotification(const std::string& id,
                                           const string16& title,
                                           const string16& message,
                                           const string16& display_source,
                                           const std::string& extension_id) {
   notification_list_->AddNotification(
       id, title, message, display_source, extension_id);
   UpdateTrayAndBubble();
   ShowNotificationBubble();
 }
 
-void WebNotificationTray::UpdateNotification(const std::string& id,
+void WebNotificationTray::UpdateNotification(const std::string& old_id,
+                                             const std::string& new_id,
                                              const string16& title,
                                              const string16& message) {
-  notification_list_->UpdateNotificationMessage(id, title, message);
+  notification_list_->UpdateNotificationMessage(old_id, new_id, title, message);
   UpdateTrayAndBubble();
   ShowNotificationBubble();
 }
 
 void WebNotificationTray::RemoveNotification(const std::string& id) {
   if (id == notification_list_->GetFirstId())
     HideNotificationBubble();
   if (!notification_list_->RemoveNotification(id))
     return;
-  if (delegate_)
-    delegate_->NotificationRemoved(id);
   UpdateTrayAndBubble();
 }
 
-void WebNotificationTray::RemoveAllNotifications() {
-  const WebNotificationList::Notifications& notifications =
-      notification_list_->notifications();
-  if (delegate_) {
-    for (WebNotificationList::Notifications::const_iterator loopiter =
-             notifications.begin();
-         loopiter != notifications.end(); ) {
-      WebNotificationList::Notifications::const_iterator curiter = loopiter++;
-      std::string notification_id = curiter->id;
-      // May call RemoveNotification and erase curiter.
-      delegate_->NotificationRemoved(notification_id);
-    }
-  }
-  notification_list_->RemoveAllNotifications();
-  HideMessageCenterBubble();
-  UpdateTrayAndBubble();
-}
-
 void WebNotificationTray::SetNotificationImage(const std::string& id,
                                                const gfx::ImageSkia& image) {
   if (!notification_list_->SetNotificationImage(id, image))
     return;
   UpdateTrayAndBubble();
   if (notification_bubble() && id == notification_list_->GetFirstId())
     ShowNotificationBubble();
 }
 
-void WebNotificationTray::DisableByExtension(const std::string& id) {
-  // When we disable notifications, we remove any existing matching
-  // notifications to avoid adding complicated UI to re-enable the source.
-  if (id == notification_list_->GetFirstId())
-    HideNotificationBubble();
-  notification_list_->RemoveNotificationsByExtension(id);
-  UpdateTrayAndBubble();
-  if (delegate_)
-    delegate_->DisableExtension(id);
-}
-
-void WebNotificationTray::DisableByUrl(const std::string& id) {
-  // See comment for DisableByExtension.
-  if (id == notification_list_->GetFirstId())
-    HideNotificationBubble();
-  notification_list_->RemoveNotificationsBySource(id);
-  UpdateTrayAndBubble();
-  if (delegate_)
-    delegate_->DisableNotificationsFromSource(id);
-}
-
 void WebNotificationTray::ShowMessageCenterBubble() {
   if (status_area_widget()->login_status() == user::LOGGED_IN_LOCKED)
     return;
   if (message_center_bubble()) {
     UpdateTray();
     return;
   }
   notification_list_->SetIsVisible(true);  // clears notification count
   UpdateTray();
   HideNotificationBubble();
   message_center_bubble_.reset(
       new Bubble(this, Bubble::BUBBLE_TYPE_MESAGE_CENTER));
   status_area_widget()->SetHideSystemNotifications(true);
 }
 
 void WebNotificationTray::HideMessageCenterBubble() {
   if (!message_center_bubble())
     return;
   message_center_bubble_.reset();
   show_message_center_on_unlock_ = false;
   notification_list_->SetIsVisible(false);
   status_area_widget()->SetHideSystemNotifications(false);
 }
 
-void WebNotificationTray::HideMessageCenterBubbleIfEmpty() {
-  if (GetNotificationCount() == 0)
-    HideMessageCenterBubble();
-}
-
 void WebNotificationTray::ShowNotificationBubble() {
   if (status_area_widget()->login_status() == user::LOGGED_IN_LOCKED)
     return;
   if (message_center_bubble())
     return;
   if (!status_area_widget()->ShouldShowNonSystemNotifications())
     return;
   UpdateTray();
   if (notification_bubble()) {
     notification_bubble()->ScheduleUpdate();
@@ skipping 16 matching lines @@
     }
     HideNotificationBubble();
   } else {
     if (show_message_center_on_unlock_)
       ShowMessageCenterBubble();
     show_message_center_on_unlock_ = false;
   }
   UpdateTray();
 }
 
-void WebNotificationTray::ShowSettings(const std::string& id) {
-  if (delegate_)
-    delegate_->ShowSettings(id);
-}
-
-void WebNotificationTray::OnClicked(const std::string& id) {
-  if (delegate_)
-    delegate_->OnClicked(id);
-}
-
 void WebNotificationTray::SetShelfAlignment(ShelfAlignment alignment) {
   if (alignment == shelf_alignment())
     return;
   internal::TrayBackgroundView::SetShelfAlignment(alignment);
   if (alignment == SHELF_ALIGNMENT_BOTTOM)
     tray_container()->set_size(gfx::Size(kTrayWidth, kTrayHeight));
   else
     tray_container()->set_size(gfx::Size(kTraySideWidth, kTraySideHeight));
   // Destroy any existing bubble so that it will be rebuilt correctly.
   HideMessageCenterBubble();
   HideNotificationBubble();
 }
 
+// Protected methods (invoked only from Bubble and its child classes)
+
+void WebNotificationTray::SendRemoveNotification(const std::string& id) {
+  // If this is the only notification in the list, close the bubble.
+  if (notification_list_->notifications().size() == 1 &&
+      id == notification_list_->GetFirstId()) {
+    HideMessageCenterBubble();
+  }
+  if (delegate_)
+    delegate_->NotificationRemoved(id);
+}
+
+void WebNotificationTray::SendRemoveAllNotifications() {
+  HideMessageCenterBubble();
+  if (delegate_) {
+    const WebNotificationList::Notifications& notifications =
+        notification_list_->notifications();
+    for (WebNotificationList::Notifications::const_iterator loopiter =
+             notifications.begin();
+         loopiter != notifications.end(); ) {
+      WebNotificationList::Notifications::const_iterator curiter = loopiter++;
+      std::string notification_id = curiter->id;
+      // May call RemoveNotification and erase curiter.
+      delegate_->NotificationRemoved(notification_id);
+    }
+  }
+}
+
+// When we disable notifications, we remove any existing matching
+// notifications to avoid adding complicated UI to re-enable the source.
+void WebNotificationTray::DisableByExtension(const std::string& id) {
+  // Will call SendRemoveNotification for each matching notification.
+  notification_list_->RemoveNotificationsByExtension(id);
+  if (delegate_)
+    delegate_->DisableExtension(id);
+}
+
+void WebNotificationTray::DisableByUrl(const std::string& id) {
+  // Will call SendRemoveNotification for each matching notification.
+  notification_list_->RemoveNotificationsBySource(id);
+  if (delegate_)
+    delegate_->DisableNotificationsFromSource(id);
+}
+
 bool WebNotificationTray::PerformAction(const views::Event& event) {
   if (message_center_bubble())
     HideMessageCenterBubble();
   else
     ShowMessageCenterBubble();
   return true;
 }
 
 int WebNotificationTray::GetNotificationCount() const {
   return notification_list()->notifications().size();
 }
 
+void WebNotificationTray::ShowSettings(const std::string& id) {
+  if (delegate_)
+    delegate_->ShowSettings(id);
+}
+
+void WebNotificationTray::OnClicked(const std::string& id) {
+  if (delegate_)
+    delegate_->OnClicked(id);
+}
+
+// Private methods
+
 void WebNotificationTray::UpdateTray() {
   count_label_->SetText(UTF8ToUTF16(
       GetNotificationText(notification_list()->unread_count())));
   // Dim the message count text only if the message center is empty.
   count_label_->SetEnabledColor(
       (notification_list()->notifications().size() == 0) ?
       kMessageCountDimmedColor : kMessageCountColor);
   SetVisible((status_area_widget()->login_status() != user::LOGGED_IN_NONE));
   Layout();
   SchedulePaint();
@@ skipping 15 matching lines @@
 
 void WebNotificationTray::HideBubble(Bubble* bubble) {
   if (bubble == message_center_bubble()) {
     HideMessageCenterBubble();
   } else if (bubble == notification_bubble()) {
     HideNotificationBubble();
   }
 }
 
 }  // namespace ash