Correctly set dragLeave and dragEnd coords for OOPIF drag and drop

content/browser/web_contents/web_drag_source_mac.mm

 // 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.
 
 #import "content/browser/web_contents/web_drag_source_mac.h"
 
 #include <sys/param.h>
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/mac/foundation_util.h"
 #include "base/pickle.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/download/drag_download_file.h"
 #include "content/browser/download/drag_download_util.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
+#include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/drop_data.h"
 #include "net/base/escape.h"
 #include "net/base/filename_util.h"
 #include "net/base/mime_util.h"
 #include "ui/base/clipboard/custom_data_helper.h"
 #include "ui/base/cocoa/cocoa_base_utils.h"
 #include "ui/base/dragdrop/cocoa_dnd_util.h"
@@ skipping 248 matching lines @@
   // Flip |screenPoint|.
   NSRect screenFrame = [[[contentsView_ window] screen] frame];
   screenPoint.y = screenFrame.size.height - screenPoint.y;
 
   // If AppKit returns a copy and move operation, mask off the move bit
   // because WebCore does not understand what it means to do both, which
   // results in an assertion failure/renderer crash.
   if (operation == (NSDragOperationMove | NSDragOperationCopy))
     operation &= ~NSDragOperationMove;
 
-  // TODO(paulmeyer):  In the OOPIF case, should |localPoint| be converted to
-  // the coordinates local to |dragStartRWH_|?
+  // |localPoint| and |screenPoint| are in the root coordinate space, for
+  // non-root RenderWidgetHosts they need to be transformed.
+  gfx::Point transformedPoint = gfx::Point(localPoint.x, localPoint.y);
+  gfx::Point transformedScreenPoint = gfx::Point(screenPoint.x, screenPoint.y);
+  static_cast<content::RenderWidgetHostViewBase*>(
+      contents_->GetRenderWidgetHostView())
+      ->TransformPointToCoordSpaceForView(
+          gfx::Point(localPoint.x, localPoint.y),
+          static_cast<content::RenderWidgetHostViewBase*>(
+              dragStartRWH_->GetView()),
+          &transformedPoint);
+  static_cast<content::RenderWidgetHostViewBase*>(
+      contents_->GetRenderWidgetHostView())
+      ->TransformPointToCoordSpaceForView(
+          gfx::Point(screenPoint.x, screenPoint.y),
+          static_cast<content::RenderWidgetHostViewBase*>(
+              dragStartRWH_->GetView()),
+          &transformedScreenPoint);

[dcheng, 2017/02/01 20:45:31]

Is it possible to have more of these utility functions shared in general? For
example, lines 279 - 287 seem pretty similar to flipWindowPointToView /
flipWindowPointToScreen in web_drag_dest_mac.mm, and then we have similar
transformations in the rest of the drag/drop code as well.

In addition, I find this block of code a bit difficult to read... there's a lot
of downcasting involved.

[kenrb, 2017/02/02 21:21:03]

I have made changes to make the point transformation calls more readable.

As for the the coordinate flipping, is there something specific you had in mind?
There isn't a convenient Mac utility file under content/browser/web_contents
that I can see, and it is a stretch to create one for such a small change. I am
reluctant to undertake a larger refactor in part because I am a novice with
Objective-C++, and in part because I don't want to block this bug fix on
something like that.

   contents_->DragSourceEndedAt(
-      localPoint.x, localPoint.y, screenPoint.x, screenPoint.y,
-      static_cast<blink::WebDragOperation>(operation),
-      dragStartRWH_.get());
+      transformedPoint.x(), transformedPoint.y(), transformedScreenPoint.x(),
+      transformedScreenPoint.y(),
+      static_cast<blink::WebDragOperation>(operation), dragStartRWH_.get());
 
   // Make sure the pasteboard owner isn't us.
   [pasteboard_ declareTypes:[NSArray array] owner:nil];
 }
 
 - (NSString*)dragPromisedFileTo:(NSString*)path {
   // Be extra paranoid; avoid crashing.
   if (!dropData_) {
     NOTREACHED() << "No drag-and-drop data available for promised file.";
     return nil;
@@ skipping 158 matching lines @@
 - (NSImage*)dragImage {
   if (dragImage_)
     return dragImage_;
 
   // Default to returning a generic image.
   return content::GetContentClient()->GetNativeImageNamed(
       IDR_DEFAULT_FAVICON).ToNSImage();
 }
 
 @end  // @implementation WebDragSource (Private)