Bug 1453463 - Update existing code to be more efficient by using the new map. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 16 Apr 2018 09:33:08 -0400
changeset 467456 9a18481b44cdc5e2762631a4acc760dd54eb6a6a
parent 467455 6e9808db792e25fc1716f297c8cb2113331cac04
child 467457 c76fdea6986887d84c1d252634f2a68228e375f6
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1453463
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1453463 - Update existing code to be more efficient by using the new map. r=botond This updates existing bits of code (notably one of the GetTargetAPZC methods) to use the new map for more efficient lookups. Places that used GetTargetNode with a presshell-ignoring comparator can now use GetTargetAPZC as well. MozReview-Commit-ID: GFjO6KigVop
gfx/layers/apz/src/APZCTreeManager.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -2354,21 +2354,20 @@ GuidComparatorIgnoringPresShell(const Sc
   return aOne.mLayersId == aTwo.mLayersId
       && aOne.mScrollId == aTwo.mScrollId;
 }
 
 already_AddRefed<AsyncPanZoomController>
 APZCTreeManager::GetTargetAPZC(const LayersId& aLayersId,
                                const FrameMetrics::ViewID& aScrollId)
 {
-  RecursiveMutexAutoLock lock(mTreeLock);
+  MutexAutoLock lock(mMapLock);
   ScrollableLayerGuid guid(aLayersId, 0, aScrollId);
-  RefPtr<HitTestingTreeNode> node = GetTargetNode(guid, &GuidComparatorIgnoringPresShell);
-  MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
-  RefPtr<AsyncPanZoomController> apzc = node ? node->GetApzc() : nullptr;
+  auto it = mApzcMap.find(guid);
+  RefPtr<AsyncPanZoomController> apzc = (it != mApzcMap.end() ? it->second : nullptr);
   return apzc.forget();
 }
 
 already_AddRefed<HitTestingTreeNode>
 APZCTreeManager::GetTargetNode(const ScrollableLayerGuid& aGuid,
                                GuidComparator aComparator) const
 {
   mTreeLock.AssertCurrentThreadIn();
@@ -2440,21 +2439,17 @@ APZCTreeManager::GetAPZCAtPointWR(const 
   gfx::CompositorHitTestInfo hitInfo;
   bool hitSomething = wr->HitTest(wr::ToWorldPoint(aHitTestPoint),
       pipelineId, scrollId, hitInfo);
   if (!hitSomething) {
     return result.forget();
   }
 
   LayersId layersId = wr::AsLayersId(pipelineId);
-  RefPtr<HitTestingTreeNode> node = GetTargetNode(
-      ScrollableLayerGuid(layersId, 0, scrollId),
-      &GuidComparatorIgnoringPresShell);
-  MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
-  result = node ? node->GetApzc() : nullptr;
+  result = GetTargetAPZC(layersId, scrollId);
   if (!result) {
     // It falls back to the root
     // Re-enable these assertions once bug 1391318 is fixed. For now there are
     // race conditions with the WR hit-testing code that make these assertions
     // fail.
     //MOZ_ASSERT(scrollId == FrameMetrics::NULL_SCROLL_ID);
     result = FindRootApzcForLayersId(layersId);
     //MOZ_ASSERT(result);
@@ -2512,41 +2507,19 @@ APZCTreeManager::BuildOverscrollHandoffC
       }
       apzc = apzc->GetParent();
       continue;
     }
 
     // Guard against a possible infinite-loop condition. If we hit this, the
     // layout code that generates the handoff parents did something wrong.
     MOZ_ASSERT(apzc->GetScrollHandoffParentId() != apzc->GetGuid().mScrollId);
-
-    // Find the AsyncPanZoomController instance with a matching layersId and
-    // the scroll id that matches apzc->GetScrollHandoffParentId().
-    // As an optimization, we start by walking up the APZC tree from 'apzc'
-    // until we reach the top of the layer subtree for this layers id.
-    AsyncPanZoomController* scrollParent = nullptr;
-    AsyncPanZoomController* parent = apzc;
-    while (!parent->HasNoParentWithSameLayersId()) {
-      parent = parent->GetParent();
-      // While walking up to find the root of the subtree, if we encounter the
-      // handoff parent, we don't actually need to do the search so we can
-      // just abort here.
-      if (parent->GetGuid().mScrollId == apzc->GetScrollHandoffParentId()) {
-        scrollParent = parent;
-        break;
-      }
-    }
-    // If that heuristic didn't turn up the scroll parent, do a full tree search.
-    if (!scrollParent) {
-      ScrollableLayerGuid guid(parent->GetGuid().mLayersId, 0, apzc->GetScrollHandoffParentId());
-      RefPtr<HitTestingTreeNode> node = GetTargetNode(guid, &GuidComparatorIgnoringPresShell);
-      MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
-      scrollParent = node ? node->GetApzc() : nullptr;
-    }
-    apzc = scrollParent;
+    RefPtr<AsyncPanZoomController> scrollParent =
+        GetTargetAPZC(apzc->GetGuid().mLayersId, apzc->GetScrollHandoffParentId());
+    apzc = scrollParent.get();
   }
 
   // Now adjust the chain to account for scroll grabbing. Sorting is a bit
   // of an overkill here, but scroll grabbing will likely be generalized
   // to scroll priorities, so we might as well do it this way.
   result->SortByScrollPriority();
 
   // Print the overscroll chain for debugging.
@@ -2587,20 +2560,20 @@ APZCTreeManager::GetTargetApzcForNode(Hi
   for (const HitTestingTreeNode* n = aNode;
        n && n->GetLayersId() == aNode->GetLayersId();
        n = n->GetParent()) {
     if (n->GetApzc()) {
       APZCTM_LOG("Found target %p using ancestor lookup\n", n->GetApzc());
       return n->GetApzc();
     }
     if (n->GetFixedPosTarget() != FrameMetrics::NULL_SCROLL_ID) {
-      ScrollableLayerGuid guid(n->GetLayersId(), 0, n->GetFixedPosTarget());
-      RefPtr<HitTestingTreeNode> fpNode = GetTargetNode(guid, &GuidComparatorIgnoringPresShell);
-      APZCTM_LOG("Found target node %p using fixed-pos lookup on %" PRIu64 "\n", fpNode.get(), n->GetFixedPosTarget());
-      return fpNode ? fpNode->GetApzc() : nullptr;
+      RefPtr<AsyncPanZoomController> fpTarget =
+          GetTargetAPZC(n->GetLayersId(), n->GetFixedPosTarget());
+      APZCTM_LOG("Found target APZC %p using fixed-pos lookup on %" PRIu64 "\n", fpTarget.get(), n->GetFixedPosTarget());
+      return fpTarget.get();
     }
   }
   return nullptr;
 }
 
 AsyncPanZoomController*
 APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
                                 const ScreenPoint& aHitTestPoint,
@@ -2670,20 +2643,20 @@ APZCTreeManager::GetAPZCAtPoint(HitTesti
         if (n->GetScrollbarDirection() == ScrollDirection::eVertical) {
           *aOutHitResult |= CompositorHitTestInfo::eScrollbarVertical;
         }
 
         // If we hit a scrollbar, target the APZC for the content scrolled
         // by the scrollbar. (The scrollbar itself doesn't scroll with the
         // scrolled content, so it doesn't carry the scrolled content's
         // scroll metadata).
-        ScrollableLayerGuid guid(n->GetLayersId(), 0, n->GetScrollTargetId());
-        if (RefPtr<HitTestingTreeNode> scrollTarget = GetTargetNode(guid, &GuidComparatorIgnoringPresShell)) {
-          MOZ_ASSERT(scrollTarget->GetApzc());
-          return scrollTarget->GetApzc();
+        RefPtr<AsyncPanZoomController> scrollTarget =
+            GetTargetAPZC(n->GetLayersId(), n->GetScrollTargetId());
+        if (scrollTarget) {
+          return scrollTarget.get();
         }
       }
     }
 
     AsyncPanZoomController* result = GetTargetApzcForNode(resultNode);
     if (!result) {
       result = FindRootApzcForLayersId(resultNode->GetLayersId());
       MOZ_ASSERT(result);