Fix use-after-free in MessageListView.

ui/message_center/views/message_list_view.cc

 // Copyright (c) 2015 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 <algorithm>
+
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/message_center/message_center_style.h"
 #include "ui/message_center/message_center_switches.h"
 #include "ui/message_center/views/message_center_view.h"
 #include "ui/message_center/views/message_list_view.h"
 #include "ui/message_center/views/message_view.h"
@@ skipping 78 matching lines @@
   if (GetContentsBounds().IsEmpty())
     return;
 
   adding_views_.insert(view);
   DoUpdateIfPossible();
 }
 
 void MessageListView::RemoveNotification(MessageView* view) {
   DCHECK_EQ(view->parent(), this);
 
+  // TODO(yhananda): We should consider consolidating clearing_all_views_,
+  // deleting_views_ and deleted_when_done_.
+  if (std::find(clearing_all_views_.begin(), clearing_all_views_.end(), view) !=
+          clearing_all_views_.end() ||
+      deleting_views_.find(view) != deleting_views_.end() ||
+      deleted_when_done_.find(view) != deleted_when_done_.end()) {
+    // Let's skip deleting the view if it's already scheduled for deleting.
+    // Even if we check clearing_all_views_ here, we actualy have no idea
+    // whether the view is due to be removed or not because it could be in its
+    // animation before removal.
+    // In short, we could delete the view twice even if we check these three
+    // lists.
+    return;
+  }
 
   if (GetContentsBounds().IsEmpty()) {
     delete view;
   } else {
     if (adding_views_.find(view) != adding_views_.end())
       adding_views_.erase(view);
     if (animator_.IsAnimating(view))
       animator_.StopAnimatingView(view);
 
     if (view->layer()) {
       deleting_views_.insert(view);
     } else {
       delete view;
     }
     DoUpdateIfPossible();
   }
 }
 
 void MessageListView::UpdateNotification(MessageView* view,
                                          const Notification& notification) {
+  // Skip updating the notification being cleared
+  if (std::find(clearing_all_views_.begin(), clearing_all_views_.end(), view) !=
+      clearing_all_views_.end())
+    return;
+
   int index = GetIndexOf(view);
   DCHECK_LE(0, index);  // GetIndexOf is negative if not a child.
 
   animator_.StopAnimatingView(view);
   if (deleting_views_.find(view) != deleting_views_.end())
     deleting_views_.erase(view);
   if (deleted_when_done_.find(view) != deleted_when_done_.end())
     deleted_when_done_.erase(view);
   view->UpdateWithNotification(notification);
   DoUpdateIfPossible();
@@ skipping 102 matching lines @@
     const gfx::Rect& visible_scroll_rect) {
   for (int i = 0; i < child_count(); ++i) {
     // Safe cast since all views in MessageListView are MessageViews.
     MessageView* child = (MessageView*)child_at(i);
     if (!child->visible())
       continue;
     if (gfx::IntersectRects(child->bounds(), visible_scroll_rect).IsEmpty())
       continue;
     if (child->IsPinned())
       continue;
+    if (deleting_views_.find(child) != deleting_views_.end() ||
+        deleted_when_done_.find(child) != deleted_when_done_.end()) {
+      // We don't check clearing_all_views_ here, so this can lead to a
+      // notification being deleted twice. Even if we do check it, there is a
+      // problem similar to the problem in RemoveNotification(), it could be
+      // currently in its animation before removal, and we could similarly
+      // delete it twice. This is a bug.
+      continue;
+    }
     clearing_all_views_.push_back(child);
   }
   if (clearing_all_views_.empty()) {
     for (auto& observer : observers_)
       observer.OnAllNotificationsCleared();
   } else {
     DoUpdateIfPossible();
   }
 }
 
@@ skipping 36 matching lines @@
 
   if (quit_message_loop_after_animation_for_test_)
     base::MessageLoop::current()->QuitWhenIdle();
 }
 
 bool MessageListView::IsValidChild(const views::View* child) const {
   return child->visible() &&
          deleting_views_.find(const_cast<views::View*>(child)) ==
              deleting_views_.end() &&
          deleted_when_done_.find(const_cast<views::View*>(child)) ==
-             deleted_when_done_.end();
+             deleted_when_done_.end() &&
+         std::find(clearing_all_views_.begin(), clearing_all_views_.end(),
+                   child) == clearing_all_views_.end();
 }
 
 void MessageListView::DoUpdateIfPossible() {
   gfx::Rect child_area = GetContentsBounds();
   if (child_area.IsEmpty())
     return;
 
   if (animator_.IsAnimating()) {
     has_deferred_task_ = true;
     return;
@@ skipping 222 matching lines @@
         base::TimeDelta::FromMilliseconds(
             kAnimateClearingNextNotificationDelayMS));
   }
 }
 
 void MessageListView::SetRepositionTargetForTest(const gfx::Rect& target_rect) {
   SetRepositionTarget(target_rect);
 }
 
 }  // namespace message_center