Drag and drop between app list and launcher - First patch

ash/launcher/launcher_view.cc

Index: ash/launcher/launcher_view.cc
diff --git a/ash/launcher/launcher_view.cc b/ash/launcher/launcher_view.cc
index c4a9d56d347e5409771e4b00f12bf495c2cb6e72..5c7ada437ec78d9a91b16ee266d6e43e555d34c4 100644
--- a/ash/launcher/launcher_view.cc
+++ b/ash/launcher/launcher_view.cc
@@ -22,6 +22,7 @@
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell_delegate.h"
 #include "base/auto_reset.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
@@ -377,7 +378,9 @@ LauncherView::LauncherView(LauncherModel* model,
       cancelling_drag_model_changed_(false),
       last_hidden_index_(0),
       closing_event_time_(base::TimeDelta()),
-      got_deleted_(NULL) {
+      got_deleted_(NULL),
+      dnd_item_created_(false),
+      dnd_launcher_id_(0) {
   DCHECK(model_);
   bounds_animator_.reset(new views::BoundsAnimator(this));
   bounds_animator_->AddObserver(this);
@@ -523,6 +526,77 @@ View* LauncherView::GetFocusTraversableParentView() {
   return this;
 }
 
+bool LauncherView::StartDrag(const std::string& app_id,
+                             const gfx::Point& location_in_screen_coordinates) {
+  // Bail if an operation is already going on - or the cursor is not inside.
+  if (dnd_launcher_id_ ||
+      !GetBoundsInScreen().Contains(location_in_screen_coordinates))
+    return false;
+
+  // If the AppsGridView (which was dispatching this event) was opened by our
+  // button, LauncherView dragging operations are locked and we have to unlock.
+  CancelDrag(-1);
+  dnd_item_created_ = false;
+  dnd_app_id_ = app_id;
+  dnd_launcher_id_ = delegate_->GetLauncherIDForAppID(dnd_app_id_);
+
+  if (!dnd_launcher_id_) {
+    delegate_->PinAppWithID(app_id);
+    dnd_launcher_id_ = delegate_->GetLauncherIDForAppID(dnd_app_id_);
+    if (!dnd_launcher_id_)
+      return false;
+    dnd_item_created_ = true;
+  }
+  views::View* dnd_view = view_model_->view_at(
+      model_->ItemIndexByID(dnd_launcher_id_));
+  DCHECK(dnd_view);
+
+  // Since there is already an icon presented, we hide this one for now.
+  dnd_view_->SetVisible(false);
+  // First we have to center the mouse cursor over the item.

[James Cook, 2013/04/29 22:08:29]

I don't understand this comment.  Isn't the code synthesizing a mouse press at
the center of the icon, not moving the cursor (or the icon)?

[Mr4D (OOO till 08-26), 2013/04/30 16:59:01]

Done.

+  gfx::Point pt = dnd_view->GetBoundsInScreen().CenterPoint();
+  views::View::ConvertPointFromScreen(dnd_view, &pt);
+  ui::MouseEvent event(ui::ET_MOUSE_PRESSED,
+                       pt, location_in_screen_coordinates, 0);
+  PointerPressedOnButton(dnd_view, LauncherButtonHost::DND, event);
+
+  // Drag the item where it really belongs.
+  Drag(location_in_screen_coordinates);
+  return true;
+}
+
+bool LauncherView::Drag(const gfx::Point& location_in_screen_coordinates) {
+  if (!dnd_launcher_id_ ||
+      !GetBoundsInScreen().Contains(location_in_screen_coordinates))
+    return false;
+
+  gfx::Point pt = location_in_screen_coordinates;
+  views::View* dnd_view = view_model_->view_at(

[James Cook, 2013/04/29 22:08:29]

Do you need to check if this is NULL?  What if you're in the middle of a drag
and sync decides to delete all your apps?  Could this be a problem elsewhere
too?

[Mr4D (OOO till 08-26), 2013/04/30 16:59:01]

Good one! I updated this!

+      model_->ItemIndexByID(dnd_launcher_id_));
+  views::View::ConvertPointFromScreen(dnd_view, &pt);
+
+  ui::MouseEvent event(ui::ET_MOUSE_DRAGGED, pt, gfx::Point(), 0);
+  PointerDraggedOnButton(dnd_view, LauncherButtonHost::DND, event);
+  return true;
+}
+
+void LauncherView::EndDrag(bool cancel) {
+  if (!dnd_launcher_id_)
+    return;
+
+  views::View* dnd_view = view_model_->view_at(
+      model_->ItemIndexByID(dnd_launcher_id_));
+  PointerReleasedOnButton(dnd_view, LauncherButtonHost::DND, cancel);
+
+  if (dnd_item_created_ && cancel)
+    delegate_->UnpinAppsWithID(dnd_app_id_);
+
+  if (dnd_view)
+    dnd_view->SetVisible(true);
+
+  dnd_launcher_id_ = 0;
+}
+
 void LauncherView::LayoutToIdealBounds() {
   IdealBounds ideal_bounds;
   CalculateIdealBounds(&ideal_bounds);
@@ -1340,31 +1414,37 @@ void LauncherView::ButtonPressed(views::Button* sender,
             ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
     }
 
-  // Collect usage statistics before we decide what to do with the click.
-  switch (model_->items()[view_index].type) {
-    case TYPE_APP_SHORTCUT:
-    case TYPE_WINDOWED_APP:
-    case TYPE_PLATFORM_APP:
-      Shell::GetInstance()->delegate()->RecordUserMetricsAction(
-          UMA_LAUNCHER_CLICK_ON_APP);
-      // Fallthrough
-    case TYPE_TABBED:
-    case TYPE_APP_PANEL:
-      delegate_->ItemClicked(model_->items()[view_index], event);
-      break;
+    // Collect usage statistics before we decide what to do with the click.
+    switch (model_->items()[view_index].type) {
+      case TYPE_APP_SHORTCUT:
+      case TYPE_WINDOWED_APP:
+      case TYPE_PLATFORM_APP:
+        Shell::GetInstance()->delegate()->RecordUserMetricsAction(
+            UMA_LAUNCHER_CLICK_ON_APP);
+        // Fallthrough
+      case TYPE_TABBED:
+      case TYPE_APP_PANEL:
+        delegate_->ItemClicked(model_->items()[view_index], event);
+        break;
 
-    case TYPE_APP_LIST:
-      Shell::GetInstance()->delegate()->RecordUserMetricsAction(
-          UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
-      Shell::GetInstance()->ToggleAppList(GetWidget()->GetNativeView());
-      break;
+      case TYPE_APP_LIST:
+        Shell::GetInstance()->delegate()->RecordUserMetricsAction(
+            UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
+        Shell::GetInstance()->ToggleAppList(GetWidget()->GetNativeView());
+        // By setting us as DnD recipient, the app list knows that we can
+        // handle items.
+        // TODO(skuhne): Invert the flag
+        if (!CommandLine::ForCurrentProcess()->HasSwitch(

[James Cook, 2013/04/29 22:08:29]

Did you want to invert this test before committing?

[Mr4D (OOO till 08-26), 2013/04/30 16:59:01]

Done. (yes in deed - for some reason flags do not work anymore on linux)

+                ash::switches::kAshDnDAppListToLauncher))
+          Shell::GetInstance()->SetAppListDnDHost(this);

[James Cook, 2013/04/29 22:08:29]

Can this be done once in the constructor or in Init() rather than every time the
app list is clicked?

If it has to be done every time the app list is shown, is there a way to make it
more clear that the host is being associated with a particular app list view
instance rather than the Shell in general?

A comment on multi-monitor/multi-launcher handling might be good too.

[Mr4D (OOO till 08-26), 2013/04/30 16:59:01]

Since the application list is created upon every click, the state is not
persistent. As such the handler needs to be set up every time. I renamed the
function to make it clear that this call will only have an impact on the current
application list. Done.

+        break;
 
-    case TYPE_BROWSER_SHORTCUT:
-      // Click on browser icon is counted in app clicks.
-      Shell::GetInstance()->delegate()->RecordUserMetricsAction(
-          UMA_LAUNCHER_CLICK_ON_APP);
-      delegate_->OnBrowserShortcutClicked(event.flags());
-      break;
+      case TYPE_BROWSER_SHORTCUT:
+        // Click on browser icon is counted in app clicks.
+        Shell::GetInstance()->delegate()->RecordUserMetricsAction(
+            UMA_LAUNCHER_CLICK_ON_APP);
+        delegate_->OnBrowserShortcutClicked(event.flags());
+        break;
     }
   }