2012-02-18 Antonio Gomes <agomes@rim.com>

        Fat fingers - cache the first rect-based hit test so we do not need to do it again
        https://bugs.webkit.org/show_bug.cgi?id=79115

        Reviewed by Adam Treat.

        Our FatFingers implementation runs currently in two phases:
        the first checks for the elements intrinsically clickable;
        the second checks for elements made clickable by the page
        (for example, a div with a onclick event listener attached to it).
        For each phase, we perform a rect hittest, which is not needed since
        the result of each is the same.

        Patch introduces a caching mechanism so we avoid on rect hittest:
        when the first phase runs, it caches each nodeset per document in
        a hashmap. This second phase works with the cached results.

        No behavioral change, but performance is better since we
        avoid one (possibly expensive) rect hittest.

        I measured the performance gain on https://www.kvd.se/, and we
        save up to 0.04 seconds, by caching and re-using the results.

        * WebKitSupport/FatFingers.cpp:
        (BlackBerry::WebKit::dumpHitTestResult):
        (BlackBerry::WebKit::FatFingers::findBestPoint):
        (BlackBerry::WebKit::FatFingers::findIntersectingRegions):
        (BlackBerry::WebKit::FatFingers::cachingStrategy):
        (WebKit):
        (BlackBerry::WebKit::FatFingers::getNodesFromRect):
        * WebKitSupport/FatFingers.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108380 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebKit/blackberry/ChangeLog b/Source/WebKit/blackberry/ChangeLog
index ed7dafd..9d6b63f 100644 (file)
--- a/Source/WebKit/blackberry/ChangeLog
+++ b/Source/WebKit/blackberry/ChangeLog
@@ -1,3 +1,36 @@
+2012-02-18  Antonio Gomes  <agomes@rim.com>
+
+        Fat fingers - cache the first rect-based hit test so we do not need to do it again
+        https://bugs.webkit.org/show_bug.cgi?id=79115
+
+        Reviewed by Adam Treat.
+
+        Our FatFingers implementation runs currently in two phases:
+        the first checks for the elements intrinsically clickable;
+        the second checks for elements made clickable by the page
+        (for example, a div with a onclick event listener attached to it).
+        For each phase, we perform a rect hittest, which is not needed since
+        the result of each is the same.
+
+        Patch introduces a caching mechanism so we avoid on rect hittest:
+        when the first phase runs, it caches each nodeset per document in
+        a hashmap. This second phase works with the cached results.
+
+        No behavioral change, but performance is better since we
+        avoid one (possibly expensive) rect hittest.
+
+        I measured the performance gain on https://www.kvd.se/, and we
+        save up to 0.04 seconds, by caching and re-using the results.
+
+        * WebKitSupport/FatFingers.cpp:
+        (BlackBerry::WebKit::dumpHitTestResult):
+        (BlackBerry::WebKit::FatFingers::findBestPoint):
+        (BlackBerry::WebKit::FatFingers::findIntersectingRegions):
+        (BlackBerry::WebKit::FatFingers::cachingStrategy):
+        (WebKit):
+        (BlackBerry::WebKit::FatFingers::getNodesFromRect):
+        * WebKitSupport/FatFingers.h:
+
 2012-02-20  Antonio Gomes  <agomes@rim.com>
 
         All default video/audio control elements should be rect-hit testable (Part II)

diff --git a/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp b/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp
index d67578b..016a426 100644 (file)
--- a/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp
+++ b/Source/WebKit/blackberry/WebKitSupport/FatFingers.cpp
@@ -170,9 +170,10 @@ FatFingers::~FatFingers()
 const FatFingersResult FatFingers::findBestPoint()
 {
     ASSERT(m_webPage);
+    ASSERT(m_webPage->m_mainFrame);
+
+    m_cachedRectHitTestResults.clear();
 
-    if (!m_webPage->m_mainFrame)
-        return m_contentPos;
     FatFingersResult result(m_contentPos);
     m_matchingApproach = ClickableByDefault;
 
@@ -216,6 +217,7 @@ const FatFingersResult FatFingers::findBestPoint()
     }
 
     m_matchingApproach = Done;
+    m_cachedRectHitTestResults.clear();
 
     if (!foundOne)
         return result;
