Bug 1630781 - Eliminate the frame_delay pref, assume it true everywhere. r=botond,mstange
authorKartikaya Gupta <kgupta@mozilla.com>
Sat, 25 Apr 2020 00:47:54 +0000
changeset 593292 0e01cc6c7ba91263e548d761af1b0f70fd17c706
parent 593291 b1f668b65e691fa492b2a73f2b505a8f78a83fd3
child 593293 a0a7f8bd9b39466f69d97bcd5abed41fd9e1fc17
push id2335
push userffxbld-merge
push dateMon, 25 May 2020 13:47:24 +0000
treeherdermozilla-release@69ca1d06f46a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, mstange
bugs1630781
milestone77.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 1630781 - Eliminate the frame_delay pref, assume it true everywhere. r=botond,mstange Having to think about multiple codepaths adds complexity and it doesn't seem like we're going to turn this pref back off anytime soon. Differential Revision: https://phabricator.services.mozilla.com/D72040
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
gfx/layers/apz/test/mochitest/helper_checkerboard_apzforcedisabled.html
modules/libpref/init/StaticPrefList.yaml
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -886,17 +886,17 @@ void APZCTreeManager::SampleForWebRender
         wr::ToWrTransformProperty(*info.mStickyPositionAnimationId, transform));
   }
 
   aTxn.AppendTransformProperties(transforms);
 
   // Advance animations. It's important that this happens after
   // sampling all async transforms, because AdvanceAnimations() updates
   // the effective scroll offset to the value it should have for the *next*
