Move touch hit testing to PointerEventManager

third_party/WebKit/Source/core/input/PointerEventManager.cpp

Index: third_party/WebKit/Source/core/input/PointerEventManager.cpp
diff --git a/third_party/WebKit/Source/core/input/PointerEventManager.cpp b/third_party/WebKit/Source/core/input/PointerEventManager.cpp
index 1528d20163ce97e7746e7dda79ead3d038609fce..24326593accfdb7c151c41862db13a9705d1299b 100644
--- a/third_party/WebKit/Source/core/input/PointerEventManager.cpp
+++ b/third_party/WebKit/Source/core/input/PointerEventManager.cpp
@@ -10,6 +10,9 @@
 #include "core/frame/FrameView.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/input/EventHandler.h"
+#include "core/input/TouchActionUtil.h"
+#include "core/page/ChromeClient.h"
+#include "core/page/Page.h"
 #include "platform/PlatformTouchEvent.h"
 
 namespace blink {
@@ -397,11 +400,6 @@ WebInputEventResult PointerEventManager::handleTouchEvents(
         unblockTouchPointers();
     HeapVector<TouchEventManager::TouchInfo> touchInfos;
 
-    // TODO(crbug.com/606822): This will be moved after pointer events so
-    // pointer event operations will get the first shot to fill up this array.
-    if (!m_touchEventManager.generateTouchInfosAfterHittest(event, touchInfos))
-        return WebInputEventResult::NotHandled;
-
     dispatchTouchPointerEvents(event, touchInfos);
 
     return m_touchEventManager.handleTouchEvent(event, touchInfos);
@@ -411,30 +409,73 @@ void PointerEventManager::dispatchTouchPointerEvents(
     const PlatformTouchEvent& event,
     HeapVector<TouchEventManager::TouchInfo>& touchInfos)
 {
-    if (!RuntimeEnabledFeatures::pointerEventEnabled())
-        return;
-
-    if (m_inCanceledStateForPointerTypeTouch)
-        return;
-
     // Iterate through the touch points, sending PointerEvents to the targets as required.
-    for (auto& touchInfo: touchInfos) {
-        const PlatformTouchPoint &touchPoint = touchInfo.point;
+    for (const auto& touchPoint : event.touchPoints()) {
+        TouchEventManager::TouchInfo touchInfo;
+        touchInfo.point = touchPoint;
+
+        int pointerId = m_pointerEventFactory.getPointerEventId(
+            touchPoint.pointerProperties());
+        // Do the hit test either when the touch first starts or when the touch
+        // is not captured.
+        if (touchInfo.point.state() == PlatformTouchPoint::TouchPressed
+            || !m_pendingPointerCaptureTarget.contains(pointerId)) {

[mustaq, 2016/05/13 20:03:16]

It took some time to realize why this is m_pendingPointerCaptureTarget instead
of m_pointerCaptureTarget. Same below. Worth adding a note that we process
pending capture later on.

[Navid Zolghadr, 2016/05/16 20:01:50]

Done.

+            HitTestRequest::HitTestRequestType hitType = HitTestRequest::TouchEvent | HitTestRequest::ReadOnly | HitTestRequest::Active;
+            LayoutPoint pagePoint = roundedLayoutPoint(m_frame->view()->rootFrameToContents(touchInfo.point.pos()));
+            HitTestResult hitTestTesult = m_frame->eventHandler().hitTestResultAtPoint(pagePoint, hitType);
+            Node* node = hitTestTesult.innerNode();
+            if (node) {
+                touchInfo.targetFrame = node->document().frame();
+                if (isHTMLCanvasElement(node)) {
+                    std::pair<Element*, String> regionInfo = toHTMLCanvasElement(node)->getControlAndIdIfHitRegionExists(hitTestTesult.pointInInnerNodeFrame());
+                    if (regionInfo.first)
+                        node = regionInfo.first;
+                    touchInfo.region = regionInfo.second;
+                }
+                // Touch events should not go to text nodes.
+                if (node->isTextNode())
+                    node = FlatTreeTraversal::parent(*node);
+                touchInfo.touchNode = node;
+
+                TouchAction effectiveTouchAction =

[dtapuska, 2016/05/13 16:51:36]

Is this correct to do here? What if the touch sequence document is different?

[Navid Zolghadr, 2016/05/13 17:27:55]

I'm not sure. My understanding was that the touchaction will be per touch and
the node that that particular touch is on. Mustaq, WDYT? Should I remove this
from here and put it back in TouchEventManager?

[mustaq, 2016/05/13 19:18:01]

The info in communicated back to browser through this IPC handler:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
so the input_router for the correct frame should receive the IPC. So I think
Dave is correct: this should move to TouchEventManager.

Thanks Dave for the catch.

[Navid Zolghadr, 2016/05/16 20:01:50]

Done.

+                    TouchActionUtil::computeEffectiveTouchAction(
+                        *touchInfo.touchNode);
+                if (effectiveTouchAction != TouchActionAuto)
+                    m_frame->page()->chromeClient().setTouchAction(effectiveTouchAction);
+            }
+        } else {
+            touchInfo.touchNode = m_pendingPointerCaptureTarget.get(pointerId)->toNode();

[mustaq, 2016/05/13 20:03:16]

If we remove/disable implicit touch capturing, I suspect we can have
pendingPointerCaptureTarget==null and pointerCaptureTarget!=null here. If this
is the case, please add a TODO/comment here.

[Navid Zolghadr, 2016/05/16 20:01:50]

That is right. But I also check for this condition in the if condition and if
pendingPointercaptureTarget is null I do hit test. I added a comment here as
well to explain.

+            touchInfo.targetFrame = touchInfo.touchNode->document().frame();
+        }
+
         WebInputEventResult result = WebInputEventResult::NotHandled;
-        // Do not send pointer events for stationary touches.
-        if (touchPoint.state() != PlatformTouchPoint::TouchStationary) {
-            // TODO(crbug.com/608394): The adjustedPagePoint should be converted
-            // to client coordinates.
-            PointerEvent* pointerEvent = m_pointerEventFactory.create(
-                pointerEventNameForTouchPointState(touchPoint.state()),
-                touchPoint, event.getModifiers(),
-                touchInfo.adjustedRadius,
-                touchInfo.adjustedPagePoint);
-
-            // Consume the touch point if its pointer event is anything but NotHandled
-            // (e.g. preventDefault is called in the listener for the pointer event).
-            result = sendTouchPointerEvent(touchInfo.touchNode, pointerEvent);
-            touchInfo.consumed = result != WebInputEventResult::NotHandled;
+
+        if (touchInfo.targetFrame) {
+            FloatPoint pagePoint = touchInfo.targetFrame->view()
+                ->rootFrameToContents(touchInfo.point.pos());
+            float scaleFactor = 1.0f / touchInfo.targetFrame->pageZoomFactor();
+
+            touchInfo.adjustedPagePoint = pagePoint.scaledBy(scaleFactor);
+            touchInfo.adjustedRadius = touchInfo.point.radius().scaledBy(scaleFactor);
+
+            // Do not send pointer events for stationary touches.
+            if (touchPoint.state() != PlatformTouchPoint::TouchStationary
+                && !m_inCanceledStateForPointerTypeTouch) {
+                // TODO(crbug.com/608394): The adjustedPagePoint should be converted

[mustaq, 2016/05/11 14:30:55]

Isn't adjustedPagePoint now in client coordinates? The bug is fixed.

[Navid Zolghadr, 2016/05/11 16:43:42]

I need to rebase this change to get the updates.

[Navid Zolghadr, 2016/05/16 20:01:50]

Done.

+                // to client coordinates.
+                PointerEvent* pointerEvent = m_pointerEventFactory.create(
+                    pointerEventNameForTouchPointState(touchPoint.state()),
+                    touchPoint, event.getModifiers(),
+                    touchInfo.adjustedRadius,
+                    touchInfo.adjustedPagePoint);
+
+                // Consume the touch point if its pointer event is anything but NotHandled
+                // (e.g. preventDefault is called in the listener for the pointer event).
+                result = sendTouchPointerEvent(touchInfo.touchNode, pointerEvent);

[mustaq, 2016/05/11 14:30:55]

This is now leaking PEs I believe! Please check if PEs are enabled or not.

[Navid Zolghadr, 2016/05/11 16:43:42]

No. It does check the pointer enable at the last stage before sending the
pointer event.

+            }
+        }
+        if (result == WebInputEventResult::NotHandled) {
+            touchInfos.append(touchInfo);
         }
     }
 }