Bug 1547435 - Ensure NotifyApzTransaction is only called once a transaction is actually sent. r=kats,rhunt
authorBotond Ballo <botond@mozilla.com>
Thu, 02 May 2019 16:42:09 +0000
changeset 531143 85c60519f4c93d2eead5cb2963be5649d6d5d87b
parent 531142 cc36119ab2b4ce55fae2a6b2be5ec04734040591
child 531144 a6ad4039d785f53692c799110dae82ca82a2d279
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, rhunt
bugs1547435
milestone68.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 1547435 - Ensure NotifyApzTransaction is only called once a transaction is actually sent. r=kats,rhunt Differential Revision: https://phabricator.services.mozilla.com/D29283
gfx/layers/Layers.cpp
gfx/layers/Layers.h
gfx/layers/apz/test/mochitest/helper_scrollby_bug1531796.html
gfx/layers/basic/BasicLayerManager.cpp
gfx/layers/client/ClientLayerManager.cpp
gfx/layers/wr/WebRenderLayerManager.cpp
--- a/gfx/layers/Layers.cpp
+++ b/gfx/layers/Layers.cpp
@@ -242,24 +242,24 @@ AsyncPanZoomController* Layer::GetAsyncP
 #endif
   return mApzcs[aIndex];
 }
 
 void Layer::ScrollMetadataChanged() {
   mApzcs.SetLength(GetScrollMetadataCount());
 }
 