-  // composite after this one (if the APZ frame delay is enabled).
+  // composite after this one.
   bool activeAnimations =
       AdvanceAnimationsInternal(lock, Some(aRenderRoot), aSampleTime);
   if (activeAnimations) {
     RefPtr<CompositorController> controller;
     CompositorBridgeParent::CallWithIndirectShadowTree(
         mRootLayersId, [&](LayerTreeState& aState) -> void {
           controller = aState.GetCompositorController();
         });
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -316,23 +316,16 @@ typedef PlatformSpecificStateBase
  * \li\b apz.fling_stopped_threshold
  * When flinging, if the velocity goes below this number, we just stop the
  * animation completely. This is to prevent asymptotically approaching 0
  * velocity and rerendering unnecessarily.\n
  * Units: screen pixels per millisecond.\n
  * NOTE: Should not be set to anything
  * other than 0.0 for Android except for tests to disable flings.
  *
- * \li\b apz.frame_delay.enabled
- * If this is set to true, changes to the async scroll offset and async zoom
- * will not be immediately reflected in GetCurrentAsyncTransform() when called
- * with |AsyncTransformConsumer::eForCompositing|. Rather, the transform will
- * reflect the value of the async scroll offset and async zoom at the last time
- * SampleCompositedAsyncTransform() was called.
- *
  * \li\b apz.keyboard.enabled
  * Determines whether scrolling with the keyboard will be allowed to be handled
  * by APZ.
  *
  * \li\b apz.keyboard.passive-listeners
  * When enabled, APZ will interpret the passive event listener flag to mean
  * that the event listener won't change the focused element or selection of
  * the page. With this, web content can use passive key listeners and not have
@@ -3151,22 +3144,17 @@ nsEventStatus AsyncPanZoomController::St
 void AsyncPanZoomController::UpdateWithTouchAtDevicePoint(
     const MultiTouchInput& aEvent) {
   ParentLayerPoint point = GetFirstTouchPoint(aEvent);
   mX.UpdateWithTouchAtDevicePoint(point.x, aEvent.mTime);
   mY.UpdateWithTouchAtDevicePoint(point.y, aEvent.mTime);
 }
 
 Maybe<CompositionPayload> AsyncPanZoomController::NotifyScrollSampling() {
-  if (StaticPrefs::apz_frame_delay_enabled()) {
-    return std::move(mCompositedScrollPayload);
-  }
-  // If frame.delay disabled, the triggering events are those
-  // from the most recent frame
-  return std::move(mScrollPayload);
+  return std::move(mCompositedScrollPayload);
 }
 
 bool AsyncPanZoomController::AttemptScroll(
     ParentLayerPoint& aStartPoint, ParentLayerPoint& aEndPoint,
     OverscrollHandoffState& aOverscrollHandoffState) {
   // "start - end" rather than "end - start" because e.g. moving your finger
   // down (*positive* direction along y axis) causes the vertical scroll offset
   // to *decrease* as the page follows your finger.
@@ -4182,55 +4170,52 @@ LayoutDeviceToParentLayerScale AsyncPanZ
   return scale.ToScaleFactor() / Metrics().GetDevPixelsPerCSSPixel();
 }
 
 CSSRect AsyncPanZoomController::GetEffectiveLayoutViewport(
     AsyncTransformConsumer aMode) const {
   if (aMode == eForCompositing && mScrollMetadata.IsApzForceDisabled()) {
     return mLastContentPaintMetrics.GetLayoutViewport();
   }
-  if (aMode == eForCompositing && StaticPrefs::apz_frame_delay_enabled()) {
+  if (aMode == eForCompositing) {
     return mCompositedLayoutViewport;
   }
   return Metrics().GetLayoutViewport();
 }
 
 CSSPoint AsyncPanZoomController::GetEffectiveScrollOffset(
     AsyncTransformConsumer aMode) const {
   if (aMode == eForCompositing && mScrollMetadata.IsApzForceDisabled()) {
     return mLastContentPaintMetrics.GetVisualViewportOffset();
   }
-  if (aMode == eForCompositing && StaticPrefs::apz_frame_delay_enabled()) {
+  if (aMode == eForCompositing) {
     return mCompositedScrollOffset;
   }
   return Metrics().GetScrollOffset();
 }
 
 CSSToParentLayerScale2D AsyncPanZoomController::GetEffectiveZoom(
     AsyncTransformConsumer aMode) const {
   if (aMode == eForCompositing && mScrollMetadata.IsApzForceDisabled()) {
     return mLastContentPaintMetrics.GetZoom();
   }
-  if (aMode == eForCompositing && StaticPrefs::apz_frame_delay_enabled()) {
+  if (aMode == eForCompositing) {
     return mCompositedZoom;
   }
   return Metrics().GetZoom();
 }
 
 bool AsyncPanZoomController::SampleCompositedAsyncTransform(
     const RecursiveMutexAutoLock& aProofOfLock) {
   if (!mCompositedLayoutViewport.IsEqualEdges(Metrics().GetLayoutViewport()) ||
       mCompositedScrollOffset != Metrics().GetScrollOffset() ||
       mCompositedZoom != Metrics().GetZoom()) {
     mCompositedLayoutViewport = Metrics().GetLayoutViewport();
     mCompositedScrollOffset = Metrics().GetScrollOffset();
     mCompositedZoom = Metrics().GetZoom();
-
-    // If frame.delay enabled, we pass along the scroll events from
-    // frame before since they were what moved the items on the layer
     mCompositedScrollPayload = std::move(mScrollPayload);
     return true;
   }
   return false;
 }
 
 void AsyncPanZoomController::ApplyAsyncTestAttributes(
     const RecursiveMutexAutoLock& aProofOfLock) {
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -1160,20 +1160,16 @@ class AsyncPanZoomController {
  private:
   /**
    * Samples the composited async transform, making the result of
    * |GetCurrentAsyncTransform(eForCompositing)| and similar functions reflect
    * the async scroll offset and zoom stored in |Metrics()|.
    *
    * Returns true if the newly sampled value is different from the previously
    * sampled value.
-   *
-   * (This is only relevant when StaticPrefs::apz_frame_delay_enabled() is
-   * true. Otherwise, GetCurrentAsyncTransform() always reflects what's stored
-   * in |Metrics()| immediately, without any delay.)
    */
   bool SampleCompositedAsyncTransform(
       const RecursiveMutexAutoLock& aProofOfLock);
 
   /*
    * Helper functions to query the async layout viewport, scroll offset, and
    * zoom either directly from |Metrics()|, or from cached variables that
    * store the required value from the last time it was sampled by calling
--- a/gfx/layers/apz/test/mochitest/helper_checkerboard_apzforcedisabled.html
+++ b/gfx/layers/apz/test/mochitest/helper_checkerboard_apzforcedisabled.html
@@ -53,19 +53,17 @@ function* test(testDriver) {
   window.scrollBy({top: 500, left: 0, behavior: 'auto'});
   is(window.scrollY, 500, "document got scrolled instantly");
 
   // Note that at this point we must NOT call flushApzRepaints, because
   // otherwise APZCCallbackHelper::NotifyFlushComplete will trigger a repaint
   // (for unrelated reasons), and the repaint will clear the checkerboard
   // state. We do, however, want to wait for a "steady state" here that
   // includes all pending paints from the main thread and a composite that
-  // samples the APZ state. (The latter is not required if the APZ frame delay
-  // is disabled, but it's enabled by default so it's desirable to handle that
-  // situation as well.) In order to accomplish this we wait for all the main
+  // samples the APZ state. In order to accomplish this we wait for all the main
   // thread paints, and then force a composite via advanceTimeAndRefresh. The
   // advanceTimeAndRefresh has the additional advantage of freezing the refresh
   // driver which avoids any additional externally-triggered repaints from
   // erasing the symptoms of the bug.
   yield waitForAllPaints(() => setTimeout(testDriver, 0));
   utils.advanceTimeAndRefresh(0);
 
   var data = utils.getCompositorAPZTestData();
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -402,21 +402,16 @@
   value: 0.05f
   mirror: always
 
 - name: apz.fling_stopped_threshold
   type: AtomicFloat
   value: 0.01f
   mirror: always
 
-- name: apz.frame_delay.enabled
-  type: RelaxedAtomicBool
-  value: true
-  mirror: always
-
 - name: apz.touch_acceleration_factor_x
   type: float
   value: 1.0f
   mirror: always
 
 - name: apz.touch_acceleration_factor_y
   type: float
   value: 1.0f