Convert drag event position to root frame.

Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
Committed: https://crrev.com/3d4fa85236725f161e2870a69faaca75c4e2a67d
Cr-Commit-Position: refs/heads/master@{#423250}

[hush (inactive), 2016-10-04 22:54:32]

Patchset #1 (id:1) has been deleted

[hush (inactive), 2016-10-04 22:54:48]

hush@chromium.org changed reviewers:
+ dcheng@chromium.org

[hush (inactive), 2016-10-04 22:54:49]

PTAL

[hush (inactive), 2016-10-04 22:55:15]

Description was changed from

==========
Convert drag event position to root frame.

The drag event position are firstly converted into DIP pixel on the Java side.
Then they are applied with the same conversion function used by gesture events.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626
==========

to

==========
Convert drag event position to root frame.

The drag event position are firstly converted into DIP pixel on the Java side.
Then they are applied with the same conversion function used by gesture events.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

[dcheng, 2016-10-04 23:08:07]

dcheng@chromium.org changed reviewers:
+ bokan@chromium.org

[dcheng, 2016-10-04 23:08:07]

I'm not a good reviewer for this. My only thought is that the coordinate
conversion helper function feels out place in WebInputEventConversion.

+bokan would probably be a better reviewer

[hush (inactive), 2016-10-04 23:14:38]

On 2016/10/04 23:08:07, dcheng wrote:
> I'm not a good reviewer for this. My only thought is that the coordinate
> conversion helper function feels out place in WebInputEventConversion.
> 
> +bokan would probably be a better reviewer

I didn't think of a better place to put it. So yeah, I just stick it there..

[bokan, 2016-10-04 23:47:23]

https://codereview.chromium.org/2396693002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/2396693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:3712: WebPoint
clientPointInRootFrame(convertDragEventPointToRootFrame(mainFrameImpl()->frameView(),
IntPoint(clientPoint.x, clientPoint.y)));
no need for the helper function, just call
page()->frameHost().visualViewport().viewportToRootFrame(clientPoint). This
misses out on DevTools emulation adjustments but that shouldn't be a problem on
Android.

Also, please rename clientPoint to pointInViewport.

[hush (inactive), 2016-10-05 00:18:12]

Patchset #2 (id:40001) has been deleted

[hush (inactive), 2016-10-05 00:23:49]

Patchset #2 (id:60001) has been deleted

[hush (inactive), 2016-10-05 00:24:33]

https://codereview.chromium.org/2396693002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/2396693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:3712: WebPoint
clientPointInRootFrame(convertDragEventPointToRootFrame(mainFrameImpl()->frameView(),
IntPoint(clientPoint.x, clientPoint.y)));
On 2016/10/04 23:47:23, bokan wrote:
> no need for the helper function, just call
> page()->frameHost().visualViewport().viewportToRootFrame(clientPoint). This
> misses out on DevTools emulation adjustments but that shouldn't be a problem
on
> Android.
> 
> Also, please rename clientPoint to pointInViewport.

Done.

[bokan, 2016-10-05 00:26:00]

lgtm, though I'm only an owner for Source/core

[bokan, 2016-10-05 00:26:16]

On 2016/10/05 00:26:00, bokan wrote:
> lgtm, though I'm only an owner for Source/core

erm, Source/web

[hush (inactive), 2016-10-05 00:27:07]

Description was changed from

==========
Convert drag event position to root frame.

The drag event position are firstly converted into DIP pixel on the Java side.
Then they are applied with the same conversion function used by gesture events.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

to

==========
Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

[dcheng, 2016-10-05 00:28:04]

rs lgtm based on bokan's review

[hush (inactive), 2016-10-05 00:28:45]

hush@chromium.org changed reviewers:
+ tedchoc@chromium.org

[hush (inactive), 2016-10-05 00:28:46]

Hi dcheng@
can you take a look at third_party/Webkit/public?

tedchoc@:
PTAL ContentViewCore.java

[hush (inactive), 2016-10-05 00:38:09]

The CQ bit was checked by hush@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 00:38:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 02:11:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 02:11:04]

Dry run: This issue passed the CQ dry run.

[Ted C, 2016-10-05 16:14:06]

lgtm

[hush (inactive), 2016-10-05 16:42:27]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2016-10-05 16:42:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 16:50:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 16:50:24]

Failed to apply patch for third_party/WebKit/Source/web/WebViewImpl.cpp:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/web/WebViewImpl.cpp:3641
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/web/WebViewImpl.cpp' with
conflicts.
  U third_party/WebKit/Source/web/WebViewImpl.cpp

