Bug 1359808 - Don't do empty transactions for scroll updates if there are already pending transforms in the layer tree. r=mstange
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 12 Jul 2017 11:14:11 -0400
changeset 418599 b5294562d3f5e080e594bd8ad44157cf83445769
parent 418598 8af8abf2aa0d5064f7b4b5557007e125b3db42eb
child 418600 f9cbe3a1e13290a978a7db2c26f4fbaf26e55b83
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1359808
milestone56.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 1359808 - Don't do empty transactions for scroll updates if there are already pending transforms in the layer tree. r=mstange The pending transforms must have been computed using the older scroll offset values, which means that updating the scroll offsets without recomputing the transforms will make them wrong. If we do an empty transaction for the scroll offset updates, the transforms will not get computed. This patch catches this scenario and schedules a full paint instead of the empty transaction instead. The case where the scroll offset is modified *before* the transform is already handled by code in nsIFrame::TryUpdateTransformOnly. MozReview-Commit-ID: I5s5J7BS1ru
gfx/layers/Layers.cpp
gfx/layers/Layers.h
layout/generic/nsGfxScrollFrame.cpp
--- a/gfx/layers/Layers.cpp
+++ b/gfx/layers/Layers.cpp
@@ -2486,21 +2486,30 @@ LayerManager::ClearDisplayItemLayers()
 }
 
 /*static*/ bool
 LayerManager::IsLogEnabled()
 {
   return MOZ_LOG_TEST(GetLog(), LogLevel::Debug);
 }
 
-void
+bool
 LayerManager::SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
                                                        const ScrollUpdateInfo& aUpdateInfo)
 {
+  Layer* withPendingTransform = DepthFirstSearch<ForwardIterator>(GetRoot(),
+      [](Layer* aLayer) {
+        return aLayer->HasPendingTransform();
+      });
+  if (withPendingTransform) {
+    return false;
+  }
+
   mPendingScrollUpdates[aScrollId] = aUpdateInfo;
+  return true;
 }
 
 Maybe<ScrollUpdateInfo>
 LayerManager::GetPendingScrollInfoUpdate(FrameMetrics::ViewID aScrollId)
 {
   auto it = mPendingScrollUpdates.find(aScrollId);
   if (it != mPendingScrollUpdates.end()) {
     return Some(it->second);
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -797,17 +797,17 @@ private:
   FramesTimingRecording mRecording;
 
 public:
   /*
    * Methods to store/get/clear a "pending scroll info update" object on a
    * per-scrollid basis. This is used for empty transactions that push over
    * scroll position updates to the APZ code.
    */
-  void SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
+  bool SetPendingScrollUpdateForNextTransaction(FrameMetrics::ViewID aScrollId,
                                                 const ScrollUpdateInfo& aUpdateInfo);
   Maybe<ScrollUpdateInfo> GetPendingScrollInfoUpdate(FrameMetrics::ViewID aScrollId);
   void ClearPendingScrollInfoUpdate();
 private:
   std::map<FrameMetrics::ViewID,ScrollUpdateInfo> mPendingScrollUpdates;
 
   // Display items are only valid during this transaction.
   // At the end of the transaction, we have to go and clear out
@@ -1397,16 +1397,18 @@ public:
   int32_t GetFixedPositionSides() { return mSimpleAttrs.FixedPositionSides(); }
   FrameMetrics::ViewID GetStickyScrollContainerId() { return mSimpleAttrs.StickyScrollContainerId(); }
   const LayerRect& GetStickyScrollRangeOuter() { return mSimpleAttrs.StickyScrollRangeOuter(); }
   const LayerRect& GetStickyScrollRangeInner() { return mSimpleAttrs.StickyScrollRangeInner(); }
   FrameMetrics::ViewID GetScrollbarTargetContainerId() { return mSimpleAttrs.ScrollbarTargetContainerId(); }
   const ScrollThumbData& GetScrollThumbData() const { return mSimpleAttrs.ThumbData(); }
   bool IsScrollbarContainer() { return mSimpleAttrs.IsScrollbarContainer(); }
   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 {
     return mAncestorMaskLayers.Length();
   }
   Layer* GetAncestorMaskLayerAt(size_t aIndex) const {
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2868,27 +2868,31 @@ ScrollFrameHelper::ScrollToImpl(nsPoint 
         if (LastScrollOrigin() == nsGkAtoms::apz) {
           schedulePaint = false;
           PAINT_SKIP_LOG("Skipping due to APZ scroll\n");
         } else if (mScrollableByAPZ) {
           nsIWidget* widget = presContext->GetNearestWidget();
           LayerManager* manager = widget ? widget->GetLayerManager() : nullptr;
           if (manager) {
             mozilla::layers::FrameMetrics::ViewID id;
-            DebugOnly<bool> success = nsLayoutUtils::FindIDFor(content, &id);
+            bool success = nsLayoutUtils::FindIDFor(content, &id);
             MOZ_ASSERT(success); // we have a displayport, we better have an ID
 
             // Schedule an empty transaction to carry over the scroll offset update,
             // instead of a full transaction. This empty transaction might still get
             // squashed into a full transaction if something happens to trigger one.
-            schedulePaint = false;
-            manager->SetPendingScrollUpdateForNextTransaction(id,
+            success = manager->SetPendingScrollUpdateForNextTransaction(id,
                 { mScrollGeneration, CSSPoint::FromAppUnits(GetScrollPosition()) });
-            mOuter->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);
-            PAINT_SKIP_LOG("Skipping due to APZ-forwarded main-thread scroll\n");
+            if (success) {
+              schedulePaint = false;
+              mOuter->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);
+              PAINT_SKIP_LOG("Skipping due to APZ-forwarded main-thread scroll\n");
+            } else {
+              PAINT_SKIP_LOG("Failed to set pending scroll update on layer manager\n");
+            }
           }
         }
       }
     }
   }
 
   if (schedulePaint) {
     mOuter->SchedulePaint();