mash: Use the new mus drag and drop API to get drag working in mash.

ui/views/mus/drag_drop_client_mus.cc

Index: ui/views/mus/drag_drop_client_mus.cc
diff --git a/ui/views/mus/drag_drop_client_mus.cc b/ui/views/mus/drag_drop_client_mus.cc
new file mode 100644
index 0000000000000000000000000000000000000000..83bf55251be018f71b5cb0d20d0883ad195c0767
--- /dev/null
+++ b/ui/views/mus/drag_drop_client_mus.cc
@@ -0,0 +1,111 @@
+// Copyright 2016 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 "ui/views/mus/drag_drop_client_mus.h"
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "services/ui/public/cpp/window.h"
+#include "ui/aura/window.h"
+#include "ui/views/mus/os_exchange_data_provider_mus.h"
+
+namespace views {
+namespace {
+
+DragDropClientMus* g_current_dragging_client = nullptr;

[sky, 2016/09/23 16:14:08]

My understanding from the style guide is that this is not global (it's internal
to this file), and so should be named "current_dragging_client"

+
+void OnMoveLoopEnd(bool* out_success,
+                   uint32_t* out_action,
+                   base::Closure quit_closure,
+                   bool in_success,
+                   uint32_t in_action) {
+  *out_success = in_success;
+  *out_action = in_action;
+  quit_closure.Run();
+}
+
+}  // namespace
+
+// TODO(erg): Do we need to observe?
+
+DragDropClientMus::DragDropClientMus(
+    aura::Window* aura_window,
+    ui::Window* ui_window)
+    : aura_window_(aura_window),
+      ui_window_(ui_window) {}
+
+DragDropClientMus::~DragDropClientMus() {}
+
+int DragDropClientMus::StartDragAndDrop(
+    const ui::OSExchangeData& data,
+    aura::Window* root_window,
+    aura::Window* source_window,
+    const gfx::Point& screen_location,
+    int drag_operations,
+    ui::DragDropTypes::DragEventSource source) {
+  aura_window_->ReleaseCapture();

[sky, 2016/09/23 16:14:08]

Please comment why this is necessary.

[Elliot Glaysher, 2016/09/23 21:40:48]

Deleted; it wasn't necessary anymore now that there's an implicit release
capture on the server side.

+
+  base::MessageLoopForUI* loop = base::MessageLoopForUI::current();

[sadrul, 2016/09/23 16:22:36]

Can this be just MessageLoop? Because mus apps do not have (more like should not
have) TYPE_UI message loops (because there can be side-effects, e.g. the glib
message loop triggers sandbox traps).

[Elliot Glaysher, 2016/09/23 21:40:48]

Done.

+  base::MessageLoop::ScopedNestableTaskAllower allow_nested(loop);
+  base::RunLoop run_loop;
+
+  std::map<std::string, std::vector<uint8_t>> drag_data =
+      static_cast<const OSExchangeDataProviderMus&>(data.provider()).GetData();
+
+  // TODO(erg): Right now, I'm passing the cursor_location, but maybe I want to
+  // pass OSExchangeData::GetDragImageOffset() instead?
+
+  // TODO(erg): We don't have access to the pointer event at this level, and
+  // we'll have to do work to thread the actual events down to this
+  // layer. There are two callers to View::DoDrag(), which is the input to this
+  // code path:
+  //   - view.cc:View::ProcessMouseDragged(), which can translate to Pointer.
+  //   - root_view.cc:PosteEventDispatchHandler::OnGestureEvent() which can't.
+  //
+  // Getting the Gesture events translated to PointerEvents so that we can
+  // thread them to here is going to be...fun.
+  int32_t drag_pointer = ui::PointerEvent::kMousePointerId;

[Elliot Glaysher, 2016/09/22 20:23:20]

This is the one final issue with this patch. At this point, we've been called by
either of the two methods in the above comment, and we need a pointer id.
However, gesture events can have multiple pointer ids, if I'm reading a comment
from sadrul correctly.

The pointer used on the mus server side in drag_controller.cc to only respond to
events which have the same pointer id, to ensure if I'm dragging with the mouse,
pointer down and up with a finger shouldn't then release the drag.

[sadrul, 2016/09/23 16:22:36]

Does it matter what pointer id you use on the client side for dragging? Can it
be some special pointer id, e.g. kDragPointer? That way, you can continue to
process events from other non-dragging pointers (although that may be a bit
weird to do that?), and the WS will take care of making sure the pointer-id
involved in dragging only takes part in dragging until the release.

Having said that, ui::GestureEvent has a unique_touch_event_id() field that you
*could* use if you really really wanted to
(https://cs.chromium.org/chromium/src/ui/events/event.h?sq=package:chromium&dr...),
but best avoid it, because a gesture event can have more than one contributing
touch point (e.g. two-finger scroll, pinch-zoom etc.)

[sky, 2016/09/23 17:56:27]

On the mus side we know we are waiting on a response from the client to a
particular event. So, we should be able to tie the pointer id together there,
right?

[Elliot Glaysher, 2016/09/23 21:40:48]

At this point, mus will no longer have the event which caused this, because the
event dispatch machinery in NativeWidgetMus creates a EventAckHandler, which is
a MessageLoop::NestingObserver which sends handled event acks to mus on a nested
message loop being created, which happens at the top of this method. Therefore,
by the time we even start the drag drop on the mus side,
WindowManagerState::event_awaiting_input_ack_ is null.

(Also, implicit spooky action at a distance sucks.)

I also tried to look at the |pointer_targets_| in the EventDispatcher, but since
there appears to be an explicit capture taken by client during mousedown (not
related to dragging; just normal event dispatch stuff), pointer targets gets
cleared.

Should I add another tracking mechanism in event dispatcher that keeps track of
which pointers are down regardless of capture state?

[sky, 2016/09/26 02:12:59]

Can you move crating the RunLoop until after calling PerformDragDrop? It means
you can't supply QuitClosure to the callback, but you should be able to roll
your own.

[Elliot Glaysher, 2016/09/26 20:57:29]

Done, with a few additional changes. (For example, we weren't always keeping
track of the event that we were waiting on in the server side.)

+
+  bool success = false;
+  gfx::Point cursor_location = screen_location;
+  uint32_t action_taken = ui::mojom::kDropEffectNone;
+  g_current_dragging_client = this;
+  ui_window_->PerformDragDrop(
+      drag_pointer, drag_data, drag_operations, cursor_location,
+      *data.provider().GetDragImage().bitmap(),
+      base::Bind(OnMoveLoopEnd, &success, &action_taken,
+                 run_loop.QuitClosure()));
+
+  run_loop.Run();
+  g_current_dragging_client = nullptr;
+
+  return action_taken;
+}
+
+void DragDropClientMus::DragUpdate(aura::Window* target,
+                                   const ui::LocatedEvent& event) {
+  // Part of a fat interface for mac.

[sky, 2016/09/23 16:14:08]

I'm not clear on what this comment means. Maybe you mean 'only called on the
mac'?

+  NOTREACHED();
+}
+
+void DragDropClientMus::Drop(aura::Window* target,
+                             const ui::LocatedEvent& event) {
+  // Part of a fat interface for mac.
+  NOTREACHED();
+}
+
+void DragDropClientMus::DragCancel() {
+  ui_window_->CancelDragDrop();
+}
+
+bool DragDropClientMus::IsDragDropInProgress() {
+  return !!g_current_dragging_client;
+}
+
+}  // namespace views
+