Reduce hit test on ShowPress by moving event targeting to WebViewImpl

Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 53 matching lines @@
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/UniqueIdentifier.h"
 #include "core/page/Chrome.h"
 #include "core/page/ContextMenuController.h"
 #include "core/page/ContextMenuProvider.h"
 #include "core/page/DragController.h"
 #include "core/page/DragData.h"
 #include "core/page/DragSession.h"
 #include "core/page/EventHandler.h"
-#include "core/page/EventWithHitTestResults.h"
 #include "core/page/FocusController.h"
 #include "core/page/FrameTree.h"
 #include "core/page/InjectedStyleSheets.h"
 #include "core/page/Page.h"
 #include "core/page/PagePopupClient.h"
 #include "core/page/PointerLockController.h"
 #include "core/page/ScopedPageLoadDeferrer.h"
 #include "core/page/TouchDisambiguation.h"
 #include "core/rendering/RenderView.h"
 #include "core/rendering/RenderWidget.h"
@@ skipping 573 matching lines @@
             eventSwallowed = true;
 
         m_client->didHandleGestureEvent(event, eventCancelled);
         return eventSwallowed;
     default:
         break;
     }
 
     PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
 
-    // FIXME: Remove redundant hit tests by pushing the call to EventHandler::targetGestureEvent
-    // up to this point and pass GestureEventWithHitTestResults around.
+    // Special handling for double tap and scroll events as we don't want to
+    // hit test for them.
+    switch (event.type) {
+    case WebInputEvent::GestureDoubleTap:

[aelias_OOO_until_Jul13, 2014/08/27 03:26:34]

It seems strange to treat double-tap specially here and I don't follow what
makes it fundamentally different from the other tap-related events.  (Double-tap
also effectively hit-tests and then figures out a containing node.)  Could you
explain or stop special-casing it?

[Zeeshan Qureshi, 2014/08/27 19:37:56]

The way I see it is that DoubleTap is handled directly in WebViewImpl whereas
most of the others go down to blink::core and get processed there. Looking at
animateDoubleTapZoom, it might be doing a hit test but it's not quite like what
we do in the event targeting for the others. It was already special cased in the
previous switch, we're just moving it up a bit to spare the unnecessary hit test
we would have done.

Maybe we should start tracking how many hit tests are triggered for a double tap
and look into unifying it if that's an issue.

[Rick Byers, 2014/08/27 20:30:25]

I agree that this CL doesn't change the special-handling for double tap at all -
just moves it up above the blink core hit test we're doing.  But I agree that
this comment is misleading - we do want to hit test for double tap (looks like
it happens in WebViewImpl::computeBlockBounds).  Perhaps we should just add a
FIXME saying to unify double-tap hit testing with targetGestureEvent?  That
would have the benefit of increasing the chance of getting a hit on the (to be
written) hit-test cache, and probably wouldn't be hard to implement (eg. change
animateDoubleTapZoom to take a GestureEventWithHitTestResults).  Bug again I
don't think that needs to happen in this CL (although it's simple enough that
I'm not opposed to lumping it in here too).

+        if (m_webSettings->doubleTapToZoomEnabled() && minimumPageScaleFactor() != maximumPageScaleFactor()) {
+            m_client->cancelScheduledContentIntents();
+            animateDoubleTapZoom(platformEvent.position());
+        }
+        // GestureDoubleTap is currently only used by Android for zooming. For WebCore,
+        // GestureTap with tap count = 2 is used instead. So we drop GestureDoubleTap here.
+        eventSwallowed = true;
+        m_client->didHandleGestureEvent(event, eventCancelled);
+        return eventSwallowed;
+    case WebInputEvent::GestureScrollBegin:
+    case WebInputEvent::GesturePinchBegin:
+        m_client->cancelScheduledContentIntents();
+    case WebInputEvent::GestureScrollEnd:
+    case WebInputEvent::GestureScrollUpdate:
+    case WebInputEvent::GestureScrollUpdateWithoutPropagation:
+    case WebInputEvent::GesturePinchEnd:
+    case WebInputEvent::GesturePinchUpdate:
+    case WebInputEvent::GestureFlingStart:
+        // Scrolling-related gesture events invoke EventHandler recursively for each frame down
+        // the chain, doing a single-frame hit-test per frame. This matches handleWheelEvent.
+        // Perhaps we could simplify things by rewriting scroll handling to work inner frame
+        // out, and then unify with other gesture events.
+        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureScrollEvent(platformEvent);
+        m_client->didHandleGestureEvent(event, eventCancelled);
+        return eventSwallowed;
+    default:
+        break;
+    }
+
+    // Hit test across all frames and do touch adjustment as necessary for the event type.
+    GestureEventWithHitTestResults targetedEvent =
+        m_page->deprecatedLocalMainFrame()->eventHandler().targetGestureEvent(platformEvent);
 
     // Handle link highlighting outside the main switch to avoid getting lost in the
     // complicated set of cases handled below.
     switch (event.type) {
     case WebInputEvent::GestureShowPress:
         // Queue a highlight animation, then hand off to regular handler.
-        enableTapHighlightAtPoint(platformEvent);
+        enableTapHighlightAtPoint(targetedEvent);
         break;
     case WebInputEvent::GestureTapCancel:
     case WebInputEvent::GestureTap:
     case WebInputEvent::GestureLongPress:
         for (size_t i = 0; i < m_linkHighlights.size(); ++i)
             m_linkHighlights[i]->startHighlightAnimationIfNeeded();
         break;
     default:
         break;
     }
 
     switch (event.type) {
     case WebInputEvent::GestureTap: {
         m_client->cancelScheduledContentIntents();
-        if (detectContentOnTouch(platformEvent.position())) {
+        // FIXME: Use targeted event here and save another hit test.
+        if (detectContentOnTouch(targetedEvent.event().position())) {
             eventSwallowed = true;
             break;
         }
 
         RefPtr<PopupContainer> selectPopup;
         selectPopup = m_selectPopup;
         hideSelectPopup();
         ASSERT(!m_selectPopup);
 
         // Don't trigger a disambiguation popup on sites designed for mobile devices.
@@ skipping 15 matching lines @@
             if (goodTargets.size() >= 2 && m_client && m_client->didTapMultipleTargets(scaledEvent, goodTargets)) {
                 enableTapHighlights(highlightNodes);
                 for (size_t i = 0; i < m_linkHighlights.size(); ++i)
                     m_linkHighlights[i]->startHighlightAnimationIfNeeded();
                 eventSwallowed = true;
                 eventCancelled = true;
                 break;
             }
         }
 
-        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(platformEvent);
+        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(targetedEvent);
 
         if (m_selectPopup && m_selectPopup == selectPopup) {
             // That tap triggered a select popup which is the same as the one that
             // was showing before the tap. It means the user tapped the select
             // while the popup was showing, and as a result we first closed then
             // immediately reopened the select popup. It needs to be closed.
             hideSelectPopup();
         }
 
         break;
     }
     case WebInputEvent::GestureTwoFingerTap:
     case WebInputEvent::GestureLongPress:
     case WebInputEvent::GestureLongTap: {
         if (!mainFrameImpl() || !mainFrameImpl()->frameView())
             break;
 
         m_client->cancelScheduledContentIntents();
         m_page->contextMenuController().clearContextMenu();
         m_contextMenuAllowed = true;
-        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(platformEvent);
+        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(targetedEvent);
         m_contextMenuAllowed = false;
 
         break;
     }
-    case WebInputEvent::GestureShowPress: {
-        m_client->cancelScheduledContentIntents();
-        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(platformEvent);
-        break;
-    }
-    case WebInputEvent::GestureDoubleTap:
-        if (m_webSettings->doubleTapToZoomEnabled() && minimumPageScaleFactor() != maximumPageScaleFactor()) {
-            m_client->cancelScheduledContentIntents();
-            animateDoubleTapZoom(platformEvent.position());
-        }
-        // GestureDoubleTap is currently only used by Android for zooming. For WebCore,
-        // GestureTap with tap count = 2 is used instead. So we drop GestureDoubleTap here.
-        eventSwallowed = true;
-        break;
-    case WebInputEvent::GestureScrollBegin:
-    case WebInputEvent::GesturePinchBegin:
+    case WebInputEvent::GestureShowPress:
         m_client->cancelScheduledContentIntents();
     case WebInputEvent::GestureTapDown:
-    case WebInputEvent::GestureScrollEnd:
-    case WebInputEvent::GestureScrollUpdate:
-    case WebInputEvent::GestureScrollUpdateWithoutPropagation:
     case WebInputEvent::GestureTapCancel:
-    case WebInputEvent::GestureTapUnconfirmed:
-    case WebInputEvent::GesturePinchEnd:
-    case WebInputEvent::GesturePinchUpdate:
-    case WebInputEvent::GestureFlingStart: {
-        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(platformEvent);
+    case WebInputEvent::GestureTapUnconfirmed: {
+        eventSwallowed = mainFrameImpl()->frame()->eventHandler().handleGestureEvent(targetedEvent);
         break;
     }
     default:
         ASSERT_NOT_REACHED();
     }
     m_client->didHandleGestureEvent(event, eventCancelled);
     return eventSwallowed;
 }
 
 void WebViewImpl::transferActiveWheelFlingAnimation(const WebActiveWheelFlingParameters& parameters)
