Store the last_show_press_timestamp_ in root frame EventHandler

This issue is caused by:

1. When user tap on screen, we receive TapDown, ShowPress, Tap.
   TapDown and ShowPress will active the tapped element. Then Tap
   checks the LastShowPressTimeStamp() to stay active or cancel.
2. Each frame has GestureManager and we store LastShowPressTimeStamp()
   in GestureManager.
3. We check the root frame GestureManager LastShowPressTimeStamp()
   before hit test so we read the wrong LastShowPressTimeStamp().

In this patch, we move last_show_press_timestamp_ to root frame
EventHandler so we don't need to hit test and figure which
GestureManager should use.

Bug: 714573
Change-Id: I80c75bf2f0493a6cbfd3b24873e1127da4fd7a27
Test: EventHandlerSimTest_TapActiveInFrame
Test: Manual test for OOPIF
Reviewed-on: https://chromium-review.googlesource.com/1041065Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555719}

third_party/blink/renderer/core/input/event_handler.cc

@@ -209,6 +209,7 @@ void EventHandler::Clear() {
   gesture_manager_->Clear();
   mouse_event_manager_->Clear();
   mouse_wheel_event_manager_->Clear();
+  last_show_press_timestamp_.reset();
   last_deferred_tap_element_ = nullptr;
   event_handler_will_reset_capturing_mouse_events_node_ = false;
   should_use_touch_event_adjusted_point_ = false;
@@ -1323,6 +1324,9 @@ WebInputEventResult EventHandler::HandleGestureEvent(
   // etc.
   DCHECK(!targeted_event.Event().IsScrollEvent());
 
+  if (targeted_event.Event().GetType() == WebInputEvent::kGestureShowPress)
+    last_show_press_timestamp_ = CurrentTimeTicks();
+
   // Update mouseout/leave/over/enter events before jumping directly to the
   // inner most frame.
   if (targeted_event.Event().GetType() == WebInputEvent::kGestureTap)
@@ -1641,12 +1645,11 @@ GestureEventWithHitTestResults EventHandler::TargetGestureEvent(
   if (read_only) {
     hit_type |= HitTestRequest::kReadOnly;
   } else if (gesture_event.GetType() == WebInputEvent::kGestureTap &&
-             gesture_manager_->GetLastShowPressTimestamp()) {
+             last_show_press_timestamp_) {
     // 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 a couple of frames.
-    active_interval = CurrentTimeTicks() -
-                      gesture_manager_->GetLastShowPressTimestamp().value();
+    active_interval = CurrentTimeTicks() - last_show_press_timestamp_.value();
     should_keep_active_for_min_interval =
         active_interval < kMinimumActiveInterval;
     if (should_keep_active_for_min_interval)
@@ -1657,9 +1660,10 @@ GestureEventWithHitTestResults EventHandler::TargetGestureEvent(
       HitTestResultForGestureEvent(gesture_event, hit_type);
   // Now apply hover/active state to the final target.
   HitTestRequest request(hit_type | HitTestRequest::kAllowChildFrameContent);
-  if (!request.ReadOnly())
+  if (!request.ReadOnly()) {
     UpdateGestureHoverActiveState(
         request, event_with_hit_test_results.GetHitTestResult().InnerElement());
+  }
 
   if (should_keep_active_for_min_interval) {
     last_deferred_tap_element_ =

third_party/blink/renderer/core/input/event_handler.h

@@ -28,6 +28,7 @@
 
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "third_party/blink/public/platform/web_input_event.h"
 #include "third_party/blink/public/platform/web_input_event_result.h"
 #include "third_party/blink/public/platform/web_menu_source_type.h"
@@ -51,6 +52,7 @@
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 #include "third_party/blink/renderer/platform/wtf/hash_traits.h"
+#include "third_party/blink/renderer/platform/wtf/time.h"
 
 namespace blink {
 
@@ -398,7 +400,13 @@ class CORE_EXPORT EventHandler final
   bool long_tap_should_invoke_context_menu_;
 
   TaskRunnerTimer<EventHandler> active_interval_timer_;
-  double last_show_press_timestamp_;
+
+  // last_show_press_timestamp_ prevents the active state rewrited by following
+  // events too soon (less than 0.15s).
+  // It is ok we only record last_show_press_timestamp_ in root frame since
+  // root frame will have subframe as active element if subframe has active
+  // element.
+  base::Optional<WTF::TimeTicks> last_show_press_timestamp_;
   Member<Element> last_deferred_tap_element_;
 
   // Set on GestureTapDown if unique_touch_event_id_ matches cached adjusted

third_party/blink/renderer/core/input/event_handler_test.cc

@@ -27,6 +27,7 @@
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_request.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_test.h"
+#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 
 namespace blink {
 
@@ -55,6 +56,36 @@ class TapEventBuilder : public WebGestureEvent {
   }
 };
 
+class TapDownEventBuilder : public WebGestureEvent {
+ public:
+  TapDownEventBuilder(FloatPoint position)
+      : WebGestureEvent(WebInputEvent::kGestureTapDown,
+                        WebInputEvent::kNoModifiers,
+                        CurrentTimeTicks(),
+                        kWebGestureDeviceTouchscreen) {
+    SetPositionInWidget(position);
+    SetPositionInScreen(position);
+    data.tap_down.width = 5;
+    data.tap_down.height = 5;
+    frame_scale_ = 1;
+  }
+};
+
+class ShowPressEventBuilder : public WebGestureEvent {
+ public:
+  ShowPressEventBuilder(FloatPoint position)
+      : WebGestureEvent(WebInputEvent::kGestureShowPress,
+                        WebInputEvent::kNoModifiers,
+                        CurrentTimeTicks(),
+                        kWebGestureDeviceTouchscreen) {
+    SetPositionInWidget(position);
+    SetPositionInScreen(position);
+    data.show_press.width = 5;
+    data.show_press.height = 5;
+    frame_scale_ = 1;
+  }
+};
+
 class LongPressEventBuilder : public WebGestureEvent {
  public:
   LongPressEventBuilder(FloatPoint position)
@@ -1154,4 +1185,68 @@ TEST_F(EventHandlerSimTest, CursorStyleBeforeStartDragging) {
                                      .GetType());
 }
 
+// Ensure that tap on element in iframe should apply active state.
+TEST_F(EventHandlerSimTest, TapActiveInFrame) {
+  WebView().Resize(WebSize(800, 600));
+
+  SimRequest main_resource("https://example.com/test.html", "text/html");
+  SimRequest frame_resource("https://example.com/iframe.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  main_resource.Complete(R"HTML(
+    <!DOCTYPE html>
+    <style>
+    body {
+      margin: 0;
+    }
+    iframe {
+      width: 200px;
+      height: 200px;
+    }
+    </style>
+    <iframe id='iframe' src='iframe.html'>
+    </iframe>
+  )HTML");
+
+  frame_resource.Complete(R"HTML(
+    <!DOCTYPE html>
+    <style>
+    body {
+      margin: 0;
+    }
+    div {
+      width: 100px;
+      height: 100px;
+    }
+    </style>
+    <div></div>
+  )HTML");
+  Compositor().BeginFrame();
+
+  auto* iframe_element =
+      ToHTMLIFrameElement(GetDocument().getElementById("iframe"));
+  Document* iframe_doc = iframe_element->contentDocument();
+
+  TapDownEventBuilder tap_down(FloatPoint(10, 10));
+  GetDocument().GetFrame()->GetEventHandler().HandleGestureEvent(tap_down);
+
+  ShowPressEventBuilder show_press(FloatPoint(10, 10));
+  GetDocument().GetFrame()->GetEventHandler().HandleGestureEvent(show_press);
+
+  // TapDown and ShowPress active the iframe.
+  EXPECT_TRUE(GetDocument().GetActiveElement());
+  EXPECT_TRUE(iframe_doc->GetActiveElement());
+
+  TapEventBuilder tap(FloatPoint(10, 10), 1);
+  GetDocument().GetFrame()->GetEventHandler().HandleGestureEvent(tap);
+
+  // Should still active.
+  EXPECT_TRUE(GetDocument().GetActiveElement());
+  EXPECT_TRUE(iframe_doc->GetActiveElement());
+
+  // The active will cancel after 15ms.
+  test::RunDelayedTasks(TimeDelta::FromSecondsD(0.2));
+  EXPECT_FALSE(GetDocument().GetActiveElement());
+  EXPECT_FALSE(iframe_doc->GetActiveElement());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/input/gesture_manager.cc

@@ -49,7 +49,6 @@ GestureManager::GestureManager(LocalFrame& frame,
 void GestureManager::Clear() {
   suppress_mouse_events_from_gestures_ = false;
   long_tap_should_invoke_context_menu_ = false;
-  last_show_press_timestamp_.reset();
 }
 
 void GestureManager::Trace(blink::Visitor* visitor) {
@@ -430,8 +429,6 @@ WebInputEventResult GestureManager::SendContextMenuEventForGesture(
 }
 
 WebInputEventResult GestureManager::HandleGestureShowPress() {
-  last_show_press_timestamp_ = CurrentTimeTicks();
-
   LocalFrameView* view = frame_->View();
   if (!view)
     return WebInputEventResult::kNotHandled;
@@ -448,11 +445,6 @@ WebInputEventResult GestureManager::HandleGestureShowPress() {
   return WebInputEventResult::kNotHandled;
 }
 
-base::Optional<WTF::TimeTicks> GestureManager::GetLastShowPressTimestamp()
-    const {
-  return last_show_press_timestamp_;
-}
-
 void GestureManager::ShowUnhandledTapUIIfNeeded(
     bool dom_tree_changed,
     bool style_changed,

third_party/blink/renderer/core/input/gesture_manager.h

@@ -6,13 +6,11 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_INPUT_GESTURE_MANAGER_H_
 
 #include "base/macros.h"
-#include "base/optional.h"
 #include "third_party/blink/public/platform/web_input_event_result.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/layout/hit_test_request.h"
 #include "third_party/blink/renderer/core/page/event_with_hit_test_results.h"
-#include "third_party/blink/renderer/platform/wtf/time.h"
 
 namespace blink {
 
@@ -41,11 +39,6 @@ class CORE_EXPORT GestureManager
   WebInputEventResult HandleGestureEventInFrame(
       const GestureEventWithHitTestResults&);
 
-  // TODO(nzolghadr): This can probably be hidden and the related logic
-  // be moved to this class (see crrev.com/112023010). Since that might cause
-  // regression it's better to move that logic in another change.
-  base::Optional<WTF::TimeTicks> GetLastShowPressTimestamp() const;
-
  private:
   WebInputEventResult HandleGestureShowPress();
   WebInputEventResult HandleGestureTapDown(
@@ -85,7 +78,6 @@ class CORE_EXPORT GestureManager
 
   const Member<SelectionController> selection_controller_;
 
-  base::Optional<WTF::TimeTicks> last_show_press_timestamp_;
   DISALLOW_COPY_AND_ASSIGN(GestureManager);
 };