@@ -311,16 +313,15 @@ bool FatFingers::findIntersectingRegions(Document* document,
         m_debugFatFingerRect = m_webPage->mapToTransformed(m_webPage->mapFromContentsToViewport(fingerRect));
 #endif
 
-    // Document::nodesFromRect needs viewport coordinates, but FatFinger::nodesFromRect gets contents coordinates
-    // for simplicity instead.
     bool foundOne = false;
-    HitTestResult result = nodesFromRect(document, frameContentPos);
 
     RenderLayer* lowestPositionedEnclosingLayerSoFar = 0;
 
     // Iterate over the list of nodes (and subrects of nodes where possible), for each saving the
     // intersection of the bounding box with the finger rect.
-    ListHashSet<RefPtr<Node> > intersectedNodes = result.rectBasedTestResult();
+    ListHashSet<RefPtr<Node> > intersectedNodes;
+    getNodesFromRect(document, frameContentPos, intersectedNodes);
+
     ListHashSet<RefPtr<Node> >::const_iterator it = intersectedNodes.begin();
     ListHashSet<RefPtr<Node> >::const_iterator end = intersectedNodes.end();
     for ( ; it != end; ++it) {
@@ -461,8 +462,32 @@ void FatFingers::getPaddings(unsigned& top, unsigned& right, unsigned& bottom, u
     left = leftPadding / currentScale;
 }
 
-HitTestResult FatFingers::nodesFromRect(Document* document, const IntPoint& contentPos) const
+FatFingers::CachedResultsStrategy FatFingers::cachingStrategy() const
+{
+    switch (m_matchingApproach) {
+    case ClickableElement:
+        return GetFromRenderTree;
+    case MadeClickableByTheWebpage:
+        return GetFromCache;
+    case Done:
+    default:
+        ASSERT_NOT_REACHED();
+        return GetFromRenderTree;
+    }
+}
+
+void FatFingers::getNodesFromRect(Document* document, const IntPoint& contentPos, ListHashSet<RefPtr<WebCore::Node> >& intersectedNodes)
 {
+    FatFingers::CachedResultsStrategy cacheResolvingStrategy = cachingStrategy();
+
+    if (cacheResolvingStrategy == GetFromCache) {
+        ASSERT(m_cachedRectHitTestResults.contains(document));
+        intersectedNodes = m_cachedRectHitTestResults.get(document);
+        return;
+    }
+
+    ASSERT(cacheResolvingStrategy == GetFromRenderTree);
+
     unsigned topPadding, rightPadding, bottomPadding, leftPadding;
     getPaddings(topPadding, rightPadding, bottomPadding, leftPadding);
 
@@ -470,7 +495,8 @@ HitTestResult FatFingers::nodesFromRect(Document* document, const IntPoint& cont
     HitTestResult result(contentPos, topPadding, rightPadding, bottomPadding, leftPadding, HitTestShadowDOM);
 
     document->renderView()->layer()->hitTest(request, result);
-    return result;
+    intersectedNodes = result.rectBasedTestResult();
+    m_cachedRectHitTestResults.add(document, intersectedNodes);
 }
 
 void FatFingers::getRelevantInfoFromPoint(Document* document, const IntPoint& contentPos, Element*& elementUnderPoint, Element*& clickableElementUnderPoint) const

diff --git a/Source/WebKit/blackberry/WebKitSupport/FatFingers.h b/Source/WebKit/blackberry/WebKitSupport/FatFingers.h
index c17d7b4..6b847e1 100644 (file)
--- a/Source/WebKit/blackberry/WebKitSupport/FatFingers.h
+++ b/Source/WebKit/blackberry/WebKitSupport/FatFingers.h
@@ -25,6 +25,9 @@
 #include <BlackBerryPlatformIntRectRegion.h>
 
 #include <utility>
+
+#include <wtf/HashSet.h>
+#include <wtf/ListHashSet.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -113,6 +116,10 @@ private:
 
     typedef std::pair<WebCore::Node*, Platform::IntRectRegion> IntersectingRegion;
 
+    enum CachedResultsStrategy { GetFromRenderTree = 0, GetFromCache };
+    CachedResultsStrategy cachingStrategy() const;
+    typedef HashMap<RefPtr<WebCore::Document>, ListHashSet<RefPtr<WebCore::Node> > > CachedRectHitTestResults;
+
     bool checkFingerIntersection(const Platform::IntRectRegion&,
                                  const Platform::IntRectRegion& remainingFingerRegion,
                                  WebCore::Node*,
@@ -133,8 +140,7 @@ private:
 
     void setSuccessfulFatFingersResult(FatFingersResult&, WebCore::Node*, const WebCore::IntPoint&);
 
-    // It mimics Document::nodesFromRect, but has a different return value to fit our needs.
-    WebCore::HitTestResult nodesFromRect(WebCore::Document*, const WebCore::IntPoint&) const;
+    void getNodesFromRect(WebCore::Document*, const WebCore::IntPoint&, ListHashSet<RefPtr<WebCore::Node> >&);
 
     // It mimics Document::elementFromPoint, but recursively hit-tests in case an inner frame is found.
     void getRelevantInfoFromPoint(WebCore::Document*,
@@ -151,6 +157,7 @@ private:
     WebCore::IntPoint m_contentPos;
     TargetType m_targetType;
     MatchingApproachForClickable m_matchingApproach;
+    CachedRectHitTestResults m_cachedRectHitTestResults;
 };
 
 }