Fix cleaning up of cancelled reparents in the Windows App List Folder UI.

This CL fixes the drag cancellation behavior for the app list on Windows.
Previously, the drag view in the root level grid view would not be cleaned
up causing an invalid AppListItemView to persist in the root level view.
When interacted with, this would crash.

BUG=365600

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

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

6 files changed, 52 insertions, 43 deletions

diff --git a/ui/app_list/views/app_list_folder_view.cc b/ui/app_list/views/app_list_folder_view.cc
index 86833f3..b9e93b0 100644
--- a/ui/app_list/views/app_list_folder_view.cc
+++ b/ui/app_list/views/app_list_folder_view.cc
@@ -276,9 +276,10 @@ void AppListFolderView::DispatchDragEventForReparent(
 }
 
 void AppListFolderView::DispatchEndDragEventForReparent(
-    bool events_forwarded_to_drag_drop_host) {
-  container_view_->apps_grid_view()->
-      EndDragFromReparentItemInRootLevel(events_forwarded_to_drag_drop_host);
+    bool events_forwarded_to_drag_drop_host,
+    bool cancel_drag) {
+  container_view_->apps_grid_view()->EndDragFromReparentItemInRootLevel(
+      events_forwarded_to_drag_drop_host, cancel_drag);
 }
 
 void AppListFolderView::HideViewImmediately() {
diff --git a/ui/app_list/views/app_list_folder_view.h b/ui/app_list/views/app_list_folder_view.h
index bdbf62d..6172ec4 100644
--- a/ui/app_list/views/app_list_folder_view.h
+++ b/ui/app_list/views/app_list_folder_view.h
@@ -103,7 +103,8 @@ class AppListFolderView : public views::View,
       AppsGridView::Pointer pointer,
       const gfx::Point& drag_point_in_folder_grid) OVERRIDE;
   virtual void DispatchEndDragEventForReparent(
-      bool events_forwarded_to_drag_drop_host) OVERRIDE;
+      bool events_forwarded_to_drag_drop_host,
+      bool cancel_drag) OVERRIDE;
   virtual bool IsPointOutsideOfFolderBoundary(const gfx::Point& point) OVERRIDE;
   virtual bool IsOEMFolder() const OVERRIDE;
   virtual void SetRootLevelDragViewVisible(bool visible) OVERRIDE;
diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index f63b4fb..2533413 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -370,7 +370,7 @@ void AppsGridView::SetLayout(int icon_size, int cols, int rows_per_page) {
 
 void AppsGridView::ResetForShowApps() {
   activated_item_view_ = NULL;
-  ClearDragState(false);
+  ClearDragState();
   layer()->SetOpacity(1.0f);
   SetVisible(true);
   // Set all views to visible in case they weren't made visible again by an
@@ -620,12 +620,17 @@ void AppsGridView::EndDrag(bool cancel) {
     DCHECK(!IsDraggingForReparentInRootLevelGridView());
     forward_events_to_drag_and_drop_host_ = false;
     drag_and_drop_host_->EndDrag(cancel);
-    if (IsDraggingForReparentInHiddenGridView())
-      folder_delegate_->DispatchEndDragEventForReparent(true);
+    if (IsDraggingForReparentInHiddenGridView()) {
+      folder_delegate_->DispatchEndDragEventForReparent(
+          true /* events_forwarded_to_drag_drop_host */,
+          cancel /* cancel_drag */);
+    }
   } else {
     if (IsDraggingForReparentInHiddenGridView()) {
       // Forward the EndDrag event to the root level grid view.
-      folder_delegate_->DispatchEndDragEventForReparent(cancel);
+      folder_delegate_->DispatchEndDragEventForReparent(
+          false /* events_forwarded_to_drag_drop_host */,
+          cancel /* cancel_drag */);
       EndDragForReparentInHiddenFolderGridView();
       return;
     }
@@ -668,7 +673,7 @@ void AppsGridView::EndDrag(bool cancel) {
   CleanUpSynchronousDrag();
 
   SetAsFolderDroppingTarget(drop_target_, false);
-  ClearDragState(false);
+  ClearDragState();
   AnimateToIdealBounds();
 
   StopPageFlipTimer();
@@ -763,16 +768,21 @@ bool AppsGridView::IsDraggedView(const views::View* view) const {
   return drag_view_ == view;
 }
 
-void AppsGridView::ClearDragState(bool cancel_reparent) {
+void AppsGridView::ClearDragState() {
   drop_attempt_ = DROP_FOR_NONE;
   drag_pointer_ = NONE;
   drop_target_ = Index();
-  if (!cancel_reparent && drag_view_)
-    drag_view_->OnDragEnded();
-  drag_view_ = NULL;
   drag_start_grid_view_ = gfx::Point();
   drag_start_page_ = -1;
   drag_view_offset_ = gfx::Point();
+
+  if (drag_view_) {
+    drag_view_->OnDragEnded();
+    if (IsDraggingForReparentInRootLevelGridView()) {
+      DeleteItemViewAtIndex(view_model_.GetIndexOfView(drag_view_));
+    }
+  }
+  drag_view_ = NULL;
   dragging_for_reparent_item_ = false;
 }
 
@@ -1398,37 +1408,24 @@ void AppsGridView::DispatchDragEventForReparent(Pointer pointer,
 }
 
 void AppsGridView::EndDragFromReparentItemInRootLevel(
-    bool events_forwarded_to_drag_drop_host) {
+    bool events_forwarded_to_drag_drop_host,
+    bool cancel_drag) {
   // EndDrag was called before if |drag_view_| is NULL.
   if (!drag_view_)
     return;
 
   DCHECK(IsDraggingForReparentInRootLevelGridView());
-  bool cancel_reparent = false;
-  scoped_ptr<AppListItemView> cached_drag_view;
-  if (!events_forwarded_to_drag_drop_host) {
+  bool cancel_reparent = cancel_drag || drop_attempt_ == DROP_FOR_NONE;
+  if (!events_forwarded_to_drag_drop_host && !cancel_reparent) {
     CalculateDropTarget(last_drag_point_, true);
     if (IsValidIndex(drop_target_)) {
       if (drop_attempt_ == DROP_FOR_REORDER) {
         ReparentItemForReorder(drag_view_, drop_target_);
       } else if (drop_attempt_ == DROP_FOR_FOLDER) {
         ReparentItemToAnotherFolder(drag_view_, drop_target_);
-      } else {  // DROP_FOR_NONE_
-        cancel_reparent = true;
-        // Note(jennyz): cached_drag_view makes sure drag_view_ will be deleted
-        // after AnimateToIdealBounds() is called.
-        // There is a problem in layer() animation which cause DCHECK failure
-        // if a child view is deleted immediately before re-creating layer in
-        // layer animation. The layer tree seems marked dirty, and complaining
-        // when we try to re-create layer in AnimationBetweenRows when calling
-        // AnimateToIdealBounds.
-        cached_drag_view.reset(drag_view_);
       }
     }
-
-    if (!cancel_reparent) {
-      SetViewHidden(drag_view_, false /* show */, true /* no animate */);
-    }
+    SetViewHidden(drag_view_, false /* show */, true /* no animate */);
   }
 
   // The drag can be ended after the synchronous drag is created but before it
@@ -1436,9 +1433,16 @@ void AppsGridView::EndDragFromReparentItemInRootLevel(
   CleanUpSynchronousDrag();
 
   SetAsFolderDroppingTarget(drop_target_, false);
-  ClearDragState(cancel_reparent);
-  if (cancel_reparent)
-    CancelFolderItemReparent(cached_drag_view.get());
+  if (cancel_reparent) {
+    CancelFolderItemReparent(drag_view_);
+  } else {
+    // By setting |drag_view_| to NULL here, we prevent ClearDragState() from
+    // cleaning up the newly created AppListItemView, effectively claiming
+    // ownership of the newly created drag view.
+    drag_view_->OnDragEnded();
+    drag_view_ = NULL;
+  }
+  ClearDragState();
   AnimateToIdealBounds();
 
   StopPageFlipTimer();
@@ -1456,7 +1460,7 @@ void AppsGridView::EndDragForReparentInHiddenFolderGridView() {
   CleanUpSynchronousDrag();
 
   SetAsFolderDroppingTarget(drop_target_, false);
-  ClearDragState(false);
+  ClearDragState();
 }
 
 void AppsGridView::OnFolderItemRemoved() {
@@ -1763,10 +1767,6 @@ void AppsGridView::CancelFolderItemReparent(AppListItemView* drag_item_view) {
   // the animation completes.
   CalculateIdealBounds();
 
-  // Remove drag_view_ from view_model_, it will be deleted after the animation.
-  int drag_view_index = view_model_.GetIndexOfView(drag_item_view);
-  view_model_.Remove(drag_view_index);
-
   gfx::Rect target_icon_rect =
       GetTargetIconRectInFolder(drag_item_view, activated_item_view_);
 
@@ -1798,6 +1798,8 @@ void AppsGridView::DeleteItemViewAtIndex(int index) {
   view_model_.Remove(index);
   if (item_view == activated_item_view_)
     activated_item_view_ = NULL;
+  if (item_view == drag_view_)
+    drag_view_ = NULL;
   delete item_view;
 }
 
diff --git a/ui/app_list/views/apps_grid_view.h b/ui/app_list/views/apps_grid_view.h
index ba6ef2c..3738a22 100644
--- a/ui/app_list/views/apps_grid_view.h
+++ b/ui/app_list/views/apps_grid_view.h
@@ -115,7 +115,7 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   void UpdateDrag(Pointer pointer, const gfx::Point& point);
   void EndDrag(bool cancel);
   bool IsDraggedView(const views::View* view) const;
-  void ClearDragState(bool cancel_reparent);
+  void ClearDragState();
   void SetDragViewVisible(bool visible);
 
   // Set the drag and drop host for application links.
@@ -175,8 +175,10 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   // root level grid view to end reparenting a folder item.
   // |events_forwarded_to_drag_drop_host|: True if the dragged item is dropped
   // to the drag_drop_host, eg. dropped on shelf.
+  // |cancel_drag|: True if the drag is ending because it has been canceled.
   void EndDragFromReparentItemInRootLevel(
-      bool events_forwarded_to_drag_drop_host);
+      bool events_forwarded_to_drag_drop_host,
+      bool cancel_drag);
 
   // Handles EndDrag event in the hidden folder grid view to end reparenting
   // a folder item.
diff --git a/ui/app_list/views/apps_grid_view_folder_delegate.h b/ui/app_list/views/apps_grid_view_folder_delegate.h
index 7dfaa4c..4be530c 100644
--- a/ui/app_list/views/apps_grid_view_folder_delegate.h
+++ b/ui/app_list/views/apps_grid_view_folder_delegate.h
@@ -44,8 +44,10 @@ class APP_LIST_EXPORT AppsGridViewFolderDelegate {
   // view for reparenting a folder item.
   // |events_forwarded_to_drag_drop_host|: True if the dragged item is dropped
   // to the drag_drop_host, eg. dropped on shelf.
+  // |cancel_drag|: True if the drag is ending because it has been canceled.
   virtual void DispatchEndDragEventForReparent(
-      bool events_forwarded_to_drag_drop_host) = 0;
+      bool events_forwarded_to_drag_drop_host,
+      bool cancel_drag) = 0;
 
   // Returns true if |point| falls outside of the folder container ink bubble.
   virtual bool IsPointOutsideOfFolderBoundary(const gfx::Point& point) = 0;
diff --git a/ui/app_list/views/apps_grid_view_unittest.cc b/ui/app_list/views/apps_grid_view_unittest.cc
index 25a9916..d93943d 100644
--- a/ui/app_list/views/apps_grid_view_unittest.cc
+++ b/ui/app_list/views/apps_grid_view_unittest.cc
@@ -206,7 +206,8 @@ class TestAppsGridViewFolderDelegate : public AppsGridViewFolderDelegate {
       const gfx::Point& drag_point_in_folder_grid) OVERRIDE {}
 
   virtual void DispatchEndDragEventForReparent(
-      bool events_forwarded_to_drag_drop_host) OVERRIDE {}
+      bool events_forwarded_to_drag_drop_host,
+      bool cancel_drag) OVERRIDE {}
 
   virtual bool IsPointOutsideOfFolderBoundary(const gfx::Point& point)
       OVERRIDE {