Skip to content

Commit

Permalink
Fix dragging items out of folders in the app list for Windows.
Browse files Browse the repository at this point in the history
This CL fixes dragging of items out of a folder view in the Windows app
list. This issue was being caused by the SynchronousDrag code not being
correctly dealt with, causing an extra instance to be created and
skipping the view switching code.

This CL allows SynchronousDrag to interact with folder update code
and prevents a new SynchronousDrag from being created
in the root level AppsGridView during a reparent which causes it to
correctly move around an invisible drag view.

This CL also changes the reparenting drag events to all use a Point rather
than a LocatedEvent.

Fixes a bug seen on Windows when cancelling a drag after the reparent
begins would cause a crash.

BUG=348941
TEST=On Windows:
1. In one continuous drag, move an item out of a folder by dragging to
a corner of the app list, then move it onto the main grid so that an empty
space is created, then press ESC. There should not be a crash.
2. Drag an item out of the app list folder and place it in the top
level of the app list.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260493 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
calamity@chromium.org committed Mar 31, 2014
1 parent e7a9444 commit f2724cf
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 33 deletions.
7 changes: 5 additions & 2 deletions ui/app_list/views/app_list_folder_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,11 @@ void AppListFolderView::ReparentItem(

void AppListFolderView::DispatchDragEventForReparent(
AppsGridView::Pointer pointer,
const ui::LocatedEvent& event) {
container_view_->apps_grid_view()->UpdateDragFromReparentItem(pointer, event);
const gfx::Point& drag_point_in_folder_grid) {
AppsGridView* root_grid = container_view_->apps_grid_view();
gfx::Point drag_point_in_root_grid = drag_point_in_folder_grid;
ConvertPointToTarget(items_grid_view_, root_grid, &drag_point_in_root_grid);
root_grid->UpdateDragFromReparentItem(pointer, drag_point_in_folder_grid);
}

void AppListFolderView::DispatchEndDragEventForReparent(
Expand Down
6 changes: 3 additions & 3 deletions ui/app_list/views/app_list_folder_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ class AppListFolderView : public views::View,
virtual void ReparentItem(AppListItemView* original_drag_view,
const gfx::Point& drag_point_in_folder_grid)
OVERRIDE;
virtual void DispatchDragEventForReparent(AppsGridView::Pointer pointer,
const ui::LocatedEvent& event)
OVERRIDE;
virtual void DispatchDragEventForReparent(
AppsGridView::Pointer pointer,
const gfx::Point& drag_point_in_folder_grid) OVERRIDE;
virtual void DispatchEndDragEventForReparent(
bool events_forwarded_to_drag_drop_host) OVERRIDE;
virtual bool IsPointOutsideOfFolderBoundary(const gfx::Point& point) OVERRIDE;
Expand Down
41 changes: 21 additions & 20 deletions ui/app_list/views/apps_grid_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,10 @@ void AppsGridView::StartSettingUpSynchronousDrag() {
if (drag_and_drop_host_)
return;

// Never create a second synchronous drag if the drag started in a folder.
if (IsDraggingForReparentInRootLevelGridView())
return;

delegate_->GetShortcutPathForApp(
drag_view_->item()->id(),
base::Bind(&AppsGridView::OnGotShortcutPath, base::Unretained(this)));
Expand Down Expand Up @@ -493,9 +497,6 @@ bool AppsGridView::UpdateDragFromItem(Pointer pointer,
const ui::LocatedEvent& event) {
DCHECK(drag_view_);

if (folder_delegate_)
UpdateDragStateInsideFolder(pointer, event);

gfx::Point drag_point_in_grid_view;
ExtractDragLocation(event, &drag_point_in_grid_view);
UpdateDrag(pointer, drag_point_in_grid_view);
Expand All @@ -513,6 +514,9 @@ bool AppsGridView::UpdateDragFromItem(Pointer pointer,
}

void AppsGridView::UpdateDrag(Pointer pointer, const gfx::Point& point) {
if (folder_delegate_)
UpdateDragStateInsideFolder(pointer, point);

// EndDrag was called before if |drag_view_| is NULL.
if (!drag_view_)
return;
Expand Down Expand Up @@ -591,13 +595,15 @@ void AppsGridView::EndDrag(bool cancel) {
drag_and_drop_host_->EndDrag(cancel);
if (IsDraggingForReparentInHiddenGridView())
folder_delegate_->DispatchEndDragEventForReparent(true);
} else if (!cancel && dragging()) {
} else {
if (IsDraggingForReparentInHiddenGridView()) {
// Forward the EndDrag event to the root level grid view.
folder_delegate_->DispatchEndDragEventForReparent(false);
folder_delegate_->DispatchEndDragEventForReparent(cancel);
EndDragForReparentInHiddenFolderGridView();
return;
} else {
}

if (!cancel && dragging()) {
// Regular drag ending path, ie, not for reparenting.
CalculateDropTarget(last_drag_point_, true);
if (IsValidIndex(drop_target_)) {
Expand Down Expand Up @@ -728,15 +734,12 @@ void AppsGridView::InitiateDragFromReparentItemInRootLevelGridView(
dragging_for_reparent_item_ = true;
}

void AppsGridView::UpdateDragFromReparentItem(
Pointer pointer,
const ui::LocatedEvent& event) {
void AppsGridView::UpdateDragFromReparentItem(Pointer pointer,
const gfx::Point& drag_point) {
DCHECK(drag_view_);
DCHECK(IsDraggingForReparentInRootLevelGridView());

gfx::Point drag_point_in_grid_view;
ExtractDragLocation(event, &drag_point_in_grid_view);
UpdateDrag(pointer, drag_point_in_grid_view);
UpdateDrag(pointer, drag_point);
}

bool AppsGridView::IsDraggedView(const views::View* view) const {
Expand Down Expand Up @@ -1290,16 +1293,15 @@ void AppsGridView::OnFolderDroppingTimer() {
SetAsFolderDroppingTarget(drop_target_, true);
}

void AppsGridView::UpdateDragStateInsideFolder(
Pointer pointer,
const ui::LocatedEvent& event) {
void AppsGridView::UpdateDragStateInsideFolder(Pointer pointer,
const gfx::Point& drag_point) {
if (IsUnderOEMFolder())
return;

if (IsDraggingForReparentInHiddenGridView()) {
// Dispatch drag event to root level grid view for re-parenting folder
// folder item purpose.
DispatchDragEventForReparent(pointer, event);
DispatchDragEventForReparent(pointer, drag_point);
return;
}

Expand Down Expand Up @@ -1353,10 +1355,9 @@ bool AppsGridView::IsUnderOEMFolder() {
return folder_delegate_->IsOEMFolder();
}

void AppsGridView::DispatchDragEventForReparent(
Pointer pointer,
const ui::LocatedEvent& event) {
folder_delegate_->DispatchDragEventForReparent(pointer, event);
void AppsGridView::DispatchDragEventForReparent(Pointer pointer,
const gfx::Point& drag_point) {
folder_delegate_->DispatchDragEventForReparent(pointer, drag_point);
}

void AppsGridView::EndDragFromReparentItemInRootLevel(
Expand Down
6 changes: 3 additions & 3 deletions ui/app_list/views/apps_grid_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
// Updates drag in the root level grid view when receiving the drag event
// dispatched from the hidden grid view for reparenting a folder item.
void UpdateDragFromReparentItem(Pointer pointer,
const ui::LocatedEvent& event);
const gfx::Point& drag_point);

// Dispatches the drag event from hidden grid view to the top level grid view.
void DispatchDragEventForReparent(Pointer pointer,
const ui::LocatedEvent& event);
const gfx::Point& drag_point);

// Handles EndDrag event dispatched from the hidden folder grid view in the
// root level grid view to end reparenting a folder item.
Expand Down Expand Up @@ -422,7 +422,7 @@ class APP_LIST_EXPORT AppsGridView : public views::View,

// Updates drag state for dragging inside a folder's grid view.
void UpdateDragStateInsideFolder(Pointer pointer,
const ui::LocatedEvent& event);
const gfx::Point& drag_point);

// Returns true if drag event is happening in the root level AppsGridView
// for reparenting a folder item.
Expand Down
5 changes: 3 additions & 2 deletions ui/app_list/views/apps_grid_view_folder_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ class APP_LIST_EXPORT AppsGridViewFolderDelegate {

// Dispatches drag event from the hidden grid view to the root level grid view
// for re-parenting a folder item.
virtual void DispatchDragEventForReparent(AppsGridView::Pointer pointer,
const ui::LocatedEvent& event) = 0;
virtual void DispatchDragEventForReparent(
AppsGridView::Pointer pointer,
const gfx::Point& drag_point_in_folder_grid) = 0;

// Dispatches EndDrag event from the hidden grid view to the root level grid
// view for reparenting a folder item.
Expand Down
6 changes: 3 additions & 3 deletions ui/app_list/views/apps_grid_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ class TestAppsGridViewFolderDelegate : public AppsGridViewFolderDelegate {
const gfx::Point& drag_point_in_folder_grid)
OVERRIDE {}

virtual void DispatchDragEventForReparent(AppsGridView::Pointer pointer,
const ui::LocatedEvent& event)
OVERRIDE {}
virtual void DispatchDragEventForReparent(
AppsGridView::Pointer pointer,
const gfx::Point& drag_point_in_folder_grid) OVERRIDE {}

virtual void DispatchEndDragEventForReparent(
bool events_forwarded_to_drag_drop_host) OVERRIDE {}
Expand Down

0 comments on commit f2724cf

Please sign in to comment.