-void Layer::ApplyPendingUpdatesToSubtree() {
+std::unordered_set<ScrollableLayerGuid::ViewID>
+Layer::ApplyPendingUpdatesToSubtree() {
   ForEachNode<ForwardIterator>(this, [](Layer* layer) {
     layer->ApplyPendingUpdatesForThisTransaction();
   });
-
   // Once we're done recursing through the whole tree, clear the pending
   // updates from the manager.
-  Manager()->ClearPendingScrollInfoUpdate();
+  return Manager()->ClearPendingScrollInfoUpdate();
 }
 
 bool Layer::IsOpaqueForVisibility() {
   return GetEffectiveOpacity() == 1.0f &&
          GetEffectiveMixBlendMode() == CompositionOp::OP_OVER;
 }
 
 bool Layer::CanUseOpaqueSurface() {
@@ -616,17 +616,16 @@ void Layer::ApplyPendingUpdatesForThisTr
 
   for (size_t i = 0; i < mScrollMetadata.Length(); i++) {
     FrameMetrics& fm = mScrollMetadata[i].GetMetrics();
     ScrollableLayerGuid::ViewID scrollId = fm.GetScrollId();
     Maybe<ScrollUpdateInfo> update =
         Manager()->GetPendingScrollInfoUpdate(scrollId);
     if (update) {
       fm.UpdatePendingScrollInfo(update.value());
-      nsLayoutUtils::NotifyPaintSkipTransaction(scrollId);
       Mutated();
     }
   }
 }
 
 float Layer::GetLocalOpacity() {
   float opacity = mSimpleAttrs.GetOpacity();
   if (HostLayer* shadow = AsHostLayer()) opacity = shadow->GetShadowOpacity();
@@ -2259,20 +2258,27 @@ Maybe<ScrollUpdateInfo> LayerManager::Ge
   MOZ_ASSERT(GetBackendType() != LayersBackend::LAYERS_WR);
   auto it = mPendingScrollUpdates[wr::RenderRoot::Default].find(aScrollId);
   if (it != mPendingScrollUpdates[wr::RenderRoot::Default].end()) {
     return Some(it->second);
   }
   return Nothing();
 }
 
-void LayerManager::ClearPendingScrollInfoUpdate() {
+std::unordered_set<ScrollableLayerGuid::ViewID>
+LayerManager::ClearPendingScrollInfoUpdate() {
+  std::unordered_set<ScrollableLayerGuid::ViewID> scrollIds;
   for (auto renderRoot : wr::kRenderRoots) {
-    mPendingScrollUpdates[renderRoot].clear();
+    auto& updates = mPendingScrollUpdates[renderRoot];
+    for (const auto& update : updates) {
+      scrollIds.insert(update.first);
+    }
+    updates.clear();
   }
+  return scrollIds;
 }
 
 void PrintInfo(std::stringstream& aStream, HostLayer* aLayerComposite) {
   if (!aLayerComposite) {
     return;
   }
   if (const Maybe<ParentLayerIntRect>& clipRect =
           aLayerComposite->GetShadowClipRect()) {
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef GFX_LAYERS_H
 #define GFX_LAYERS_H
 
 #include <map>
+#include <unordered_set>
 #include <stdint.h>        // for uint32_t, uint64_t, uint8_t
 #include <stdio.h>         // for FILE
 #include <sys/types.h>     // for int32_t
 #include "FrameMetrics.h"  // for FrameMetrics
 #include "Units.h"         // for LayerMargin, LayerPoint, ParentLayerIntRect
 #include "gfxContext.h"
 #include "gfxTypes.h"
 #include "gfxPoint.h"  // for gfxPoint
@@ -789,17 +790,18 @@ class LayerManager : public FrameRecorde
    * per-scrollid basis. This is used for empty transactions that push over
    * scroll position updates to the APZ code.
    */
   virtual bool SetPendingScrollUpdateForNextTransaction(
       ScrollableLayerGuid::ViewID aScrollId,
       const ScrollUpdateInfo& aUpdateInfo, wr::RenderRoot aRenderRoot);
   Maybe<ScrollUpdateInfo> GetPendingScrollInfoUpdate(
       ScrollableLayerGuid::ViewID aScrollId);
-  void ClearPendingScrollInfoUpdate();
+  std::unordered_set<ScrollableLayerGuid::ViewID>
+  ClearPendingScrollInfoUpdate();
 
  protected:
   wr::RenderRootArray<ScrollUpdatesMap> mPendingScrollUpdates;
 };
 
 /**
  * A Layer represents anything that can be rendered onto a destination
  * surface.
@@ -1478,18 +1480,21 @@ class Layer {
    */
   float GetLocalOpacity();
 
   /**
    * DRAWING PHASE ONLY
    *
    * Apply pending changes to layers before drawing them, if those
    * pending changes haven't been overridden by later changes.
+   *
+   * Returns a list of scroll ids which had pending updates.
    */
-  void ApplyPendingUpdatesToSubtree();
+  std::unordered_set<ScrollableLayerGuid::ViewID>
+  ApplyPendingUpdatesToSubtree();
 
   /**
    * DRAWING PHASE ONLY
    *
    * Write layer-subtype-specific attributes into aAttrs.  Used to
    * synchronize layer attributes to their shadows'.
    */
   virtual void FillSpecificAttributes(SpecificLayerAttributes& aAttrs) {}
--- a/gfx/layers/apz/test/mochitest/helper_scrollby_bug1531796.html
+++ b/gfx/layers/apz/test/mochitest/helper_scrollby_bug1531796.html
@@ -5,21 +5,16 @@
   <meta name="viewport" content="width=device-width; initial-scale=1.0">
   <title>Test that scrollBy() doesn't scroll more than it should</title>
   <script type="application/javascript" src="apz_test_native_event_utils.js"></script>
   <script type="application/javascript" src="apz_test_utils.js"></script>
   <script src="/tests/SimpleTest/paint_listener.js"></script>
   <script type="application/javascript">
 
 function* test(testDriver) {
-  if (getPlatform() == "android") {
-    ok(true, "Skipping test on Android (bug 1547435)");
-    return;
-  }
-
   const maxSteps = 20;
   let scrollPerStep = 40;
   for (let step = 0; step < maxSteps; step++) {
     window.scrollBy(0, scrollPerStep);
     window.requestAnimationFrame(testDriver);
     yield;
   }
   is(window.scrollY, maxSteps * scrollPerStep, "Scrolled by the expected amount");
--- a/gfx/layers/basic/BasicLayerManager.cpp
+++ b/gfx/layers/basic/BasicLayerManager.cpp
@@ -36,16 +36,17 @@
 #include "mozilla/gfx/Matrix.h"     // for Matrix
 #include "mozilla/gfx/PathHelpers.h"
 #include "mozilla/gfx/Rect.h"            // for IntRect, Rect
 #include "mozilla/layers/LayersTypes.h"  // for BufferMode::BUFFER_NONE, etc
 #include "mozilla/mozalloc.h"            // for operator new
 #include "nsCOMPtr.h"                    // for already_AddRefed
 #include "nsDebug.h"                     // for NS_ASSERTION, etc
 #include "nsISupportsImpl.h"             // for gfxContext::Release, etc
+#include "nsLayoutUtils.h"               // for nsLayoutUtils
 #include "nsPoint.h"                     // for nsIntPoint
 #include "nsRect.h"                      // for mozilla::gfx::IntRect
 #include "nsRegion.h"                    // for nsIntRegion, etc
 #include "nsTArray.h"                    // for AutoTArray
 #include "TreeTraversal.h"               // for ForEachNode
 
 class nsIWidget;
 
@@ -559,21 +560,23 @@ bool BasicLayerManager::EndTransactionIn
   mPhase = PHASE_DRAWING;
 
   SetCompositionTime(TimeStamp::Now());
 
   RenderTraceLayers(mRoot, "FF00");
 
   mTransactionIncomplete = false;
 
+  std::unordered_set<ScrollableLayerGuid::ViewID> scrollIdsUpdated;
+
   if (mRoot) {
     if (aFlags & END_NO_COMPOSITE) {
       // Apply pending tree updates before recomputing effective
       // properties.
-      mRoot->ApplyPendingUpdatesToSubtree();
+      scrollIdsUpdated = mRoot->ApplyPendingUpdatesToSubtree();
     }
 
     // Need to do this before we call ApplyDoubleBuffering,
     // which depends on correct effective transforms
     if (mTarget) {
       mSnapEffectiveTransforms =
           !mTarget->GetDrawTarget()->GetUserData(&sDisablePixelSnapping);
     } else {
@@ -619,16 +622,24 @@ bool BasicLayerManager::EndTransactionIn
       // Clear out target if we have a complete transaction.
       mTarget = nullptr;
     }
   }
 
   if (mRoot) {
     mAnimationReadyTime = TimeStamp::Now();
     mRoot->StartPendingAnimations(mAnimationReadyTime);
+
+    // Once we're sure we're not going to fall back to a full paint,
+    // notify the scroll frames which had pending updates.
+    if (!mTransactionIncomplete) {
+      for (ScrollableLayerGuid::ViewID scrollId : scrollIdsUpdated) {
+        nsLayoutUtils::NotifyPaintSkipTransaction(scrollId);
+      }
+    }
   }
 
 #ifdef MOZ_LAYERS_HAVE_LOG
   Log();
   MOZ_LAYERS_LOG(("]----- EndTransaction"));
 #endif
 
   // Go back to the construction phase if the transaction isn't complete.
--- a/gfx/layers/client/ClientLayerManager.cpp
+++ b/gfx/layers/client/ClientLayerManager.cpp
@@ -296,17 +296,17 @@ bool ClientLayerManager::EndTransactionI
 
   ClientLayer* root = ClientLayer::ToClientLayer(GetRoot());
 
   mTransactionIncomplete = false;
   mQueuedAsyncPaints = false;
 
   // Apply pending tree updates before recomputing effective
   // properties.
-  GetRoot()->ApplyPendingUpdatesToSubtree();
+  auto scrollIdsUpdated = GetRoot()->ApplyPendingUpdatesToSubtree();
 
   mPaintedLayerCallback = aCallback;
   mPaintedLayerCallbackData = aCallbackData;
 
   GetRoot()->ComputeEffectiveTransforms(Matrix4x4());
 
   // Skip the painting if the device is in device-reset status.
   if (!gfxPlatform::GetPlatform()->DidRenderingDeviceReset()) {
@@ -316,16 +316,24 @@ bool ClientLayerManager::EndTransactionI
       mLastPaintTime = TimeStamp::Now() - start;
     } else {
       root->RenderLayer();
     }
   } else {
     gfxCriticalNote << "LayerManager::EndTransaction skip RenderLayer().";
   }
 
+  // Once we're sure we're not going to fall back to a full paint,
+  // notify the scroll frames which had pending updates.
+  if (!mTransactionIncomplete) {
+    for (ScrollableLayerGuid::ViewID scrollId : scrollIdsUpdated) {
+      nsLayoutUtils::NotifyPaintSkipTransaction(scrollId);
+    }
+  }
+
   if (!mRepeatTransaction && !GetRoot()->GetInvalidRegion().IsEmpty()) {
     GetRoot()->Mutated();
   }
 
   if (!mIsRepeatTransaction) {
     mAnimationReadyTime = TimeStamp::Now();
     GetRoot()->StartPendingAnimations(mAnimationReadyTime);
   }
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -364,17 +364,20 @@ void WebRenderLayerManager::EndTransacti
       scrollData.SetPaintSequenceNumber(mPaintSequenceNumber);
     }
   }
   mIsFirstPaint = false;
 
   // Since we're sending a full mScrollData that will include the new scroll
   // offsets, and we can throw away the pending scroll updates we had kept for
   // an empty transaction.
-  ClearPendingScrollInfoUpdate();
+  auto scrollIdsUpdated = ClearPendingScrollInfoUpdate();
+  for (ScrollableLayerGuid::ViewID update : scrollIdsUpdated) {
+    nsLayoutUtils::NotifyPaintSkipTransaction(update);
+  }
 
   mLatestTransactionId =
       mTransactionIdAllocator->GetTransactionId(/*aThrottle*/ true);
 
   // Get the time of when the refresh driver start its tick (if available),
   // otherwise use the time of when LayerManager::BeginTransaction was called.
   TimeStamp refreshStart = mTransactionIdAllocator->GetTransactionStart();
   if (!refreshStart) {