Enable HitTestCache by default.

UMA metrics indicate we have great validity. Enable the hit test cache
in release by default. Keep the validity checks around in ASSERTs
are enabled.

BUG=398920

Review URL: https://codereview.chromium.org/1294913004

git-svn-id: svn://svn.chromium.org/blink/trunk@200852 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/fast/events/hit-test-counts-expected.txt

@@ -16,14 +16,14 @@ MouseMove: 1+0
 MouseDown: 1+2
 MouseUp: 0+1
 Wheel: 0+1
-TouchStart: 1+0
+TouchStart: 0+1
 TouchMove: 0+0
 TouchEnd: 0+0
 GestureTapDown: 1+1
 GestureShowPress: 1+1
 GestureTap: 1+3 [with multiTargetTapNotification: 1+4]
 GestureScrollBegin: 0+1
-GestureTapCancel: 1+0
+GestureTapCancel: 0+1
 GestureScrollUpdate: 0+0
 GestureScrollEnd: 0+0
 
@@ -34,14 +34,14 @@ MouseMove: 1+0 1+0 1+0
 MouseDown: 1+1 1+1 1+1
 MouseUp: 0+1 0+1 0+1
 Wheel: 0+1 0+1 0+1
-TouchStart: 1+0 1+0 1+0
+TouchStart: 1+0 1+0 0+1
 TouchMove: 0+0 0+0 0+0
 TouchEnd: 0+0 0+0 0+0
 GestureTapDown: 1+0 1+0 1+1
 GestureShowPress: 1+0 1+0 1+1
 GestureTap: 1+0 1+0 1+3
 GestureScrollBegin: 0+1 0+1 0+1
-GestureTapCancel: 1+0 1+0 1+0
+GestureTapCancel: 1+0 1+0 0+1
 GestureScrollUpdate: 0+0 0+0 0+0
 GestureScrollEnd: 0+0 0+0 0+0
 
@@ -52,7 +52,7 @@ MouseMove: 1+0 1+0 0+0
 MouseDown: 1+1 1+1 0+0
 MouseUp: 0+1 0+1 0+0
 Wheel: 0+1 0+1 0+0
-TouchStart: 1+0 1+0 0+0
+TouchStart: 1+0 0+1 0+0
 TouchMove: 0+0 0+0 0+0
 TouchEnd: 0+0 0+0 0+0
 GestureTapDown: 1+0 1+0 1+0
@@ -70,14 +70,14 @@ MouseMove: 1+0
 MouseDown: 1+2
 MouseUp: 0+1
 Wheel: 0+1
-TouchStart: 1+0
+TouchStart: 0+1
 TouchMove: 0+0
 TouchEnd: 0+0
 GestureTapDown: 1+1
 GestureShowPress: 1+1
 GestureTap: 1+3 [with multiTargetTapNotification: 2+4]
 GestureScrollBegin: 0+1
-GestureTapCancel: 1+0
+GestureTapCancel: 0+1
 GestureScrollUpdate: 0+0
 GestureScrollEnd: 0+0
 

third_party/WebKit/Source/core/layout/HitTestCache.cpp

@@ -33,21 +33,6 @@ bool HitTestCache::lookupCachedResult(HitTestResult& hitResult, uint64_t domTree
     return result;
 }
 
-void HitTestCache::verifyCachedResult(const HitTestResult& expected, const HitTestResult& actual)
-{
-    ValidityHistogramMetric metric;
-    if (!actual.equalForCacheability(expected)) {
-        metric = expected.hitTestLocation().isRectBasedTest() ? ValidityHistogramMetric::INCORRECT_RECT_BASED_EXACT_MATCH : ValidityHistogramMetric::INCORRECT_POINT_EXACT_MATCH;
-        Platform::current()->histogramSparse("Event.HitTestValidityScore", static_cast<int>(actual.equalityScore(expected)));
-
-        // ASSERT that the cache hit is the same as the actual result.
-        ASSERT_NOT_REACHED();
-    } else {
-        metric = ValidityHistogramMetric::VALID_EXACT_MATCH;
-    }
-    Platform::current()->histogramEnumeration("Event.HitTestValidity", static_cast<int>(metric), static_cast<int>(ValidityHistogramMetric::MAX_VALIDITY_METRIC));
-}
-
 void HitTestCache::addCachedResult(const HitTestResult& result, uint64_t domTreeVersion)
 {
     if (!result.isCacheable())

third_party/WebKit/Source/core/layout/HitTestCache.h

@@ -46,10 +46,6 @@ public:
 
     void clear();
 
-    // Verify that the |actual| object matches the |expected| object; and
-    // log UMA metrics indicating the result.
-    static void verifyCachedResult(const HitTestResult& expected, const HitTestResult& actual);
-
     // Adds a HitTestResult to the cache.
     void addCachedResult(const HitTestResult&, uint64_t domTreeVersion);
 
@@ -76,19 +72,6 @@ private:
         MAX_HIT_METRIC = HIT_REGION_MATCH,
     };
 
