Removes grab input window and extra grab and ungrab in X11WholeScreenMoveLoop

ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc

 // Copyright (c) 2012 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/widget/desktop_aura/desktop_drag_drop_client_aurax11.h"
 
 #include <X11/Xatom.h>
 
 #include "base/event_types.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/dragdrop/drop_target_event.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/base/dragdrop/os_exchange_data_provider_aurax11.h"
 #include "ui/base/x/selection_utils.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/events/event.h"
 #include "ui/events/platform/platform_event_source.h"
+#include "ui/gfx/screen.h"
+#include "ui/views/controls/image_view.h"
 #include "ui/views/widget/desktop_aura/desktop_native_cursor_manager.h"
 #include "ui/views/widget/desktop_aura/x11_topmost_window_finder.h"
+#include "ui/views/widget/widget.h"
 #include "ui/wm/public/drag_drop_client.h"
 #include "ui/wm/public/drag_drop_delegate.h"
 
 using aura::client::DragDropDelegate;
 using ui::OSExchangeData;
 
 namespace {
 
+// The minimum alpha before we declare a pixel transparent when searching in
+// our source image.
+const uint32 kMinAlpha = 32;
+const unsigned char kDragWidgetOpacity = 0xc0;
+
+// Drag widget is positioned off screen when drag image is not set or is
+// completely transparent.
+const gfx::Size kNullDragWidgetSize = gfx::Size(1, 1);
+const gfx::Point kNullDragWidgetLocation = gfx::Point(-100, -100);

[pkotwicz, 2014/05/12 16:04:29]

I do not know if there is much precedent for non primitive constants. Can you
double check?

[varkha, 2014/05/20 17:13:35]

Good catch.
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...
bans class objects with static storage duration, including global variables,
static variables, static class member variables, and function static variables;
they must be Plain Old Data.

+
+bool CheckIfIconValid(const gfx::ImageSkia& drag_image) {

[pkotwicz, 2014/05/12 16:04:29]

Is CheckIfIconValid() still needed in a world where we always construct a
|drag_widget_|?

[varkha, 2014/05/20 17:13:35]

Done.

+  // Because we need a GL context per window, we do a quick check so that we
+  // don't make another context if the window would just be displaying a mostly
+  // transparent image.
+  const SkBitmap* in_bitmap = drag_image.bitmap();
+  SkAutoLockPixels in_lock(*in_bitmap);
+  for (int y = 0; y < in_bitmap->height(); ++y) {
+    uint32* in_row = in_bitmap->getAddr32(0, y);
+
+    for (int x = 0; x < in_bitmap->width(); ++x) {
+      if (SkColorGetA(in_row[x]) > kMinAlpha)
+        return true;
+    }
+  }
+
+  return false;
+}
+
 const int kMinXdndVersion = 5;
 
 const int kWillAcceptDrop = 1;
 const int kWantFurtherPosEvents = 2;
 
 const char kXdndActionCopy[] = "XdndActionCopy";
 const char kXdndActionMove[] = "XdndActionMove";
 const char kXdndActionLink[] = "XdndActionLink";
 const char kXdndActionDirectSave[] = "XdndActionDirectSave";
 
@@ skipping 455 matching lines @@
     // We were waiting on the status message so we could send the XdndDrop.
     if (negotiated_operation_ == ui::DragDropTypes::DRAG_NONE) {
       move_loop_.EndMoveLoop();
       return;
     }
     source_state_ = SOURCE_STATE_DROPPED;
     SendXdndDrop(source_window);
     return;
   }
 
-  switch (negotiated_operation_) {
-    case ui::DragDropTypes::DRAG_COPY:
-      move_loop_.UpdateCursor(copy_grab_cursor_);
-      break;
-    case ui::DragDropTypes::DRAG_MOVE:
-      move_loop_.UpdateCursor(move_grab_cursor_);
-      break;
-    default:
-      move_loop_.UpdateCursor(grab_cursor_);
-      break;
-  }
+  UpdateCursor();
 
   // Note: event.data.[2,3] specify a rectangle. It is a request by the other
   // window to not send further XdndPosition messages while the cursor is
   // within it. However, it is considered advisory and (at least according to
   // the spec) the other side must handle further position messages within
   // it. GTK+ doesn't bother with this, so neither should we.
 
   if (next_position_message_.get()) {
     // We were waiting on the status message so we could send off the next
     // position message we queued up.
@@ skipping 95 matching lines @@
         source_provider_->file_contents_name().AsUTF8Unsafe());
   }
   ui::SetAtomArrayProperty(xwindow_, "XdndActionList", "ATOM", actions);
 
   // It is possible for the DesktopWindowTreeHostX11 to be destroyed during the
   // move loop, which would also destroy this drag-client. So keep track of
   // whether it is alive after the drag ends.
   base::WeakPtr<DesktopDragDropClientAuraX11> alive(
       weak_ptr_factory_.GetWeakPtr());
 
