[Notifications] Fix crash in AddNotification.

Calling ResetRepositionSession during UpdateNotificationSize can end up
deleting notification items. This caused two crashes. One in
AddNotification, which calls onItemUpdated, which, without this change,
can set item_ to null. The other is a crash in
ShouldUpdateControlButtonsColor which does not check if item_ is null
since it is called from a context where item_ should not be null.

BUG=714493
BUG=714032

Review-Url: https://codereview.chromium.org/2833403002
Cr-Commit-Position: refs/heads/master@{#466914}

ui/message_center/views/message_center_view.cc

@@ -407,6 +407,8 @@ bool MessageCenterView::SetRepositionTarget() {
 }
 
 void MessageCenterView::OnNotificationUpdated(const std::string& id) {
+  // TODO(edcourtney): We may be able to remove this, since |UpdateNotification|
+  // checks it anyway.
   NotificationViewsMap::const_iterator view_iter = notification_views_.find(id);
   if (view_iter == notification_views_.end())
     return;
@@ -416,30 +418,7 @@ void MessageCenterView::OnNotificationUpdated(const std::string& id) {
   if (!SetRepositionTarget())
     message_list_view_->ResetRepositionSession();
 
-  // TODO(dimich): add MessageCenter::GetVisibleNotificationById(id)
-  MessageView* view = view_iter->second;
-  const NotificationList::Notifications& notifications =
-      message_center_->GetVisibleNotifications();
-  for (NotificationList::Notifications::const_iterator iter =
-           notifications.begin(); iter != notifications.end(); ++iter) {
-    if ((*iter)->id() == id) {
-      int old_width = view->width();
-      int old_height = view->height();
-      bool old_pinned = view->IsPinned();
-      message_list_view_->UpdateNotification(view, **iter);
-      if (view->GetHeightForWidth(old_width) != old_height) {
-        Update(true /* animate */);
-      } else if (view->IsPinned() != old_pinned) {
-        // Animate flag is false, since the pinned flag transition doesn't need
-        // animation.
-        Update(false /* animate */);
-      }
-      break;
-    }
-  }
-
-  // Notify accessibility that the contents have changed.
-  view->NotifyAccessibilityEvent(ui::AX_EVENT_CHILDREN_CHANGED, false);
+  UpdateNotification(id);
 }
 
 void MessageCenterView::OnLockedStateChanged(bool locked) {
@@ -481,7 +460,14 @@ void MessageCenterView::ClickOnSettingsButton(
 
 void MessageCenterView::UpdateNotificationSize(
     const std::string& notification_id) {
-  OnNotificationUpdated(notification_id);
+  // TODO(edcourtney, yoshiki): We don't call OnNotificationUpdated directly
+  // because it resets the reposition session, which can end up deleting
+  // notification items when it cancels animations. This causes problems for
+  // ARC notifications. See crbug.com/714493. OnNotificationUpdated should not
+  // have to consider the reposition session, but OnMouseEntered and
+  // OnMouseExited don't work properly for ARC notifications at the moment.
+  // See crbug.com/714587.
+  UpdateNotification(notification_id);
 }
 
 void MessageCenterView::AnimationEnded(const gfx::Animation* animation) {
@@ -673,4 +659,38 @@ void MessageCenterView::SetNotificationViewForTest(MessageView* view) {
   message_list_view_->AddNotificationAt(view, 0);
 }
 
+void MessageCenterView::UpdateNotification(const std::string& id) {
+  // TODO(edcourtney, yoshiki): This check seems like it should not be needed.
+  // Investigate what circumstances (if any) trigger it.
+  NotificationViewsMap::const_iterator view_iter = notification_views_.find(id);
+  if (view_iter == notification_views_.end())
+    return;
+
+  // TODO(dimich): add MessageCenter::GetVisibleNotificationById(id)
+  MessageView* view = view_iter->second;
+  const NotificationList::Notifications& notifications =
+      message_center_->GetVisibleNotifications();
+  for (NotificationList::Notifications::const_iterator iter =
+           notifications.begin();
+       iter != notifications.end(); ++iter) {
+    if ((*iter)->id() == id) {
+      int old_width = view->width();
+      int old_height = view->height();
+      bool old_pinned = view->IsPinned();
+      message_list_view_->UpdateNotification(view, **iter);
+      if (view->GetHeightForWidth(old_width) != old_height) {
+        Update(true /* animate */);
+      } else if (view->IsPinned() != old_pinned) {
+        // Animate flag is false, since the pinned flag transition doesn't need
+        // animation.
+        Update(false /* animate */);
+      }
+      break;
+    }
+  }
+
+  // Notify accessibility that the contents have changed.
+  view->NotifyAccessibilityEvent(ui::AX_EVENT_CHILDREN_CHANGED, false);
+}
+
 }  // namespace message_center

ui/message_center/views/message_center_view.h

@@ -122,6 +122,7 @@ class MESSAGE_CENTER_EXPORT MessageCenterView
   void UpdateButtonBarStatus();
   void EnableCloseAllIfAppropriate();
   void SetNotificationViewForTest(MessageView* view);
+  void UpdateNotification(const std::string& notification_id);
 
   MessageCenter* message_center_;  // Weak reference.
   MessageCenterTray* tray_;  // Weak reference.