Bug 1454485 - Stop passing around the scroll view and container direction since it's already in the scrollbar data. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 17 Apr 2018 14:54:36 -0400
changeset 783815 2a44754d1c036cb97d07a8351e86734f67745d54
parent 783814 2d6e80c00582afb265d628d9bce26da52becd1f8
push id106789
push userkgupta@mozilla.com
push dateTue, 17 Apr 2018 18:54:58 +0000
reviewersbotond
bugs1454485
milestone61.0a1
Bug 1454485 - Stop passing around the scroll view and container direction since it's already in the scrollbar data. r?botond MozReview-Commit-ID: 3t4uLBQZSAi
gfx/layers/LayerAttributes.h
gfx/layers/LayerMetricsWrapper.h
gfx/layers/Layers.cpp
gfx/layers/Layers.h
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/HitTestingTreeNode.cpp
gfx/layers/apz/src/HitTestingTreeNode.h
gfx/layers/composite/AsyncCompositionManager.cpp
gfx/layers/wr/WebRenderScrollDataWrapper.h
--- a/gfx/layers/LayerAttributes.h
+++ b/gfx/layers/LayerAttributes.h
@@ -86,16 +86,20 @@ struct ScrollbarData {
            mThumbIsAsyncDraggable == aOther.mThumbIsAsyncDraggable &&
            mScrollTrackStart == aOther.mScrollTrackStart &&
            mScrollTrackLength == aOther.mScrollTrackLength &&
            mTargetViewId == aOther.mTargetViewId;
   }
   bool operator!=(const ScrollbarData& aOther) const {
     return !(*this == aOther);
   }
+
+  bool IsThumb() const {
+    return mScrollbarLayerType == ScrollbarLayerType::Thumb;
+  }
 };
 
 /**
  * Infrequently changing layer attributes that require no special
  * serialization work.
  */
 class SimpleLayerAttributes final
 {
@@ -272,30 +276,20 @@ public:
   float GetOpacity() const {
     return mOpacity;
   }
 
   bool IsFixedPosition() const {
     return mIsFixedPosition;
   }
 
-  FrameMetrics::ViewID GetScrollbarTargetViewId() const {
-    return mScrollbarData.mTargetViewId;
-  }
-
   const ScrollbarData& GetScrollbarData() const {
     return mScrollbarData;
   }
 
-  Maybe<ScrollDirection> GetScrollbarContainerDirection() const {
-    return (mScrollbarData.mScrollbarLayerType == ScrollbarLayerType::Container)
-      ? mScrollbarData.mDirection
-      : Nothing();
-  }
-
   gfx::CompositionOp GetMixBlendMode() const {
     return mMixBlendMode;
   }
 
   bool GetForceIsolatedGroup() const {
     return mForceIsolatedGroup;
   }
 
--- a/gfx/layers/LayerMetricsWrapper.h
+++ b/gfx/layers/LayerMetricsWrapper.h
@@ -421,29 +421,16 @@ public:
   {
     MOZ_ASSERT(IsValid());
     // This function is only really needed for template-compatibility with
     // WebRenderScrollDataWrapper. Although it will be called, the return
     // value is not used.
     return 0;
   }
 
-  FrameMetrics::ViewID GetScrollbarTargetContainerId() const
-  {
-    MOZ_ASSERT(IsValid());
-
-    return mLayer->GetScrollbarTargetViewId();
-  }
-
-  Maybe<ScrollDirection> GetScrollbarContainerDirection() const
-  {
-    MOZ_ASSERT(IsValid());
-    return mLayer->GetScrollbarContainerDirection();
-  }
-
   FrameMetrics::ViewID GetFixedPositionScrollContainerId() const
   {
     MOZ_ASSERT(IsValid());
 
     return mLayer->GetFixedPositionScrollContainerId();
   }
 
   bool IsBackfaceHidden() const
--- a/gfx/layers/Layers.cpp
+++ b/gfx/layers/Layers.cpp
@@ -614,16 +614,25 @@ Layer::GetLocalTransform()
 
 const LayerToParentLayerMatrix4x4
 Layer::GetLocalTransformTyped()
 {
   return ViewAs<LayerToParentLayerMatrix4x4>(GetLocalTransform());
 }
 
 bool
+Layer::IsScrollbarContainer() const
+{
+  const ScrollbarData& data = GetScrollbarData();
+  return (data.mScrollbarLayerType == ScrollbarLayerType::Container)
+      ? data.mDirection.isSome()
+      : false;
+}
+
+bool
 Layer::HasOpacityAnimation() const
 {
   return mAnimationInfo.HasOpacityAnimation();
 }
 
 bool
 Layer::HasTransformAnimation() const
 {
@@ -1808,25 +1817,27 @@ Layer::PrintInfo(std::stringstream& aStr
     aStream << " [extend3DContext]";
   }
   if (Combines3DTransformWithAncestors()) {
     aStream << " [combines3DTransformWithAncestors]";
   }
   if (Is3DContextLeaf()) {
     aStream << " [is3DContextLeaf]";
   }
-  if (GetScrollbarContainerDirection().isSome()) {
+  if (IsScrollbarContainer()) {
     aStream << " [scrollbar]";
   }
-  if (Maybe<ScrollDirection> thumbDirection = GetScrollbarData().mDirection) {
-    if (*thumbDirection == ScrollDirection::eVertical) {
-      aStream << nsPrintfCString(" [vscrollbar=%" PRIu64 "]", GetScrollbarTargetViewId()).get();
-    }
-    if (*thumbDirection == ScrollDirection::eHorizontal) {
-      aStream << nsPrintfCString(" [hscrollbar=%" PRIu64 "]", GetScrollbarTargetViewId()).get();
+  if (GetScrollbarData().IsThumb()) {
+    if (Maybe<ScrollDirection> thumbDirection = GetScrollbarData().mDirection) {
+      if (*thumbDirection == ScrollDirection::eVertical) {
+        aStream << nsPrintfCString(" [vscrollbar=%" PRIu64 "]", GetScrollbarData().mTargetViewId).get();
+      }
+      if (*thumbDirection == ScrollDirection::eHorizontal) {
+        aStream << nsPrintfCString(" [hscrollbar=%" PRIu64 "]", GetScrollbarData().mTargetViewId).get();
+      }
     }
   }
   if (GetIsFixedPosition()) {
     LayerPoint anchor = GetFixedPositionAnchor();
     aStream << nsPrintfCString(" [isFixedPosition scrollId=%" PRIu64 " sides=0x%x anchor=%s]",
                      GetFixedPositionScrollContainerId(),
                      GetFixedPositionSides(),
                      ToString(anchor).c_str()).get();
@@ -1966,17 +1977,17 @@ Layer::DumpPacket(layerscope::LayersPack
   layer->set_copaque(static_cast<bool>(GetContentFlags() & CONTENT_OPAQUE));
   // Component alpha
   layer->set_calpha(static_cast<bool>(GetContentFlags() & CONTENT_COMPONENT_ALPHA));
   // Vertical or horizontal bar
   if (GetScrollbarData().mScrollbarLayerType == layers::ScrollbarLayerType::Thumb) {
     layer->set_direct(*GetScrollbarData().mDirection == ScrollDirection::eVertical ?
                       LayersPacket::Layer::VERTICAL :
                       LayersPacket::Layer::HORIZONTAL);
-    layer->set_barid(GetScrollbarTargetViewId());
+    layer->set_barid(GetScrollbarData().mTargetViewId);
   }
 
   // Mask layer
   if (mMaskLayer) {
     layer->set_mask(reinterpret_cast<uint64_t>(mMaskLayer.get()));
   }
 
   // DisplayList log.
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -1323,20 +1323,18 @@ public:
   bool GetTransformIsPerspective() const { return mSimpleAttrs.GetTransformIsPerspective(); }
   bool GetIsStickyPosition() { return mSimpleAttrs.IsStickyPosition(); }
   FrameMetrics::ViewID GetFixedPositionScrollContainerId() { return mSimpleAttrs.GetFixedPositionScrollContainerId(); }
   LayerPoint GetFixedPositionAnchor() { return mSimpleAttrs.GetFixedPositionAnchor(); }
   int32_t GetFixedPositionSides() { return mSimpleAttrs.GetFixedPositionSides(); }
   FrameMetrics::ViewID GetStickyScrollContainerId() { return mSimpleAttrs.GetStickyScrollContainerId(); }
   const LayerRectAbsolute& GetStickyScrollRangeOuter() { return mSimpleAttrs.GetStickyScrollRangeOuter(); }
   const LayerRectAbsolute& GetStickyScrollRangeInner() { return mSimpleAttrs.GetStickyScrollRangeInner(); }
-  FrameMetrics::ViewID GetScrollbarTargetViewId() { return mSimpleAttrs.GetScrollbarTargetViewId(); }
   const ScrollbarData& GetScrollbarData() const { return mSimpleAttrs.GetScrollbarData(); }
-  bool IsScrollbarContainer() { return mSimpleAttrs.GetScrollbarContainerDirection().isSome(); }
-  Maybe<ScrollDirection> GetScrollbarContainerDirection() { return mSimpleAttrs.GetScrollbarContainerDirection(); }
+  bool IsScrollbarContainer() const;
   Layer* GetMaskLayer() const { return mMaskLayer; }
   bool HasPendingTransform() const { return mPendingTransform; }
 
   void CheckCanary() const { mCanary.Check(); }
 
   // Ancestor mask layers are associated with FrameMetrics, but for simplicity
   // in maintaining the layer tree structure we attach them to the layer.
   size_t GetAncestorMaskLayerCount() const {
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -873,18 +873,17 @@ APZCTreeManager::PrepareNodeForLayer(con
         GetEventRegions(aLayer),
         aLayer.GetVisibleRegion(),
         aLayer.GetTransformTyped(),
         (!parentHasPerspective && aLayer.GetClipRect())
           ? Some(ParentLayerIntRegion(*aLayer.GetClipRect()))
           : Nothing(),
         GetEventRegionsOverride(aParent, aLayer),
         aLayer.IsBackfaceHidden());
-    node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(),
-                           aLayer.GetScrollbarAnimationId(),
+    node->SetScrollbarData(aLayer.GetScrollbarAnimationId(),
                            aLayer.GetScrollbarData());
     node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId());
     return node;
   }
 
   AsyncPanZoomController* apzc = nullptr;
   // If we get here, aLayer is a scrollable layer and somebody
   // has registered a GeckoContentController for it, so we need to ensure
@@ -1093,18 +1092,17 @@ APZCTreeManager::PrepareNodeForLayer(con
         clipRegion,
         GetEventRegionsOverride(aParent, aLayer),
         aLayer.IsBackfaceHidden());
   }
 
   // Note: if layer properties must be propagated to nodes, RecvUpdate in
   // LayerTransactionParent.cpp must ensure that APZ will be notified
   // when those properties change.
-  node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(),
-                         aLayer.GetScrollbarAnimationId(),
+  node->SetScrollbarData(aLayer.GetScrollbarAnimationId(),
                          aLayer.GetScrollbarData());
   node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId());
   return node;
 }
 
 template<typename PanGestureOrScrollWheelInput>
 static bool
 WillHandleInput(const PanGestureOrScrollWheelInput& aPanInput)
--- a/gfx/layers/apz/src/HitTestingTreeNode.cpp
+++ b/gfx/layers/apz/src/HitTestingTreeNode.cpp
@@ -22,17 +22,16 @@ namespace layers {
 using gfx::CompositorHitTestInfo;
 
 HitTestingTreeNode::HitTestingTreeNode(AsyncPanZoomController* aApzc,
                                        bool aIsPrimaryHolder,
                                        LayersId aLayersId)
   : mApzc(aApzc)
   , mIsPrimaryApzcHolder(aIsPrimaryHolder)
   , mLayersId(aLayersId)
-  , mScrollViewId(FrameMetrics::NULL_SCROLL_ID)
   , mScrollbarAnimationId(0)
   , mFixedPosTarget(FrameMetrics::NULL_SCROLL_ID)
   , mIsBackfaceHidden(false)
   , mOverride(EventRegionsOverride::NoOverride)
 {
 if (mIsPrimaryApzcHolder) {
     MOZ_ASSERT(mApzc);
   }
@@ -89,31 +88,29 @@ HitTestingTreeNode::SetLastChild(HitTest
       // but it's better than nothing.
       MOZ_ASSERT(aChild->GetApzc() != parent);
       aChild->SetApzcParent(parent);
     }
   }
 }
 
 void
-HitTestingTreeNode::SetScrollbarData(FrameMetrics::ViewID aScrollViewId,
-                                     const uint64_t& aScrollbarAnimationId,
+HitTestingTreeNode::SetScrollbarData(const uint64_t& aScrollbarAnimationId,
                                      const ScrollbarData& aScrollbarData)
 {
-  mScrollViewId = aScrollViewId;
   mScrollbarAnimationId = aScrollbarAnimationId;
   mScrollbarData = aScrollbarData;
 }
 
 bool
 HitTestingTreeNode::MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetrics) const
 {
   return IsScrollThumbNode() &&
          mScrollbarData.mDirection == aDragMetrics.mDirection &&
-         mScrollViewId == aDragMetrics.mViewId;
+         mScrollbarData.mTargetViewId == aDragMetrics.mViewId;
 }
 
 bool
 HitTestingTreeNode::IsScrollThumbNode() const
 {
   return mScrollbarData.mScrollbarLayerType == layers::ScrollbarLayerType::Thumb;
 }
 
@@ -129,17 +126,17 @@ HitTestingTreeNode::GetScrollbarDirectio
   MOZ_ASSERT(IsScrollbarNode());
   MOZ_ASSERT(mScrollbarData.mDirection.isSome());
   return *mScrollbarData.mDirection;
 }
 
 FrameMetrics::ViewID
 HitTestingTreeNode::GetScrollTargetId() const
 {
-  return mScrollViewId;
+  return mScrollbarData.mTargetViewId;
 }
 
 const uint64_t&
 HitTestingTreeNode::GetScrollbarAnimationId() const
 {
   return mScrollbarAnimationId;
 }
 
--- a/gfx/layers/apz/src/HitTestingTreeNode.h
+++ b/gfx/layers/apz/src/HitTestingTreeNode.h
@@ -90,18 +90,17 @@ public:
                       const CSSTransformMatrix& aTransform,
                       const Maybe<ParentLayerIntRegion>& aClipRegion,
                       const EventRegionsOverride& aOverride,
                       bool aIsBackfaceHidden);
   bool IsOutsideClip(const ParentLayerPoint& aPoint) const;
 
   /* Scrollbar info */
 
-  void SetScrollbarData(FrameMetrics::ViewID aScrollViewId,
-                        const uint64_t& aScrollbarAnimationId,
+  void SetScrollbarData(const uint64_t& aScrollbarAnimationId,
                         const ScrollbarData& aScrollbarData);
   bool MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetrics) const;
   bool IsScrollbarNode() const;  // Scroll thumb or scrollbar container layer.
   // This can only be called if IsScrollbarNode() is true
   ScrollDirection GetScrollbarDirection() const;
   bool IsScrollThumbNode() const;  // Scroll thumb container layer.
   FrameMetrics::ViewID GetScrollTargetId() const;
   const ScrollbarData& GetScrollbarData() const;
@@ -135,20 +134,16 @@ private:
   RefPtr<HitTestingTreeNode> mPrevSibling;
   RefPtr<HitTestingTreeNode> mParent;
 
   RefPtr<AsyncPanZoomController> mApzc;
   bool mIsPrimaryApzcHolder;
 
   LayersId mLayersId;
 
-  // This is set for both scroll track and scroll thumb Container layers, and
-  // represents the scroll id of the scroll frame scrolled by the scrollbar.
-  FrameMetrics::ViewID mScrollViewId;
-
   // This is only set to non-zero if WebRender is enabled, and only for HTTNs
   // where IsScrollThumbNode() returns true. It holds the animation id that we
   // use to move the thumb node to reflect async scrolling.
   uint64_t mScrollbarAnimationId;
 
   // This is set for scrollbar Container and Thumb layers.
   ScrollbarData mScrollbarData;
 
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -750,17 +750,17 @@ MoveScrollbarForLayerMargin(Layer* aRoot
   // See bug 1223928 comment 9 - once we can detect the RCD with just the
   // isRootContent flag on the metrics, we can probably move this code into
   // ApplyAsyncTransformToScrollbar rather than having it as a separate
   // adjustment on the layer tree.
   Layer* scrollbar = BreadthFirstSearch<ReverseIterator>(aRoot,
     [aRootScrollId](Layer* aNode) {
       return (aNode->GetScrollbarData().mDirection.isSome() &&
               *aNode->GetScrollbarData().mDirection == ScrollDirection::eHorizontal &&
-              aNode->GetScrollbarTargetViewId() == aRootScrollId);
+              aNode->GetScrollbarData().mTargetViewId == aRootScrollId);
     });
   if (scrollbar) {
     // Shift the horizontal scrollbar down into the new space exposed by the
     // dynamic toolbar hiding. Technically we should also scale the vertical
     // scrollbar a bit to expand into the new space but it's not as noticeable
     // and it would add a lot more complexity, so we're going with the "it's not
     // worth it" justification.
     TranslateShadowLayer(scrollbar, ParentLayerPoint(0, -aFixedLayerMargins.bottom), true, nullptr);
@@ -1045,17 +1045,17 @@ AsyncCompositionManager::ApplyAsyncConte
 static bool
 LayerIsScrollbarTarget(const LayerMetricsWrapper& aTarget, Layer* aScrollbar)
 {
   if (!aTarget.GetApzc()) {
     return false;
   }
   const FrameMetrics& metrics = aTarget.Metrics();
   MOZ_ASSERT(metrics.IsScrollable());
-  if (metrics.GetScrollId() != aScrollbar->GetScrollbarTargetViewId()) {
+  if (metrics.GetScrollId() != aScrollbar->GetScrollbarData().mTargetViewId) {
     return false;
   }
   return !metrics.IsScrollInfoLayer();
 }
 
 static void
 ApplyAsyncTransformToScrollbarForContent(const RefPtr<APZSampler>& aSampler,
                                          Layer* aScrollbar,
--- a/gfx/layers/wr/WebRenderScrollDataWrapper.h
+++ b/gfx/layers/wr/WebRenderScrollDataWrapper.h
@@ -295,31 +295,16 @@ public:
   }
 
   uint64_t GetScrollbarAnimationId() const
   {
     MOZ_ASSERT(IsValid());
     return mLayer->GetScrollbarAnimationId();
   }
 
-  FrameMetrics::ViewID GetScrollbarTargetContainerId() const
-  {
-    MOZ_ASSERT(IsValid());
-    return mLayer->GetScrollbarData().mTargetViewId;
-  }
-
-  Maybe<ScrollDirection> GetScrollbarContainerDirection() const
-  {
-    MOZ_ASSERT(IsValid());
-    const ScrollbarData& data = mLayer->GetScrollbarData();
-    return (data.mScrollbarLayerType == ScrollbarLayerType::Container)
-        ? data.mDirection
-        : Nothing();
-  }
-
   FrameMetrics::ViewID GetFixedPositionScrollContainerId() const
   {
     MOZ_ASSERT(IsValid());
     return mLayer->GetFixedPositionScrollContainerId();
   }
 
   bool IsBackfaceHidden() const
   {