+  SetDragImage(source_provider_->GetDragImage(),
+               source_provider_->GetDragImageOffset());
+  CreateDragImageWindow();
   // Windows has a specific method, DoDragDrop(), which performs the entire
   // drag. We have to emulate this, so we spin off a nested runloop which will
   // track all cursor movement and reroute events to a specific handler.
-  move_loop_.SetDragImage(source_provider_->GetDragImage(),
-                          source_provider_->GetDragImageOffset());
-  move_loop_.RunMoveLoop(source_window, grab_cursor_);
+  move_loop_.RunMoveLoop();
 
   if (alive) {
-    move_loop_.SetDragImage(gfx::ImageSkia(), gfx::Vector2dF());
+    SetDragImage(gfx::ImageSkia(), gfx::Vector2dF());
 
     source_provider_ = NULL;
     g_current_drag_drop_client = NULL;
     drag_operation_ = 0;
     XDeleteProperty(xdisplay_, xwindow_, atom_cache_.GetAtom("XdndActionList"));
     XDeleteProperty(xdisplay_, xwindow_, atom_cache_.GetAtom(kXdndDirectSave0));
 
     return negotiated_operation_;
   }
   return ui::DragDropTypes::DRAG_NONE;
@@ skipping 63 matching lines @@
       // an XdndFinished message.
       StartEndMoveLoopTimer();
       return;
     }
   }
 
   move_loop_.EndMoveLoop();
 }
 
 void DesktopDragDropClientAuraX11::OnMoveLoopEnded() {
+  drag_widget_.reset();
   if (source_current_window_ != None) {
     SendXdndLeave(source_current_window_);
     source_current_window_ = None;
   }
   target_current_context_.reset();
   repeat_mouse_move_timer_.Stop();
   end_move_loop_timer_.Stop();
 }
 
 void DesktopDragDropClientAuraX11::ProcessMouseMove(
     const gfx::Point& screen_point,
     unsigned long event_time) {
   if (source_state_ != SOURCE_STATE_OTHER)
     return;
 
+  if (drag_widget_.get()) {

[pkotwicz, 2014/05/12 16:04:29]

If we intentionally moved |drag_widget_| offscreen, won't this put it back
onscreen? I suspect we want to check |drag_image_| or perhaps have a dedicated
bool member variable to keep track of whether we want to keep |drag_widget_|
offscreen

[varkha, 2014/05/20 17:13:35]

I am using drag_image_.isNull() for that (and resetting drag_image_ in case when
the |drag_widget_| is offscreen.

+    gfx::Point location = drag_image_.isNull() ?
+        kNullDragWidgetLocation :
+        gfx::ToFlooredPoint(screen_point - drag_offset_);
+    drag_widget_->SetBounds(gfx::Rect(
+        location,
+        drag_image_.isNull() ? kNullDragWidgetSize : drag_image_.size()));
+    drag_widget_->StackAtTop();
+  }
+
   // Find the current window the cursor is over.
   ::Window mouse_window = None;
   ::Window dest_window = None;
   FindWindowFor(screen_point, &mouse_window, &dest_window);
 
   if (source_current_window_ != dest_window) {
     if (source_current_window_ != None)
       SendXdndLeave(source_current_window_);
 
     source_current_window_ = dest_window;
@@ skipping 114 matching lines @@
   if (drag_operation_ & ui::DragDropTypes::DRAG_LINK)
     operations.push_back(atom_cache_.GetAtom(kXdndActionLink));
   return operations;
 }
 
 ui::SelectionFormatMap DesktopDragDropClientAuraX11::GetFormatMap() const {
   return source_provider_ ? source_provider_->GetFormatMap() :
       ui::SelectionFormatMap();
 }
 
+void DesktopDragDropClientAuraX11::UpdateCursor() {
+  gfx::NativeCursor cursor = grab_cursor_;
+  switch (negotiated_operation_) {
+    case ui::DragDropTypes::DRAG_COPY:
+      cursor = copy_grab_cursor_;
+      break;
+    case ui::DragDropTypes::DRAG_MOVE:
+      cursor = move_grab_cursor_;
+      break;
+    default:
+      break;
+  }
+
+  // Update the pointer with the new cursor if a grab is active.
+  XChangeActivePointerGrab(
+      xdisplay_,
+      ButtonPressMask | ButtonReleaseMask | PointerMotionMask,
+      cursor.platform(),
+      CurrentTime);
+}
+
 void DesktopDragDropClientAuraX11::CompleteXdndPosition(
     ::Window source_window,
     const gfx::Point& screen_point) {
   int drag_operation = ui::DragDropTypes::DRAG_NONE;
   scoped_ptr<ui::OSExchangeData> data;
   scoped_ptr<ui::DropTargetEvent> drop_target_event;
   DragDropDelegate* delegate = NULL;
   DragTranslate(screen_point, &data, &drop_target_event, &delegate);
   if (delegate)
     drag_operation = delegate->OnDragUpdated(*drop_target_event);
@@ skipping 138 matching lines @@
   // success or failure, and when it fails, it then sends an internal
   // GdkEvent about the failed drag. (And sending this message doesn't appear
   // to go through normal xlib machinery, but instead passes through the low
   // level xProto (the x11 wire format) that I don't understand.
   //
   // I'm unsure if I have to jump through those hoops, or if XSendEvent is
   // sufficient.
   XSendEvent(xdisplay_, xid, False, 0, xev);
 }
 
+void DesktopDragDropClientAuraX11::SetDragImage(const gfx::ImageSkia& image,
+                                                gfx::Vector2dF offset) {
+  /*if (drag_image_.isNull() || !CheckIfIconValid(drag_image_))*/ {

[pkotwicz, 2014/05/12 16:04:29]

Remove the if block entirely. It is not needed.

[varkha, 2014/05/20 17:13:35]

Done.

+    drag_image_ = gfx::ImageSkia();
+    drag_offset_ = gfx::Vector2dF();
+    return;
+  }
+  drag_image_ = image;
+  drag_offset_ = offset;
+  // Reset the Y offset, so that the drag-image is always just below the cursor,
+  // so that it is possible to see where the cursor is going.
+  drag_offset_.set_y(0.f);
+}
+
+void DesktopDragDropClientAuraX11::CreateDragImageWindow() {
+  Widget* widget = new Widget;
+  Widget::InitParams params(Widget::InitParams::TYPE_DRAG);
+  params.opacity = Widget::InitParams::OPAQUE_WINDOW;
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.accept_events = false;
+
+  gfx::Point location = drag_image_.isNull() ?

[pkotwicz, 2014/05/12 16:04:29]

I will defer to sadrul@/erg@ as to whether they like moving the widget
offscreen. The alternative would be to make X11ScopedCapture smarter. i.e.
X11ScopedCapture would track which XID has capture

This might be slightly nicer:
if (drag_image_.isNull()) {
  params.bounds = gfx::Rect(kNullDragWidgetLocation, kNullDragWidgetSize);
} else {
  gfx::Point cursor_screen_point =
gfx::Screen::GetNativeScreen()->GetCursorScreenPoint();
  params.bounds = gfx::Rect(
      gfx::ToFlooredPoint(cursor_screen_point - drag_offset_),
      drag_image_.size());
}

[varkha, 2014/05/20 17:13:35]

Right. Keeping the drag widget offscreen matches the behavior of the code prior
to this CL where an input window was created offscreen.

+      kNullDragWidgetLocation :
+      gfx::ToFlooredPoint(
+          gfx::Screen::GetNativeScreen()->GetCursorScreenPoint() -
+          drag_offset_);
+  params.bounds = gfx::Rect(
+      location,
+      drag_image_.isNull() ? kNullDragWidgetSize : drag_image_.size());
+  widget->set_focus_on_creation(false);
+  widget->set_frame_type(Widget::FRAME_TYPE_FORCE_NATIVE);
+  widget->Init(params);
+  widget->SetOpacity(kDragWidgetOpacity);
+  widget->GetNativeWindow()->SetName("DragWindow");
+
+  if (!drag_image_.isNull()) {
+    ImageView* image = new ImageView();
+    image->SetImage(drag_image_);
+    image->SetBounds(0, 0, drag_image_.width(), drag_image_.height());
+    widget->SetContentsView(image);
+  }
+
+  widget->Show();
+  widget->GetNativeWindow()->layer()->SetFillsBoundsOpaquely(false);

[pkotwicz, 2014/05/12 16:04:29]

We should have a comment as to why we set capture here. Setting capture is
important enough that it should probably occur outside of this function.

[varkha, 2014/05/20 17:13:35]

Done.

+  widget->GetNativeWindow()->SetCapture();
+  drag_widget_.reset(widget);
+}
+
 }  // namespace views