Patch:       third_party/WebKit/Source/web/WebViewImpl.cpp
Index: third_party/WebKit/Source/web/WebViewImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp
b/third_party/WebKit/Source/web/WebViewImpl.cpp
index
f3691ab0d9df068f3dad7bc7368733ce2a5d264c..a8ca3dd0b85c19a3627550175c8a2d2cf7c8849f
100644
--- a/third_party/WebKit/Source/web/WebViewImpl.cpp
+++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -3641,11 +3641,12 @@ HitTestResult WebViewImpl::coreHitTestResultAt(const
WebPoint& pointInViewport)
 }
 
 void WebViewImpl::dragSourceEndedAt(
-    const WebPoint& clientPoint,
+    const WebPoint& pointInViewport,
     const WebPoint& screenPoint,
     WebDragOperation operation)
 {
-    PlatformMouseEvent pme(clientPoint, screenPoint,
WebPointerProperties::Button::Left, PlatformEvent::MouseMoved,
+    WebPoint
pointInRootFrame(page()->frameHost().visualViewport().viewportToRootFrame(pointInViewport));
+    PlatformMouseEvent pme(pointInRootFrame, screenPoint,
WebPointerProperties::Button::Left, PlatformEvent::MouseMoved,
         0, PlatformEvent::NoModifiers,
PlatformMouseEvent::RealOrIndistinguishable,
WTF::monotonicallyIncreasingTime());
     m_page->deprecatedLocalMainFrame()->eventHandler().dragSourceEndedAt(pme,
         static_cast<DragOperation>(operation));
@@ -3663,7 +3664,7 @@ void WebViewImpl::dragSourceSystemDragEnded()
 
 WebDragOperation WebViewImpl::dragTargetDragEnter(
     const WebDragData& webDragData,
-    const WebPoint& clientPoint,
+    const WebPoint& pointInViewport,
     const WebPoint& screenPoint,
     WebDragOperationsMask operationsAllowed,
     int modifiers)
@@ -3673,18 +3674,18 @@ WebDragOperation WebViewImpl::dragTargetDragEnter(
     m_currentDragData = DataObject::create(webDragData);
     m_operationsAllowed = operationsAllowed;
 
-    return dragTargetDragEnterOrOver(clientPoint, screenPoint, DragEnter,
modifiers);
+    return dragTargetDragEnterOrOver(pointInViewport, screenPoint, DragEnter,
modifiers);
 }
 
 WebDragOperation WebViewImpl::dragTargetDragOver(
-    const WebPoint& clientPoint,
+    const WebPoint& pointInViewport,
     const WebPoint& screenPoint,
     WebDragOperationsMask operationsAllowed,
     int modifiers)
 {
     m_operationsAllowed = operationsAllowed;
 
-    return dragTargetDragEnterOrOver(clientPoint, screenPoint, DragOver,
modifiers);
+    return dragTargetDragEnterOrOver(pointInViewport, screenPoint, DragOver,
modifiers);
 }
 
 void WebViewImpl::dragTargetDragLeave()
@@ -3705,10 +3706,12 @@ void WebViewImpl::dragTargetDragLeave()
     m_currentDragData = nullptr;
 }
 
-void WebViewImpl::dragTargetDrop(const WebDragData& webDragData, const
WebPoint& clientPoint,
+void WebViewImpl::dragTargetDrop(const WebDragData& webDragData, const
WebPoint& pointInViewport,
     const WebPoint& screenPoint,
     int modifiers)
 {
+    WebPoint
pointInRootFrame(page()->frameHost().visualViewport().viewportToRootFrame(pointInViewport));
+
     DCHECK(m_currentDragData);
     m_currentDragData = DataObject::create(webDragData);
 
@@ -3730,7 +3733,7 @@ void WebViewImpl::dragTargetDrop(const WebDragData&
webDragData, const WebPoint&
     m_currentDragData->setModifiers(modifiers);
     DragData dragData(
         m_currentDragData.get(),
-        clientPoint,
+        pointInRootFrame,
         screenPoint,
         static_cast<DragOperation>(m_operationsAllowed));
 
@@ -3765,14 +3768,16 @@ void WebViewImpl::removeSpellingMarkersUnderWords(const
WebVector<WebString>& wo
     }
 }
 
-WebDragOperation WebViewImpl::dragTargetDragEnterOrOver(const WebPoint&
clientPoint, const WebPoint& screenPoint, DragAction dragAction, int modifiers)
+WebDragOperation WebViewImpl::dragTargetDragEnterOrOver(const WebPoint&
pointInViewport, const WebPoint& screenPoint, DragAction dragAction, int
modifiers)
 {
     DCHECK(m_currentDragData);
 
+    WebPoint
pointInRootFrame(page()->frameHost().visualViewport().viewportToRootFrame(pointInViewport));
+
     m_currentDragData->setModifiers(modifiers);
     DragData dragData(
         m_currentDragData.get(),
-        clientPoint,
+        pointInRootFrame,
         screenPoint,
         static_cast<DragOperation>(m_operationsAllowed));

[hush (inactive), 2016-10-05 17:59:21]

The CQ bit was checked by hush@chromium.org

[hush (inactive), 2016-10-05 17:59:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, bokan@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2396693002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-10-05 17:59:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 19:26:59]

Description was changed from

==========
Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

to

==========
Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

[commit-bot: I haz the power, 2016-10-05 19:27:01]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-10-05 19:28:59]

Description was changed from

==========
Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789
==========

to

==========
Convert drag event position to root frame.

The drag event positions are in viewport's coordinates when they are passed in
blink::WebView. We then need to convert these coordinates into the root frame.
In this way, the page scale factor is properly factored into the coordination
conversion.

BUG=651626, 652789

Committed: https://crrev.com/3d4fa85236725f161e2870a69faaca75c4e2a67d
Cr-Commit-Position: refs/heads/master@{#423250}
==========

[commit-bot: I haz the power, 2016-10-05 19:29:00]

Patchset 3 (id:??) landed as
https://crrev.com/3d4fa85236725f161e2870a69faaca75c4e2a67d
Cr-Commit-Position: refs/heads/master@{#423250}