-    // These values are reported in UMA as the "EventHitTestValidity"
-    // enumeration. Do not reorder, append new values at the end,
-    // deprecate old values and update histograms.xml.
-    enum class ValidityHistogramMetric {
-        VALID_EXACT_MATCH, // Correct node for exact point test.
-        VALID_REGION, // Correct node for region check.
-        INCORRECT_RECT_BASED_EXACT_MATCH, // Wrong node returned for cache hit with point was exact match and rect based test.
-        INCORRECT_POINT_EXACT_MATCH, // Wrong node returned for cache hit with exact point match and was explicit point test.
-        INCORRECT_RECT_BASED_REGION, // Wrong node returned for rect with region matching and was rect based test.
-        INCORRECT_POINT_REGION, // Wrong node returned for point with region matching and was explicit point test.
-        MAX_VALIDITY_METRIC = INCORRECT_POINT_REGION,
-    };
-
     unsigned m_updateIndex;
     WillBeHeapVector<HitTestResult, HIT_TEST_CACHE_SIZE> m_items;
     uint64_t m_domTreeVersion;

third_party/WebKit/Source/core/layout/HitTestResult.cpp

@@ -111,18 +111,6 @@ HitTestResult& HitTestResult::operator=(const HitTestResult& other)
     return *this;
 }
 
-unsigned HitTestResult::equalityScore(const HitTestResult& other) const
-{
-    return (m_hitTestRequest.equalForCacheability(other.m_hitTestRequest) << 7)
-        | ((m_innerNode == other.innerNode()) << 6)
-        | ((m_innerPossiblyPseudoNode == other.innerPossiblyPseudoNode()) << 5)
-        | ((m_pointInInnerNodeFrame == other.m_pointInInnerNodeFrame) << 4)
-        | ((m_localPoint == other.localPoint()) << 3)
-        | ((m_innerURLElement == other.URLElement()) << 2)
-        | ((m_scrollbar == other.scrollbar()) << 1)
-        | (m_isOverWidget == other.isOverWidget());
-}
-
 bool HitTestResult::equalForCacheability(const HitTestResult& other) const
 {
     return m_hitTestRequest.equalForCacheability(other.m_hitTestRequest)

third_party/WebKit/Source/core/layout/HitTestResult.h

@@ -66,7 +66,6 @@ public:
     DECLARE_TRACE();
 
     bool equalForCacheability(const HitTestResult&) const;
-    unsigned equalityScore(const HitTestResult&) const;
     void cacheValues(const HitTestResult&);
 
     // Populate this object based on another HitTestResult; similar to assignment operator

third_party/WebKit/Source/core/layout/LayoutView.cpp

@@ -102,24 +102,25 @@ bool LayoutView::hitTestNoLifecycleUpdate(HitTestResult& result)
 
     uint64_t domTreeVersion = document().domTreeVersion();
     HitTestResult cacheResult = result;
-    bool cacheHit = m_hitTestCache->lookupCachedResult(cacheResult, domTreeVersion);
-    bool hitLayer = layer()->hitTest(result);
+    bool hitLayer = false;
+    if (m_hitTestCache->lookupCachedResult(cacheResult, domTreeVersion)) {
+        m_hitTestCacheHits++;
+        hitLayer = true;
+        result = cacheResult;
+    } else {
+        hitLayer = layer()->hitTest(result);
 
-    // FrameView scrollbars are not the same as Layer scrollbars tested by Layer::hitTestOverflowControls,
-    // so we need to test FrameView scrollbars separately here. Note that it's important we do this after
-    // the hit test above, because that may overwrite the entire HitTestResult when it finds a hit.
-    IntPoint framePoint = frameView()->contentsToFrame(result.hitTestLocation().roundedPoint());
-    if (Scrollbar* frameScrollbar = frameView()->scrollbarAtFramePoint(framePoint))
-        result.setScrollbar(frameScrollbar);
+        // FrameView scrollbars are not the same as Layer scrollbars tested by Layer::hitTestOverflowControls,
+        // so we need to test FrameView scrollbars separately here. Note that it's important we do this after
+        // the hit test above, because that may overwrite the entire HitTestResult when it finds a hit.
+        IntPoint framePoint = frameView()->contentsToFrame(result.hitTestLocation().roundedPoint());
+        if (Scrollbar* frameScrollbar = frameView()->scrollbarAtFramePoint(framePoint))
+            result.setScrollbar(frameScrollbar);
 
-    if (cacheHit) {
-        m_hitTestCacheHits++;
-        m_hitTestCache->verifyCachedResult(result, cacheResult);
+        if (hitLayer)
+            m_hitTestCache->addCachedResult(result, domTreeVersion);
     }
 
-    if (hitLayer)
-        m_hitTestCache->addCachedResult(result, domTreeVersion);
-
     TRACE_EVENT_END1("blink,devtools.timeline", "HitTest", "endData", InspectorHitTestEvent::endData(result.hitTestRequest(), result.hitTestLocation(), result));
     return hitLayer;
 }