Enable UnifiedTouchAdjustment feature by default

Turn blink runtime_enable_feature flags to stable; Removes content_feature flags used for finch trial; Makes related histogram as obselete. intent to ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/l5PCVakcOCE Bug: 805650, 625198 Change-Id: I873a77f98670f5f293b20a1b3a4373ace93027db Reviewed-on: https://chromium-review.googlesource.com/978626 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Rick Byers <rbyers@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Commit-Queue: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/heads/master@{#557989}
Denger-Network · May 11, 2018 · 05e7982 · 05e7982
1 parent 403ed15
commit 05e7982
Show file tree

Hide file tree

Showing 13 changed files with 34 additions and 77 deletions.
diff --git a/content/child/runtime_features.cc b/content/child/runtime_features.cc
@@ -430,9 +430,6 @@ void SetRuntimeFeaturesDefaultsAndUpdateFromArgs(
   WebRuntimeFeatures::EnableCacheInlineScriptCode(
       base::FeatureList::IsEnabled(features::kCacheInlineScriptCode));
 
-  if (base::FeatureList::IsEnabled(features::kUnifiedTouchAdjustment))
-    WebRuntimeFeatures::EnableUnifiedTouchAdjustment(true);
-
   // Make srcset on link rel=preload work with SignedHTTPExchange flag too.
   if (base::FeatureList::IsEnabled(features::kSignedHTTPExchange))
     WebRuntimeFeatures::EnablePreloadImageSrcSetEnabled(true);

diff --git a/content/public/common/content_features.cc b/content/public/common/content_features.cc
@@ -409,11 +409,6 @@ const base::Feature kTopDocumentIsolation{"top-document-isolation",
 const base::Feature kTouchpadAndWheelScrollLatching{
     "TouchpadAndWheelScrollLatching", base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Enables unified touch adjustment which adjusts touch events target to a best
-// nearby node.
-const base::Feature kUnifiedTouchAdjustment{"UnifiedTouchAdjustment",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
-
 // Use Feature Policy to gate the use of permission features like midi,
 // geolocation, camera, microphone, etc.
 const base::Feature kUseFeaturePolicyForPermissions{

diff --git a/content/public/common/content_features.h b/content/public/common/content_features.h
@@ -98,7 +98,6 @@ CONTENT_EXPORT extern const base::Feature kTimerThrottlingForHiddenFrames;
 CONTENT_EXPORT extern const base::Feature kTopDocumentIsolation;
 CONTENT_EXPORT extern const base::Feature kTouchpadAndWheelScrollLatching;
 CONTENT_EXPORT extern const base::Feature kTouchpadOverscrollHistoryNavigation;
-CONTENT_EXPORT extern const base::Feature kUnifiedTouchAdjustment;
 CONTENT_EXPORT extern const base::Feature kUseFeaturePolicyForPermissions;
 CONTENT_EXPORT extern const base::Feature kUserActivationV2;
 CONTENT_EXPORT extern const base::Feature

diff --git a/third_party/WebKit/LayoutTests/fast/events/hit-test-counts-expected.txt b/third_party/WebKit/LayoutTests/fast/events/hit-test-counts-expected.txt
@@ -19,9 +19,9 @@ Wheel: 0+1
 TouchStart: 1+1
 TouchMove: 0+0
 TouchEnd: 0+0
-GestureTapDown: 1+2
-GestureShowPress: 1+2
-GestureTap: 1+4
+GestureTapDown: 0+1
+GestureShowPress: 0+1
+GestureTap: 0+3
 GestureScrollBegin: 0+1
 GestureTapCancel: 0+1
 GestureScrollUpdate: 0+0
@@ -38,9 +38,9 @@ Wheel: 0+1 0+1 0+1
 TouchStart: 2+0 2+0 1+1
 TouchMove: 0+0 0+0 0+0
 TouchEnd: 0+0 0+0 0+0
-GestureTapDown: 2+0 2+0 1+2
-GestureShowPress: 2+0 2+0 1+2
-GestureTap: 2+0 2+0 1+4
+GestureTapDown: 1+0 1+0 0+1
+GestureShowPress: 1+0 1+0 0+1
+GestureTap: 1+0 1+0 0+3
 GestureScrollBegin: 0+1 0+1 0+1
 GestureTapCancel: 1+0 1+0 0+1
 GestureScrollUpdate: 0+0 0+0 0+0
@@ -57,9 +57,9 @@ Wheel: 0+1 0+1 0+0
 TouchStart: 2+0 1+1 1+0
 TouchMove: 0+0 0+0 0+0
 TouchEnd: 0+0 0+0 0+0
-GestureTapDown: 2+0 1+1 1+0
-GestureShowPress: 2+0 1+1 1+0
-GestureTap: 2+0 1+3 1+0
+GestureTapDown: 1+0 0+1 0+0
+GestureShowPress: 1+0 0+1 0+0
+GestureTap: 1+0 0+3 0+0
 GestureScrollBegin: 0+1 0+1 0+0
 GestureTapCancel: 1+0 0+1 0+0
 GestureScrollUpdate: 0+0 0+0 0+0
@@ -76,9 +76,9 @@ Wheel: 0+1
 TouchStart: 1+1
 TouchMove: 0+0
 TouchEnd: 0+0
-GestureTapDown: 1+2
-GestureShowPress: 1+2
-GestureTap: 1+4 [with multiTargetTapNotification: 2+4]
+GestureTapDown: 0+1
+GestureShowPress: 0+1
+GestureTap: 0+3 [with multiTargetTapNotification: 1+3]
 GestureScrollBegin: 0+1
 GestureTapCancel: 0+1
 GestureScrollUpdate: 0+0

diff --git a/third_party/blink/public/platform/web_runtime_features.h b/third_party/blink/public/platform/web_runtime_features.h
@@ -186,7 +186,6 @@ class WebRuntimeFeatures {
   BLINK_PLATFORM_EXPORT static void EnableStopLoadingInBackground(bool);
   BLINK_PLATFORM_EXPORT static void EnableStopNonTimersInBackground(bool);
   BLINK_PLATFORM_EXPORT static void EnablePWAFullCodeCache(bool);
-  BLINK_PLATFORM_EXPORT static void EnableUnifiedTouchAdjustment(bool);
   BLINK_PLATFORM_EXPORT static void EnableMojoBlobURLs(bool);
   BLINK_PLATFORM_EXPORT static void EnableOffMainThreadWebSocket(bool);
   BLINK_PLATFORM_EXPORT static void EnableExperimentalProductivityFeatures(

diff --git a/third_party/blink/renderer/core/input/event_handler.cc b/third_party/blink/renderer/core/input/event_handler.cc
@@ -120,14 +120,6 @@ bool ShouldRefetchEventTarget(const MouseEventWithHitTestResults& mev) {
          IsHTMLInputElement(ToShadowRoot(target_node)->host());
 }
 
-enum class UMATouchAdjustmentNodeRelation {
-  kSameNode = 0,
-  kTouchStartNodeIsDescendantOfTapNode = 1,
-  kTapNodeIsDescendantOfTouchStartNode = 2,
-  kOthers = 3,
-  kTouchAdjustmentNodeRelationCount = 4
-};
-
 }  // namespace
 
 using namespace HTMLNames;
@@ -1687,13 +1679,16 @@ GestureEventWithHitTestResults EventHandler::HitTestResultForGestureEvent(
   LayoutSize padding;
 
   if (ShouldApplyTouchAdjustment(gesture_event)) {
-    padding = LayoutSize(adjusted_event.TapAreaInRootFrame());
-    if (!padding.IsEmpty()) {
-      padding.Scale(1.f / 2);
-      // Apply the same bound as touchstart event to adjustment padding
-      if (RuntimeEnabledFeatures::UnifiedTouchAdjustmentEnabled())
-        padding = GetHitTestRectForAdjustment(FlooredIntSize(padding));
-      hit_type |= HitTestRequest::kListBased;
+    // If gesture_event unique id matches the stored touch event result, do
+    // point-base hit test. Otherwise add padding and do rect-based hit test.
+    if (GestureCorrespondsToAdjustedTouch(gesture_event)) {
+      adjusted_event.ApplyTouchAdjustment(
+          touch_adjustment_result_.adjusted_point);
+    } else {
+      padding = GetHitTestRectForAdjustment(
+          LayoutSize(adjusted_event.TapAreaInRootFrame()) * 0.5f);
+      if (!padding.IsEmpty())
+        hit_type |= HitTestRequest::kListBased;
     }
   }
   LayoutPoint hit_test_point(frame_->View()->RootFrameToContents(
@@ -1737,37 +1732,6 @@ GestureEventWithHitTestResults EventHandler::HitTestResultForGestureEvent(
     UMA_HISTOGRAM_COUNTS_100("Event.Touch.TouchAdjustment.AdjustDistance",
                              static_cast<int>(adjusted_distance));
   }
-  if (GestureCorrespondsToAdjustedTouch(gesture_event)) {
-    LayoutPoint stored_adjusted_point(frame_->View()->RootFrameToContents(
-        touch_adjustment_result_.adjusted_point));
-    HitTestResult point_base_result = HitTestResultAtPoint(
-        stored_adjusted_point,
-        (hit_type | HitTestRequest::kReadOnly) & ~HitTestRequest::kListBased);
-    if (gesture_event.GetType() == WebInputEvent::kGestureTap) {
-      if (hit_test_result.InnerNode() && point_base_result.InnerNode()) {
-        UMATouchAdjustmentNodeRelation relation =
-            UMATouchAdjustmentNodeRelation::kOthers;
-        if (hit_test_result.InnerNode() == point_base_result.InnerNode()) {
-          relation = UMATouchAdjustmentNodeRelation::kSameNode;
-        } else if (point_base_result.InnerNode()->IsDescendantOf(
-                       hit_test_result.InnerNode())) {
-          relation = UMATouchAdjustmentNodeRelation::
-              kTouchStartNodeIsDescendantOfTapNode;
-        } else if (hit_test_result.InnerNode()->IsDescendantOf(
-                       point_base_result.InnerNode())) {
-          relation = UMATouchAdjustmentNodeRelation::
-              kTapNodeIsDescendantOfTouchStartNode;
-        }
-        UMA_HISTOGRAM_ENUMERATION(
-            "Event.Touch.TouchAdjustment.AdjustedNode", relation,
-            UMATouchAdjustmentNodeRelation::kTouchAdjustmentNodeRelationCount);
-      }
-    }
-    adjusted_event.ApplyTouchAdjustment(
-        touch_adjustment_result_.adjusted_point);
-    return GestureEventWithHitTestResults(adjusted_event, point_base_result);
-  }
-
   return GestureEventWithHitTestResults(adjusted_event, hit_test_result);
 }
 

diff --git a/third_party/blink/renderer/core/input/pointer_event_manager.cc b/third_party/blink/renderer/core/input/pointer_event_manager.cc
@@ -330,7 +330,7 @@ void PointerEventManager::AdjustTouchPointerEvent(
          WebPointerProperties::PointerType::kTouch);
 
   LayoutSize padding = GetHitTestRectForAdjustment(
-      IntSize(pointer_event.width / 2, pointer_event.height / 2));
+      LayoutSize(pointer_event.width, pointer_event.height) * 0.5f);
 
   if (padding.IsEmpty())
     return;

diff --git a/third_party/blink/renderer/core/page/touch_adjustment.cc b/third_party/blink/renderer/core/page/touch_adjustment.cc
@@ -531,10 +531,10 @@ bool FindBestContextMenuCandidate(Node*& target_node,
       subtargets, TouchAdjustment::HybridDistanceFunction);
 }
 
-LayoutSize GetHitTestRectForAdjustment(const IntSize& touch_area) {
+LayoutSize GetHitTestRectForAdjustment(const LayoutSize& touch_area) {
   const LayoutSize max_size(TouchAdjustment::kMaxAdjustmentRadiusDips,
                             TouchAdjustment::kMaxAdjustmentRadiusDips);
-  return LayoutSize(touch_area).ShrunkTo(max_size);
+  return touch_area.ShrunkTo(max_size);
 }
 
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/page/touch_adjustment.h b/third_party/blink/renderer/core/page/touch_adjustment.h
@@ -43,7 +43,7 @@ bool FindBestContextMenuCandidate(Node*& target_node,
                                   const IntRect& touch_area,
                                   const HeapVector<Member<Node>>&);
 
-LayoutSize GetHitTestRectForAdjustment(const IntSize& touch_area);
+LayoutSize GetHitTestRectForAdjustment(const LayoutSize& touch_area);
 
 struct TouchAdjustmentResult {
   uint32_t unique_event_id;

diff --git a/third_party/blink/renderer/platform/exported/web_runtime_features.cc b/third_party/blink/renderer/platform/exported/web_runtime_features.cc
@@ -504,10 +504,6 @@ void WebRuntimeFeatures::EnablePWAFullCodeCache(bool enable) {
   RuntimeEnabledFeatures::SetPWAFullCodeCacheEnabled(enable);
 }
 
-void WebRuntimeFeatures::EnableUnifiedTouchAdjustment(bool enable) {
-  RuntimeEnabledFeatures::SetUnifiedTouchAdjustmentEnabled(enable);
-}
-
 void WebRuntimeFeatures::EnableMojoBlobURLs(bool enable) {
   RuntimeEnabledFeatures::SetMojoBlobURLsEnabled(enable);
 }

diff --git a/third_party/blink/renderer/platform/geometry/layout_size.h b/third_party/blink/renderer/platform/geometry/layout_size.h
@@ -190,6 +190,10 @@ inline LayoutSize operator-(const LayoutSize& size) {
   return LayoutSize(-size.Width(), -size.Height());
 }
 
+inline LayoutSize operator*(const LayoutSize& a, const float scale) {
+  return LayoutSize(a.Width() * scale, a.Height() * scale);
+}
+
 inline bool operator==(const LayoutSize& a, const LayoutSize& b) {
   return a.Width() == b.Width() && a.Height() == b.Height();
 }

diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5
@@ -1260,7 +1260,7 @@
     },
     {
       name: "UnifiedTouchAdjustment",
-      status: "experimental",
+      status: "stable",
     },
     {
       name: "UserActivationV2",

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -23615,6 +23615,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 
 <histogram name="Event.Touch.TouchAdjustment.AdjustedNode"
     enum="TouchAdjustmentNodeRelation">
+  <obsolete>
+    Deprecated 03/2018 due to no longer needed.
+  </obsolete>
   <owner>eirage@chromium.org</owner>
   <summary>
     Records if doing touch adjustment on touchstart have same hit test node as