Backed out changeset 88c02f86a668 (bug 1783681) for causing Gtest failures CLOSED TREE

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -267,11 +267,11 @@ APZCTreeManager::APZCTreeManager(LayersId aRootLayersId,
                                  UniquePtr<IAPZHitTester> aHitTester)
     : mTestSampleTime(Nothing(), "APZCTreeManager::mTestSampleTime"),
       mInputQueue(new InputQueue()),
-      mMapLock("APZCMapLock"),
       mRootLayersId(aRootLayersId),
       mSampler(nullptr),
       mUpdater(nullptr),
       mTreeLock("APZCTreeLock"),
+      mMapLock("APZCMapLock"),
       mRetainedTouchIdentifier(-1),
       mInScrollbarTouchDrag(false),
       mCurrentMousePosition(ScreenPoint(),
@@ -748,7 +748,7 @@ void APZCTreeManager::SampleForWebRender(const Maybe<VsyncId>& aVsyncId,
         wrBridgeParent = aState.mWrBridge;
       });
 
-  const bool activeAnimations = AdvanceAnimationsInternal(lock, aSampleTime);
+  bool activeAnimations = AdvanceAnimationsInternal(lock, aSampleTime);
   if (activeAnimations && controller) {
     controller->ScheduleRenderOnCompositorThread(
         wr::RenderReasons::ANIMATED_PROPERTY);

gfx/layers/apz/src/APZCTreeManager.h

@@ -520,9 +520,6 @@ class APZCTreeManager : public IAPZCTreeManager, public APZInputBridge {
   APZSampler* GetSampler() const;
   APZUpdater* GetUpdater() const;
 
-  bool AdvanceAnimationsInternal(const MutexAutoLock& aProofOfMapLock,
-                                 const SampleTime& aSampleTime);
-
   // We need to allow APZUpdater to lock and unlock this tree during a WR
   // scene swap. We do this using private helpers to avoid exposing these
   // functions to the world.
@@ -791,6 +788,9 @@ class APZCTreeManager : public IAPZCTreeManager, public APZInputBridge {
   static already_AddRefed<GeckoContentController> GetContentController(
       LayersId aLayersId);
 
+  bool AdvanceAnimationsInternal(const MutexAutoLock& aProofOfMapLock,
+                                 const SampleTime& aSampleTime);
+
   using ClippedCompositionBoundsMap =
       std::unordered_map<ScrollableLayerGuid, ParentLayerRect,
                          ScrollableLayerGuid::HashIgnoringPresShellFn,
@@ -813,11 +813,6 @@ class APZCTreeManager : public IAPZCTreeManager, public APZInputBridge {
    */
   RefPtr<InputQueue> mInputQueue;
 
-  /** A lock that protects mApzcMap, mScrollThumbInfo, mRootScrollbarInfo,
-   * mFixedPositionInfo, and mStickyPositionInfo.
-   */
-  mutable mozilla::Mutex mMapLock;
-
  private:
   /* Layers id for the root CompositorBridgeParent that owns this
    * APZCTreeManager. */
@@ -858,6 +853,11 @@ class APZCTreeManager : public IAPZCTreeManager, public APZInputBridge {
   std::unordered_set<LayersId, LayersId::HashFn> mDetachedLayersIds
       MOZ_GUARDED_BY(mTreeLock);
 
+  /** A lock that protects mApzcMap, mScrollThumbInfo, mRootScrollbarInfo,
+   * mFixedPositionInfo, and mStickyPositionInfo.
+   */
+  mutable mozilla::Mutex mMapLock;
+
   /**
    * Helper structure to store a bunch of things in mApzcMap so that they can
    * be used from the sampler thread.

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -4697,10 +4697,8 @@ bool AsyncPanZoomController::UpdateAnimation(
   // composite multiple times at the same timestamp.
   // However we only want to do one animation step per composition so we need
   // to deduplicate these calls first.
-  // Even if there's no animation, if we have a scroll offset change pending due
-  // to the frame delay, we need to keep compositing.
   if (mLastSampleTime == aSampleTime) {
-    return !!mAnimation || HavePendingFrameDelayedOffset();
+    return !!mAnimation;
   }
 
   // We're at a new timestamp, so advance to the next sample in the deque, if
@@ -5021,17 +5019,6 @@ void AsyncPanZoomController::AdvanceToNextSample() {
   }
 }
 
-bool AsyncPanZoomController::HavePendingFrameDelayedOffset() const {
-  AssertOnSamplerThread();
-  RecursiveMutexAutoLock lock(mRecursiveMutex);
-
-  const bool nextFrameWillChange =
-      mSampledState.size() >= 2 && mSampledState[0] != mSampledState[1];
-  const bool frameAfterThatWillChange =
-      mSampledState.back() != SampledAPZCState(Metrics());
-  return nextFrameWillChange || frameAfterThatWillChange;
-}
-
 bool AsyncPanZoomController::SampleCompositedAsyncTransform(
     const RecursiveMutexAutoLock& aProofOfLock) {
   MOZ_ASSERT(mSampledState.size() <= 2);

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -1299,13 +1299,6 @@ class AsyncPanZoomController {
    */
   void AdvanceToNextSample();
 
-  /**
-   * Returns whether we have changes to the scroll offsets which need to be
-   * sampled in the next couple of frames (it depends on how many offsets we
-   * have, currently it's two).
-   */
-  bool HavePendingFrameDelayedOffset() const;
-
   /**
    * Samples the composited async transform, storing the result into
    * mSampledState. This will make the result of

gfx/layers/apz/test/gtest/APZTestCommon.h

@@ -248,11 +248,6 @@ class TestAPZCTreeManager : public APZCTreeManager {
    **/
   void CancelAnimation() { EXPECT_TRUE(false); }
 
-  bool AdvanceAnimations(const SampleTime& aSampleTime) {
-    MutexAutoLock lock(mMapLock);
-    return AdvanceAnimationsInternal(lock, aSampleTime);
-  }
-
   APZEventResult ReceiveInputEvent(
       InputData& aEvent,
       InputBlockCallback&& aCallback = InputBlockCallback()) override {

gfx/layers/apz/test/gtest/TestTreeManager.cpp

@@ -345,60 +345,3 @@ TEST_F(APZCTreeManagerTester, Bug1805601) {
   EXPECT_EQ(CSSRect(0, 0, 300, 300), compositorMetrics.CalculateScrollRange());
   EXPECT_EQ(CSSPoint(300, 300), compositorMetrics.GetVisualScrollOffset());
 }
-
-TEST_F(APZCTreeManagerTester,
-       InstantKeyScrollBetweenTwoSamplingsWithSameTimeStamp) {
-  // For instant scrolling, i.e. no async animation should not be involved.
-  SCOPED_GFX_PREF_BOOL("general.smoothScroll", false);
-
-  // Set up a keyboard shortcuts map to scroll page down.
-  AutoTArray<KeyboardShortcut, 1> shortcuts{KeyboardShortcut(
-      KeyboardInput::KEY_DOWN, 0, 0, 0, 0,
-      KeyboardScrollAction(
-          KeyboardScrollAction::KeyboardScrollActionType::eScrollPage, true))};
-  KeyboardMap keyboardMap(std::move(shortcuts));
-  manager->SetKeyboardMap(keyboardMap);
-
-  // Set up a scrollable layer.
-  CreateSimpleScrollingLayer();
-  ScopedLayerTreeRegistration registration(LayersId{0}, mcc);
-  UpdateHitTestingTree();
-
-  // Setup the scrollable layer is scrollable by key events.
-  FocusTarget focusTarget;
-  focusTarget.mSequenceNumber = 1;
-  focusTarget.mData = AsVariant<FocusTarget::ScrollTargets>(
-      {ScrollableLayerGuid::START_SCROLL_ID,
-       ScrollableLayerGuid::START_SCROLL_ID});
-  manager->UpdateFocusState(LayersId{0}, LayersId{0}, focusTarget);
-
-  // A vsync tick happens.
-  mcc->AdvanceByMillis(16);
-
-  // The first sampling happens, there's no change have happened, thus no need
-  // to composite.
-  EXPECT_FALSE(manager->AdvanceAnimations(mcc->GetSampleTime()));
-
-  // A key event causing scroll page down happens.
-  WidgetKeyboardEvent widgetEvent(true, eKeyDown, nullptr);
-  KeyboardInput input(widgetEvent);
-  Unused << manager->ReceiveInputEvent(input);
-
-  // Simulate WebRender compositing frames until APZ tells it the scroll offset
-  // has stopped changing.
-  // Important to trigger the bug: the first composite has the same time stamp
-  // as the earlier one above.
-  ParentLayerPoint compositedScrollOffset;
-  while (true) {
-    bool needMoreFrames = manager->AdvanceAnimations(mcc->GetSampleTime());
-    compositedScrollOffset = ApzcOf(root)->GetCurrentAsyncScrollOffset(
-        AsyncPanZoomController::eForCompositing);
-    if (!needMoreFrames) {
-      break;
-    }
-    mcc->AdvanceBy(TimeDuration::FromMilliseconds(16));
-  }
-
-  // Check that the effect of the keyboard scroll has been composited.
-  EXPECT_GT(compositedScrollOffset.y, 0);
-}