Bug 1451469 - Allow WR sampling without the tree lock. r=botond

For webrender, we need to be able to sample the async transforms from nodes
and thumbs without holding the tree lock, since the sampling happens on
the render backend thread, and holding the tree lock would be a
threading violation. We can use the mApzcMap to sample the node
transforms, but for the thumbs we need to introduce another data
structure. This data structure packages up all the information from
the HitTestingTreeNodes that we need for the computation and stores it
protected by the map lock. This allows us to compute the transforms
safely while just holding the map lock.

MozReview-Commit-ID: BDMEbE78NnH

--HG--
extra : rebase_source : 754ceca695b5b3e1c87b1b2bde753d1775107e5f

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -110,6 +110,17 @@ struct APZCTreeManager::TreeBuildingState {
                      ScrollableLayerGuid::HashIgnoringPresShellFn,
                      ScrollableLayerGuid::EqualIgnoringPresShellFn> mApzcMap;
 
+  // This is populated with all the HitTestingTreeNodes that are scroll thumbs
+  // and have a scrollthumb animation id (which indicates that they need to be
+  // sampled for WebRender on the sampler thread).
+  std::vector<HitTestingTreeNode*> mScrollThumbs;
+  // This is populated with all the scroll target nodes. We use in conjunction
+  // with mScrollThumbs to build APZCTreeManager::mScrollThumbInfo.
+  std::unordered_map<ScrollableLayerGuid,
+                     HitTestingTreeNode*,
+                     ScrollableLayerGuid::HashIgnoringPresShellFn,
+                     ScrollableLayerGuid::EqualIgnoringPresShellFn> mScrollTargets;
+
   // As the tree is traversed, the top element of this stack tracks whether
   // the parent scroll node has a perspective transform.
   std::stack<bool> mParentHasPerspective;
@@ -403,6 +414,19 @@ APZCTreeManager::UpdateHitTestingTreeImpl(LayersId aRootLayerTreeId,
           AsyncPanZoomController* apzc = node->GetApzc();
           aLayerMetrics.SetApzc(apzc);
 
+          // GetScrollbarAnimationId is only non-zero when webrender is enabled,
+          // which limits the extra thumb mapping work to the webrender-enabled
+          // case where it is needed.
+          // Note also that when webrender is enabled, a "valid" animation id
+          // is always nonzero, so we don't need to worry about handling the
+          // case where WR is enabled and the animation id is zero.
+          if (node->IsScrollThumbNode() && node->GetScrollbarAnimationId()) {
+            state.mScrollThumbs.push_back(node);
+          }
+          if (apzc && node->IsPrimaryHolder()) {
+            state.mScrollTargets[apzc->GetGuid()] = node;
+          }
+
           mApzcTreeLog << '\n';
 
           // Accumulate the CSS transform between layers that have an APZC.
@@ -484,6 +508,29 @@ APZCTreeManager::UpdateHitTestingTreeImpl(LayersId aRootLayerTreeId,
     // APZC instances
     MutexAutoLock lock(mMapLock);
     mApzcMap = Move(state.mApzcMap);
+    mScrollThumbInfo.clear();
+    // For non-webrender, state.mScrollThumbs will be empty so this will be a
+    // no-op.
+    for (HitTestingTreeNode* thumb : state.mScrollThumbs) {
+      MOZ_ASSERT(thumb->IsScrollThumbNode());
+      ScrollableLayerGuid targetGuid(thumb->GetLayersId(), 0, thumb->GetScrollTargetId());
+      auto it = state.mScrollTargets.find(targetGuid);
+      if (it == state.mScrollTargets.end()) {
+        // It could be that |thumb| is a scrollthumb for content which didn't
+        // have an APZC, for example if the content isn't layerized. Regardless,
+        // we can't async-scroll it so we don't need to worry about putting it
+        // in mScrollThumbInfo.
+        continue;
+      }
+      HitTestingTreeNode* target = it->second;
+      mScrollThumbInfo.emplace_back(
+          thumb->GetScrollbarAnimationId(),
+          thumb->GetTransform(),
+          thumb->GetScrollbarData(),
+          targetGuid,
+          target->GetTransform(),
+          target->IsAncestorOf(thumb));
+    }
   }
 
   for (size_t i = 0; i < state.mNodesToDestroy.Length(); i++) {
@@ -548,114 +595,57 @@ APZCTreeManager::PushStateToWR(wr::TransactionBuilder& aTxn,
                                const TimeStamp& aSampleTime)
 {
   AssertOnSamplerThread();
-
-  RecursiveMutexAutoLock lock(mTreeLock);
-
-  // During the first pass through the tree, we build a cache of guid->HTTN so
-  // that we can find the relevant APZC instances quickly in subsequent passes,
-  // such as the one below to generate scrollbar transforms. Without this, perf
-  // could end up being O(n^2) instead of O(n log n) because we'd have to search
-  // the tree to find the corresponding APZC every time we hit a thumb node.
-  // We use the presShell-ignoring map because when we do a lookup in the map
-  // for the scrollbar, we don't have (or care about) the presShellId.
-  std::unordered_map<ScrollableLayerGuid,
-                     HitTestingTreeNode*,
-                     ScrollableLayerGuid::HashIgnoringPresShellFn,
-                     ScrollableLayerGuid::EqualIgnoringPresShellFn> httnMap;
+  MutexAutoLock lock(mMapLock);
 
   bool activeAnimations = false;
-  LayersId lastLayersId{(uint64_t)-1};
-  wr::WrPipelineId lastPipelineId;
+  for (const auto& mapping : mApzcMap) {
+    AsyncPanZoomController* apzc = mapping.second;
+    ParentLayerPoint layerTranslation = apzc->GetCurrentAsyncTransform(
+        AsyncPanZoomController::eForCompositing).mTranslation;
 
-  // We iterate backwards here because the HitTestingTreeNode is optimized
-  // for backwards iteration. The equivalent code in AsyncCompositionManager
-  // iterates forwards, but the direction shouldn't really matter in practice
-  // so we do what's faster. In the future, if we need to start doing the
-  // equivalent of AlignFixedAndStickyLayers here, then the order will become
-  // important and we'll need to take that into consideration.
-  ForEachNode<ReverseIterator>(mRootNode.get(),
-      [&](HitTestingTreeNode* aNode)
-      {
-        if (!aNode->IsPrimaryHolder()) {
-          return;
-        }
-        AsyncPanZoomController* apzc = aNode->GetApzc();
-        MOZ_ASSERT(apzc);
+    // The positive translation means the painted content is supposed to
+    // move down (or to the right), and that corresponds to a reduction in
+    // the scroll offset. Since we are effectively giving WR the async
+    // scroll delta here, we want to negate the translation.
+    ParentLayerPoint asyncScrollDelta = -layerTranslation;
+    // XXX figure out what zoom-related conversions need to happen here.
+    aTxn.UpdateScrollPosition(
+        wr::AsPipelineId(apzc->GetGuid().mLayersId),
+        apzc->GetGuid().mScrollId,
+        wr::ToLayoutPoint(LayoutDevicePoint::FromUnknownPoint(asyncScrollDelta.ToUnknownPoint())));
 
-        if (aNode->GetLayersId() != lastLayersId) {
-          // If we walked into or out of a subtree, we need to get the new
-          // pipeline id.
-          RefPtr<WebRenderBridgeParent> wrBridge;
-          CompositorBridgeParent::CallWithIndirectShadowTree(aNode->GetLayersId(),
-              [&wrBridge](LayerTreeState& aState) -> void {
-                wrBridge = aState.mWrBridge;
-              });
-          if (!wrBridge) {
-            // During shutdown we might have layer tree information for stuff
-            // that has already been torn down. In that case just skip over
-            // those layers.
-            return;
-          }
-          lastPipelineId = wrBridge->PipelineId();
-          lastLayersId = aNode->GetLayersId();
-        }
+    apzc->ReportCheckerboard(aSampleTime);
+    activeAnimations |= apzc->AdvanceAnimations(aSampleTime);
+  }
 
-        httnMap.emplace(apzc->GetGuid(), aNode);
-
-        ParentLayerPoint layerTranslation = apzc->GetCurrentAsyncTransform(
-            AsyncPanZoomController::eForCompositing).mTranslation;
-        // The positive translation means the painted content is supposed to
-        // move down (or to the right), and that corresponds to a reduction in
-        // the scroll offset. Since we are effectively giving WR the async
-        // scroll delta here, we want to negate the translation.
-        ParentLayerPoint asyncScrollDelta = -layerTranslation;
-        // XXX figure out what zoom-related conversions need to happen here.
-        aTxn.UpdateScrollPosition(lastPipelineId, apzc->GetGuid().mScrollId,
-            wr::ToLayoutPoint(LayoutDevicePoint::FromUnknownPoint(asyncScrollDelta.ToUnknownPoint())));
-
-        apzc->ReportCheckerboard(aSampleTime);
-        activeAnimations |= apzc->AdvanceAnimations(aSampleTime);
-      });
-
-  // Now we iterate over the nodes again, and generate the transforms needed
-  // for scrollbar thumbs. Although we *could* do this as part of the previous
-  // iteration, it's cleaner and more efficient to do it as a separate pass
-  // because now we have a populated httnMap which allows O(log n) lookup here,
-  // resulting in O(n log n) runtime.
+  // Now collect all the async transforms needed for the scrollthumbs.
   nsTArray<wr::WrTransformProperty> scrollbarTransforms;
-  ForEachNode<ReverseIterator>(mRootNode.get(),
-      [&](HitTestingTreeNode* aNode)
-      {
-        if (!aNode->IsScrollThumbNode()) {
-          return;
-        }
-        ScrollableLayerGuid guid(aNode->GetLayersId(), 0, aNode->GetScrollTargetId());
-        auto it = httnMap.find(guid);
-        if (it == httnMap.end()) {
-          // A scrollbar for content which didn't have an APZC. Possibly the
-          // content isn't layerized. Regardless, we can't async-scroll it so
-          // we can skip the async transform on the scrollbar.
-          return;
-        }
-
-        HitTestingTreeNode* scrollTargetNode = it->second;
-        AsyncPanZoomController* scrollTargetApzc = scrollTargetNode->GetApzc();
-        MOZ_ASSERT(scrollTargetApzc);
-        LayerToParentLayerMatrix4x4 transform = scrollTargetApzc->CallWithLastContentPaintMetrics(
-            [&](const FrameMetrics& aMetrics) {
-                return ComputeTransformForScrollThumb(
-                    aNode->GetTransform() * AsyncTransformMatrix(),
-                    scrollTargetNode->GetTransform().ToUnknownMatrix(),
-                    scrollTargetApzc,
-                    aMetrics,
-                    aNode->GetScrollbarData(),
-                    scrollTargetNode->IsAncestorOf(aNode),
-                    nullptr);
-            });
-        scrollbarTransforms.AppendElement(wr::ToWrTransformProperty(
-            aNode->GetScrollbarAnimationId(),
-            transform));
-      });
+  for (const ScrollThumbInfo& info : mScrollThumbInfo) {
+    auto it = mApzcMap.find(info.mTargetGuid);
+    if (it == mApzcMap.end()) {
+      // It could be that |info| is a scrollthumb for content which didn't
+      // have an APZC, for example if the content isn't layerized. Regardless,
+      // we can't async-scroll it so we don't need to worry about putting it
+      // in mScrollThumbInfo.
+      continue;
+    }
+    AsyncPanZoomController* scrollTargetApzc = it->second;
+    MOZ_ASSERT(scrollTargetApzc);
+    LayerToParentLayerMatrix4x4 transform = scrollTargetApzc->CallWithLastContentPaintMetrics(
+        [&](const FrameMetrics& aMetrics) {
+            return ComputeTransformForScrollThumb(
+                info.mThumbTransform * AsyncTransformMatrix(),
+                info.mTargetTransform.ToUnknownMatrix(),
+                scrollTargetApzc,
+                aMetrics,
+                info.mThumbData,
+                info.mTargetIsAncestor,
+                nullptr);
+        });
+    scrollbarTransforms.AppendElement(wr::ToWrTransformProperty(
+        info.mThumbAnimationId,
+        transform));
+  }
   aTxn.AppendTransformProperties(scrollbarTransforms);
 
   return activeAnimations;

gfx/layers/apz/src/APZCTreeManager.h

@@ -75,6 +75,25 @@ struct ScrollThumbData;
  * The interpretation of the lock ordering is that if lock A precedes lock B
  * in the ordering sequence, then you must NOT wait on A while holding B.
  *
+ * In addition, the WR hit-testing codepath acquires the tree lock and then
+ * blocks on the render backend thread to do the hit-test. Similar operations
+ * elsewhere mean that we need to be careful with which threads are allowed
+ * to acquire which locks and the order they do so. At the time of this writing,
+ * https://bug1391318.bmoattachments.org/attachment.cgi?id=8965040 contains
+ * the most complete description we have of the situation. The total dependency
+ * ordering including both threads and locks is as follows:
+ *
+ * UI main thread
+ *  -> GPU main thread          // only if GPU enabled
+ *  -> Compositor thread
+ *  -> SceneBuilder thread      // only if WR enabled
+ *  -> APZ tree lock
+ *  -> RenderBackend thread     // only if WR enabled
+ *  -> APZC map lock
+ *  -> APZC instance lock
+ *  -> APZC test lock
+ *
+ * where the -> annotation means the same as described above.
  * **************************************************************************
  */
 
@@ -733,15 +752,57 @@ private:
   mutable mozilla::RecursiveMutex mTreeLock;
   RefPtr<HitTestingTreeNode> mRootNode;
 
-  /* A map for quick access to get APZC instances by guid, without having to
+  /** A lock that protects mApzcMap and mScrollThumbInfo. */
+  mutable mozilla::Mutex mMapLock;
+  /**
+   * A map for quick access to get APZC instances by guid, without having to
    * acquire the tree lock. mMapLock must be acquired while accessing or
    * modifying mApzcMap.
    */
-  mutable mozilla::Mutex mMapLock;
   std::unordered_map<ScrollableLayerGuid,
                      RefPtr<AsyncPanZoomController>,
                      ScrollableLayerGuid::HashIgnoringPresShellFn,
                      ScrollableLayerGuid::EqualIgnoringPresShellFn> mApzcMap;
+  /**
+   * A helper structure to store all the information needed to compute the
+   * async transform for a scrollthumb on the sampler thread.
+   */
+  struct ScrollThumbInfo {
+    uint64_t mThumbAnimationId;
+    CSSTransformMatrix mThumbTransform;
+    ScrollbarData mThumbData;
+    ScrollableLayerGuid mTargetGuid;
+    CSSTransformMatrix mTargetTransform;
+    bool mTargetIsAncestor;
+
+    ScrollThumbInfo(const uint64_t& aThumbAnimationId,
+                    const CSSTransformMatrix& aThumbTransform,
+                    const ScrollbarData& aThumbData,
+                    const ScrollableLayerGuid& aTargetGuid,
+                    const CSSTransformMatrix& aTargetTransform,
+                    bool aTargetIsAncestor)
+      : mThumbAnimationId(aThumbAnimationId)
+      , mThumbTransform(aThumbTransform)
+      , mThumbData(aThumbData)
+      , mTargetGuid(aTargetGuid)
+      , mTargetTransform(aTargetTransform)
+      , mTargetIsAncestor(aTargetIsAncestor)
+    {
+      MOZ_ASSERT(mTargetGuid.mScrollId == mThumbData.mTargetViewId);
+    }
+  };
+  /**
+   * If this APZCTreeManager is being used with WebRender, this vector gets
+   * populated during a layers update. It holds a package of information needed
+   * to compute and set the async transforms on scroll thumbs. This information
+   * is extracted from the HitTestingTreeNodes for the WebRender case because
+   * accessing the HitTestingTreeNodes requires holding the tree lock which
+   * we cannot do on the WR sampler thread. mScrollThumbInfo, however, can
+   * be accessed while just holding the mMapLock which is safe to do on the
+   * sampler thread.
+   * mMapLock must be acquired while accessing or modifying mScrollThumbInfo.
+   */
+  std::vector<ScrollThumbInfo> mScrollThumbInfo;
 
   /* Holds the zoom constraints for scrollable layers, as determined by the
    * the main-thread gecko code. This can only be accessed on the updater