Bug 1662013 - Replace ScrollUpdateInfo with ScrollPositionUpdate. r=botond

The existing ScrollUpdateInfo that is used to store a "paint-skipped scroll" for
empty transactions is very similar to the new ScrollPositionUpdate class, so
we can delete the former and use the latter instead.

The important part of this change is that when applying the pending info to
a FrameMetrics, we now also append the ScrollUpdateInfo to the mScrollUpdates
list on the ScrollMetadata. This aligns the code with the previous few patches,
where we duplicate the scroll information in both the regular FrameMetrics
fields and the ScrollMetadata::mScrollUpdates array.

Note that the existing code may have a defect when multiple paint-skip scrolls
occur in a single transaction; only the newest one is kept. In the case where
the last paint-skip is an absolute scroll, this is fine, but a relative scroll
may end up clobbering a previous absolute scroll. This patch explicitly detects
the scenario with multiple paint-skip scrolls and bails out to a full paint
transaction. As a followup it should be possible to relax this restriction by
storing an array of pending ScrollPositionUpdate instances.

Differential Revision: https://phabricator.services.mozilla.com/D88743

gfx/layers/FrameMetrics.h

@@ -40,17 +40,6 @@ struct ParamTraits;
 namespace mozilla {
 namespace layers {
 
-/**
- * Helper struct to hold a couple of fields that can be updated as part of
- * an empty transaction.
- */
-struct ScrollUpdateInfo {
-  uint32_t mScrollGeneration;
-  CSSPoint mLayoutScrollOffset;
-  CSSPoint mBaseScrollOffset;
-  bool mIsRelative;
-};
-
 /**
  * Metrics about a scroll frame that are sent to the compositor and used
  * by APZ.
@@ -330,12 +319,14 @@ struct FrameMetrics {
     mDoSmoothScroll = true;
   }
 
-  void UpdatePendingScrollInfo(const ScrollUpdateInfo& aInfo) {
-    SetLayoutScrollOffset(aInfo.mLayoutScrollOffset);
-    mBaseScrollOffset = aInfo.mBaseScrollOffset;
-    mScrollGeneration = aInfo.mScrollGeneration;
+  void UpdatePendingScrollInfo(const ScrollPositionUpdate& aInfo) {
+    SetLayoutScrollOffset(aInfo.GetDestination());
+    mScrollGeneration = aInfo.GetGeneration();
     mScrollUpdateType = ePending;
-    mIsRelative = aInfo.mIsRelative;
+    mIsRelative = aInfo.GetType() == ScrollUpdateType::Relative;
+    if (mIsRelative) {
+      mBaseScrollOffset = aInfo.GetSource();
+    }
   }
 
  public:
@@ -1041,6 +1032,13 @@ struct ScrollMetadata {
     mScrollUpdates = aUpdates;
   }
 
+  void UpdatePendingScrollInfo(const ScrollPositionUpdate& aInfo) {
+    mMetrics.UpdatePendingScrollInfo(aInfo);
+
+    mScrollUpdates.Clear();
+    mScrollUpdates.AppendElement(aInfo);
+  }
+
  private:
   FrameMetrics mMetrics;
 
@@ -1131,7 +1129,8 @@ struct ScrollMetadata {
   // Please add new fields above this comment.
 };
 
-typedef nsDataHashtable<ScrollableLayerGuid::ViewIDHashKey, ScrollUpdateInfo>
+typedef nsDataHashtable<ScrollableLayerGuid::ViewIDHashKey,
+                        ScrollPositionUpdate>
     ScrollUpdatesMap;
 
 }  // namespace layers

gfx/layers/Layers.cpp

@@ -640,10 +640,10 @@ void Layer::ApplyPendingUpdatesForThisTransaction() {
   for (size_t i = 0; i < mScrollMetadata.Length(); i++) {
     FrameMetrics& fm = mScrollMetadata[i].GetMetrics();
     ScrollableLayerGuid::ViewID scrollId = fm.GetScrollId();
-    Maybe<ScrollUpdateInfo> update =
+    Maybe<ScrollPositionUpdate> update =
         Manager()->GetPendingScrollInfoUpdate(scrollId);
     if (update) {
-      fm.UpdatePendingScrollInfo(update.value());
+      mScrollMetadata[i].UpdatePendingScrollInfo(update.value());
       Mutated();
     }
   }
@@ -2261,18 +2261,23 @@ bool LayerManager::IsLogEnabled() {
 
 bool LayerManager::SetPendingScrollUpdateForNextTransaction(
     ScrollableLayerGuid::ViewID aScrollId,
-    const ScrollUpdateInfo& aUpdateInfo) {
+    const ScrollPositionUpdate& aUpdateInfo) {
   Layer* withPendingTransform = DepthFirstSearch<ForwardIterator>(
       GetRoot(), [](Layer* aLayer) { return aLayer->HasPendingTransform(); });
   if (withPendingTransform) {
     return false;
   }
 
+  // XXX We should store a list of ScrollPositionUpdates here rather
+  // than bailing out if we get multiple scroll updates for the same scrollid.
+  if (mPendingScrollUpdates.Lookup(aScrollId)) {
+    return false;
+  }
   mPendingScrollUpdates.Put(aScrollId, aUpdateInfo);
   return true;
 }
 
-Maybe<ScrollUpdateInfo> LayerManager::GetPendingScrollInfoUpdate(
+Maybe<ScrollPositionUpdate> LayerManager::GetPendingScrollInfoUpdate(
     ScrollableLayerGuid::ViewID aScrollId) {
   auto p = mPendingScrollUpdates.Lookup(aScrollId);
   return p ? Some(p.Data()) : Nothing();

gfx/layers/Layers.h

@@ -815,8 +815,8 @@ class LayerManager : public FrameRecorder {
    */
   virtual bool SetPendingScrollUpdateForNextTransaction(
       ScrollableLayerGuid::ViewID aScrollId,
-      const ScrollUpdateInfo& aUpdateInfo);
-  Maybe<ScrollUpdateInfo> GetPendingScrollInfoUpdate(
+      const ScrollPositionUpdate& aUpdateInfo);
+  Maybe<ScrollPositionUpdate> GetPendingScrollInfoUpdate(
       ScrollableLayerGuid::ViewID aScrollId);
   std::unordered_set<ScrollableLayerGuid::ViewID>
   ClearPendingScrollInfoUpdate();

gfx/layers/ipc/LayersMessageUtils.h

@@ -878,10 +878,6 @@ struct ParamTraits<mozilla::layers::SimpleLayerAttributes> {
   }
 };
 
-template <>
-struct ParamTraits<mozilla::layers::ScrollUpdateInfo>
-    : public PlainOldDataSerializer<mozilla::layers::ScrollUpdateInfo> {};
-
 template <>
 struct ParamTraits<mozilla::layers::CompositionPayloadType>
     : public ContiguousEnumSerializerInclusive<

gfx/layers/wr/RenderRootTypes.cpp

@@ -65,7 +65,7 @@ bool ReadScrollUpdates(const IPC::Message* aMsg, PickleIterator* aIter,
   layers::ScrollUpdatesMap map(count);
   for (size_t i = 0; i < count; ++i) {
     layers::ScrollableLayerGuid::ViewID key;
-    layers::ScrollUpdateInfo data;
+    mozilla::ScrollPositionUpdate data;
     if (!ReadIPDLParam(aMsg, aIter, aActor, &key) ||
         !ReadIPDLParam(aMsg, aIter, aActor, &data)) {
       return false;

gfx/layers/wr/WebRenderScrollData.cpp

@@ -215,7 +215,7 @@ void WebRenderScrollData::ApplyUpdates(ScrollUpdatesMap& aUpdates,
                                        uint32_t aPaintSequenceNumber) {
   for (auto it = aUpdates.Iter(); !it.Done(); it.Next()) {
     if (Maybe<size_t> index = HasMetadataFor(it.Key())) {
-      mScrollMetadatas[*index].GetMetrics().UpdatePendingScrollInfo(it.Data());
+      mScrollMetadatas[*index].UpdatePendingScrollInfo(it.Data());
     }
   }
   mPaintSequenceNumber = aPaintSequenceNumber;
@@ -310,4 +310,4 @@ bool ParamTraits<mozilla::layers::WebRenderScrollData>::Read(
          aResult->RepopulateMap();
 }
 
-}  // namespace IPC
+}  // namespace IPC

layout/generic/nsGfxScrollFrame.cpp

@@ -3118,11 +3118,9 @@ void ScrollFrameHelper::ScrollToImpl(nsPoint aPt, const nsRect& aRange,
             // update, instead of a full transaction. This empty transaction
             // might still get squashed into a full transaction if something
             // happens to trigger one.
+            MOZ_ASSERT(!mScrollUpdates.IsEmpty());
             success = manager->SetPendingScrollUpdateForNextTransaction(
-                id,
-                {mScrollGeneration, CSSPoint::FromAppUnits(GetScrollPosition()),
-                 CSSPoint::FromAppUnits(GetApzScrollPosition()),
-                 mLastScrollOrigin == ScrollOrigin::Relative});
+                id, mScrollUpdates.LastElement());
             if (success) {
               schedulePaint = false;
               mOuter->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);