Send early ShowPress on TapDown when page isn't scrollable/pinchable.

Source/core/page/EventHandler.cpp

Index: Source/core/page/EventHandler.cpp
diff --git a/Source/core/page/EventHandler.cpp b/Source/core/page/EventHandler.cpp
index c6c5a70987acb60a90b99bea6bda6d7faa59b87d..dcc3554012221fc50962bea522e26c5c18ea02c3 100644
--- a/Source/core/page/EventHandler.cpp
+++ b/Source/core/page/EventHandler.cpp
@@ -57,6 +57,7 @@
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/html/HTMLFrameSetElement.h"
 #include "core/html/HTMLInputElement.h"
+#include "core/loader/EmptyClients.h"

[Rick Byers, 2014/02/14 21:27:53]

why do you need this new include?  Seems odd...

 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/page/AutoscrollController.h"
@@ -298,7 +299,8 @@ EventHandler::EventHandler(Frame* frame)
     , m_longTapShouldInvokeContextMenu(false)
     , m_syntheticPageScaleFactor(0)
     , m_activeIntervalTimer(this, &EventHandler::activeIntervalTimerFired)
-    , m_lastShowPressTimestamp(0)
+    , m_lastActiveTimestamp(0)
+    , m_tapDownTriggeredActive(false)
 {
 }
 
@@ -354,8 +356,9 @@ void EventHandler::clear()
     m_touchPressed = false;
     m_mouseDownMayStartSelect = false;
     m_mouseDownMayStartDrag = false;
-    m_lastShowPressTimestamp = 0;
+    m_lastActiveTimestamp = 0;
     m_lastDeferredTapElement = 0;
+    m_tapDownTriggeredActive = false;
 }
 
 void EventHandler::nodeWillBeRemoved(Node& nodeToBeRemoved)
@@ -2222,7 +2225,8 @@ void EventHandler::defaultWheelEventHandler(Node* startNode, WheelEvent* wheelEv
 
 bool EventHandler::handleGestureShowPress()
 {
-    m_lastShowPressTimestamp = WTF::currentTime();
+    if (!m_tapDownTriggeredActive)
+        m_lastActiveTimestamp = WTF::currentTime();
 
     FrameView* view = m_frame->view();
     if (!view)
@@ -2254,9 +2258,10 @@ bool EventHandler::handleGestureEvent(const PlatformGestureEvent& gestureEvent)
         // Handle directly in main frame
         break;
 
+    case PlatformEvent::GestureTapDown:
+        m_tapDownTriggeredActive = false;
     case PlatformEvent::GestureTap:
     case PlatformEvent::GestureTapUnconfirmed:
-    case PlatformEvent::GestureTapDown:
     case PlatformEvent::GestureShowPress:
     case PlatformEvent::GestureTapDownCancel:
     case PlatformEvent::GestureTwoFingerTap:
@@ -2288,8 +2293,34 @@ bool EventHandler::handleGestureEvent(const PlatformGestureEvent& gestureEvent)
     HitTestRequest::HitTestRequestType hitType = HitTestRequest::TouchEvent;
     double activeInterval = 0;
     bool shouldKeepActiveForMinInterval = false;
-    if (gestureEvent.type() == PlatformEvent::GestureShowPress
-        || gestureEvent.type() == PlatformEvent::GestureTapUnconfirmed) {
+    if (gestureEvent.type() == PlatformEvent::GestureTapDown) {
+        Page* page = m_frame->page();

[bokan, 2014/02/18 19:59:57]

Nit: I think this is involved enough that you could factor this out into a
helper method.

+        Frame* mainFrame = page ? page->mainFrame() : 0;
+        FrameView* frameView = mainFrame ? mainFrame->view() : 0;
+        ChromeClient& chromeClient = mainFrame ? mainFrame->chromeClient() : *new EmptyChromeClient();

[Rick Byers, 2014/02/14 21:27:53]

we should probably avoid creating a fake client (just use a different branch
instead).  Also you'd want stack allocation not heap allocation to avoid
leaking.

+
+        // Only activate on TapDown if page isn't scrollable or pinchable and
+        // this isn't the start of a scroll.
+        if (frameView && !frameView->isScrollable() && chromeClient.minimumPageScaleFactor() == chromeClient.maximumPageScaleFactor()) {
+            IntPoint hitTestPoint = m_frame->view()->windowToContents(adjustedPoint);
+            HitTestResult result = hitTestResultAtPoint(hitTestPoint, HitTestRequest::TouchEvent | HitTestRequest::ReadOnly  | HitTestRequest::AllowFrameScrollbars);
+            RenderObject* renderer = result.innerElement()->renderer();
+
+            // If no ancestor is scrollable then this couldn't possibly be the start of a scroll.
+            bool shouldTapDownTriggerActive = true;
+            for (RenderLayer* parent = renderer ? renderer->enclosingLayer() : 0; parent && shouldTapDownTriggerActive; parent = parent->parent()) {

[bokan, 2014/02/18 19:59:57]

I think this will break if your renderer is contained in an iframe; there may be
a scrollable frame or renderer up the hierarchy, but parent() will end at the
RenderView for the frame.

I think you should walk up ScrollableArea::enclosingScrollableArea, but they're
filled with FIXMEs to implement this case I just mentioned. See 
EventHandler::bubblingScroll for how to do this. If you're looking for brownie
points, implement those ScrollableArea FIXMEs and use those. 

+                if (parent->scrollsOverflow())
+                    shouldTapDownTriggerActive = false;

[bokan, 2014/02/18 19:59:57]

You can 'break' out of the loop here 

+            }
+
+            if (shouldTapDownTriggerActive) {
+                m_tapDownTriggeredActive = true;
+                hitType |= HitTestRequest::Active;
+            }
+        }
+    } else if (gestureEvent.type() == PlatformEvent::GestureShowPress && !m_tapDownTriggeredActive) {
+        hitType |= HitTestRequest::Active;
+    } else if (gestureEvent.type() == PlatformEvent::GestureTapUnconfirmed) {
         hitType |= HitTestRequest::Active;
     } else if (gestureEvent.type() == PlatformEvent::GestureTapDownCancel) {
         hitType |= HitTestRequest::Release;
@@ -2300,8 +2331,8 @@ bool EventHandler::handleGestureEvent(const PlatformGestureEvent& gestureEvent)
         hitType |= HitTestRequest::Release;
         // If the Tap is received very shortly after ShowPress, we want to delay clearing
         // of the active state so that it's visible to the user for at least one frame.
-        activeInterval = WTF::currentTime() - m_lastShowPressTimestamp;
-        shouldKeepActiveForMinInterval = m_lastShowPressTimestamp && activeInterval < minimumActiveInterval;
+        activeInterval = WTF::currentTime() - m_lastActiveTimestamp;
+        shouldKeepActiveForMinInterval = m_lastActiveTimestamp && activeInterval < minimumActiveInterval;
         if (shouldKeepActiveForMinInterval)
             hitType |= HitTestRequest::ReadOnly;
     }
@@ -2378,6 +2409,8 @@ bool EventHandler::handleGestureEvent(const PlatformGestureEvent& gestureEvent)
     case PlatformEvent::GestureTwoFingerTap:
         return handleGestureTwoFingerTap(gestureEvent, adjustedPoint);
     case PlatformEvent::GestureTapDown:
+        if (m_tapDownTriggeredActive)
+            m_lastActiveTimestamp = WTF::currentTime();
     case PlatformEvent::GesturePinchBegin:
     case PlatformEvent::GesturePinchEnd:
     case PlatformEvent::GesturePinchUpdate: