Check the root frame pointer events as well for capture

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

Index: third_party/WebKit/Source/core/input/EventHandler.cpp
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp
index 9a25c2fe9e6fd714872059c463c62dbec30b83fd..1d616440e66dcdd28901ddcf6cc145b2df34ff2a 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -1182,23 +1182,57 @@ Node* EventHandler::updateMouseEventTargetNode(Node* targetNode) {
   return newNodeUnderMouse;
 }
 
+bool EventHandler::isTouchPointerEventActiveOnFrame(int pointerId,
+                                                    LocalFrame* frame) {
+  DCHECK_EQ(m_frame, m_frame->localFrameRoot());
+  return m_pointerEventManager->isTouchPointerEventActiveOnFrame(pointerId,
+                                                                 frame);
+}
+
 bool EventHandler::isPointerEventActive(int pointerId) {
-  return m_pointerEventManager->isActive(pointerId);
+  return m_pointerEventManager->isActive(pointerId) ||
+         m_frame->localFrameRoot()
+             ->eventHandler()
+             .isTouchPointerEventActiveOnFrame(pointerId, m_frame);
 }
 
 void EventHandler::setPointerCapture(int pointerId, EventTarget* target) {
   // TODO(crbug.com/591387): This functionality should be per page not per
   // frame.
-  m_pointerEventManager->setPointerCapture(pointerId, target);
+  if (m_frame != m_frame->localFrameRoot() &&

[mustaq, 2016/10/13 16:38:25]

The same check is repeated 3 times---this calls for a private bool function.

[Navid Zolghadr, 2016/10/13 17:32:32]

Done.

[mustaq, 2016/10/13 19:55:38]

I think IsPointerActiveInRootFrame is a bit misnomer because it returns true iff
the root frame knows that the pointer is active in m_frame, right? Not sure.
There is clearly two frames involved in the predicate. Is
RootFramePointerActiveInCurrentFrame a better choice? :-P

Another thought: instead of returning true/false on a given frame, can't the
PEManager return the active frame for a given pointer id so that this method
here will directly call the active frame's setPointerCapture?

[Navid Zolghadr, 2016/10/14 14:50:28]

I updated the name.
The second paragraph is not applicable. It is not correct that we should always
setPointerCapture in the active frame. Because sometimes there is no active
pointer on a frame and just because someone in that frame calls
setPointerCapture that frame shouldn't go and call setPointerCapture on a frame
that actually has that active pointer. Basically the else path is needed not
only when we have the active pointer inside this frame but also when there is no
such pointer for this frame which causes setPointerCapture has no effect which
matches one of the steps in "Setting Pointer Capture" algorithm in the spec.

+      m_frame->localFrameRoot()
+          ->eventHandler()
+          .isTouchPointerEventActiveOnFrame(pointerId, m_frame)) {
+    m_frame->localFrameRoot()->eventHandler().setPointerCapture(pointerId,
+                                                                target);
+  } else {
+    m_pointerEventManager->setPointerCapture(pointerId, target);

[mustaq, 2016/10/13 16:38:25]

Not clear why we shouldn't check |isActiveOnFrame| in both cases. Could you
please clarify?

[Navid Zolghadr, 2016/10/13 17:32:32]

setPointerCapture just ignores if the pointer is not active on the frame that
m_pointerEventManager belongs to. The check that whether a pointer is active in
the root or not is to just determine whose setPointerCapture to call.

[mustaq, 2016/10/13 19:55:38]

Does that mean there is a possibility setPointerCapture could fail silently just
because IsPointerActiveInRootFrame returned false incorrectly? This could
theoretically happen if lastNodeReceivingEvent == null there somehow but I don't
see any way this is possible. Please consider adding a check for non-null there.

[Navid Zolghadr, 2016/10/14 14:50:28]

If that lastNodeReceivingEvent is null that means that frame doesn't have that
pointerevent anymore and returing false and going to the else is correct. In
that case setPointerCapture may silently do nothing which matches one of the
steps in the spec.

+  }
 }
 
 void EventHandler::releasePointerCapture(int pointerId, EventTarget* target) {
-  m_pointerEventManager->releasePointerCapture(pointerId, target);
+  if (m_frame != m_frame->localFrameRoot() &&
+      m_frame->localFrameRoot()
+          ->eventHandler()
+          .isTouchPointerEventActiveOnFrame(pointerId, m_frame)) {
+    m_frame->localFrameRoot()->eventHandler().releasePointerCapture(pointerId,
+                                                                    target);
+  } else {
+    m_pointerEventManager->releasePointerCapture(pointerId, target);
+  }
 }
 
 bool EventHandler::hasPointerCapture(int pointerId,
                                      const EventTarget* target) const {
-  return m_pointerEventManager->hasPointerCapture(pointerId, target);
+  if (m_frame != m_frame->localFrameRoot() &&
+      m_frame->localFrameRoot()
+          ->eventHandler()
+          .isTouchPointerEventActiveOnFrame(pointerId, m_frame)) {
+    return m_frame->localFrameRoot()->eventHandler().hasPointerCapture(
+        pointerId, target);
+  } else {
+    return m_pointerEventManager->hasPointerCapture(pointerId, target);
+  }
 }
 
 bool EventHandler::hasProcessedPointerCapture(int pointerId,