@@ skipping 399 matching lines @@
 static bool showsHandCursor(Node* node, LocalFrame* frame)
 {
     if (!node || !node->renderer())
         return false;
 
     ECursor cursor = node->renderer()->style()->cursor();
     return cursor == CURSOR_POINTER
         || (cursor == CURSOR_AUTO && frame->eventHandler().useHandCursor(node, node->isLink()));
 }
 
-Node* WebViewImpl::bestTapNode(const PlatformGestureEvent& tapEvent)
+Node* WebViewImpl::bestTapNode(const GestureEventWithHitTestResults& targetedTapEvent)
 {
     TRACE_EVENT0("input", "WebViewImpl::bestTapNode");
 
     if (!m_page || !m_page->mainFrame())
         return 0;
 
-    // FIXME: Rely on earlier hit test instead of hit testing again.
-    GestureEventWithHitTestResults targetedEvent =
-        m_page->deprecatedLocalMainFrame()->eventHandler().targetGestureEvent(tapEvent, true);
-    Node* bestTouchNode = targetedEvent.hitTestResult().targetNode();
+    Node* bestTouchNode = targetedTapEvent.hitTestResult().targetNode();
 
     // We might hit something like an image map that has no renderer on it
     // Walk up the tree until we have a node with an attached renderer
     while (bestTouchNode && !bestTouchNode->renderer())
         bestTouchNode = NodeRenderingTraversal::parent(bestTouchNode);
 
     Node* cursorDefiningAncestor =
         findCursorDefiningAncestor(bestTouchNode, m_page->deprecatedLocalMainFrame());
     // We show a highlight on tap only when the current node shows a hand cursor
     if (!cursorDefiningAncestor || !showsHandCursor(cursorDefiningAncestor, m_page->deprecatedLocalMainFrame())) {
         return 0;
     }
 
     // We should pick the largest enclosing node with hand cursor set. We do this by first jumping
     // up to cursorDefiningAncestor (which is already known to have hand cursor set). Then we locate
     // the next cursor-defining ancestor up in the the tree and repeat the jumps as long as the node
     // has hand cursor set.
     do {
         bestTouchNode = cursorDefiningAncestor;
         cursorDefiningAncestor = findCursorDefiningAncestor(NodeRenderingTraversal::parent(bestTouchNode),
             m_page->deprecatedLocalMainFrame());
     } while (cursorDefiningAncestor && showsHandCursor(cursorDefiningAncestor, m_page->deprecatedLocalMainFrame()));
 
     return bestTouchNode;
 }
 
-void WebViewImpl::enableTapHighlightAtPoint(const PlatformGestureEvent& tapEvent)
+void WebViewImpl::enableTapHighlightAtPoint(const GestureEventWithHitTestResults& targetedTapEvent)
 {
-    Node* touchNode = bestTapNode(tapEvent);
+    Node* touchNode = bestTapNode(targetedTapEvent);
 
     WillBeHeapVector<RawPtrWillBeMember<Node> > highlightNodes;
     highlightNodes.append(touchNode);
 
     enableTapHighlights(highlightNodes);
 }
 
 void WebViewImpl::enableTapHighlights(WillBeHeapVector<RawPtrWillBeMember<Node> >& highlightNodes)
 {
     if (highlightNodes.isEmpty())
@@ skipping 3019 matching lines @@
     const PageScaleConstraints& constraints = m_pageScaleConstraintsSet.pageDefinedConstraints();
 
     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
         return false;
 
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 } // namespace blink