Bug 2067277 - Always report APZ autoscroll rejection by message. r=hiro,botond

StartAutoscroll returned whether APZ accepted the autoscroll, but that
answer only reached the caller when the compositor ran in the parent
process. With a GPU process the request goes over an async message, so the
return value only said the message was sent and the rejection came back
via message instead. This patch makes it report the rejection that way in
both cases.

Notable details:

- AutoScrollChild now registers its "autoscroll-rejected-by-apz" observer
  before sending "Autoscroll:Start" rather than after the reply. Without a
  GPU process, ChromeProcessController::NotifyAsyncAutoscrollRejected is
  already on the UI thread and notifies synchronously, so the rejection
  would otherwise always be delivered before the observer exists and be
  dropped.

- CanonicalBrowsingContext::StartApzAutoscroll keeps returning bool for its
  own early-outs (no BrowserParent, no widget, APZ disabled) to prevent
  a regression of main-thread scrolling. nsIWidget::StartAsyncAutoscroll no
  longer has anything to report.

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

docshell/base/CanonicalBrowsingContext.cpp

@@ -3725,10 +3725,11 @@ bool CanonicalBrowsingContext::StartApzAutoscroll(float aAnchorX,
 
   mozilla::layers::ScrollableLayerGuid guid(layersId, aPresShellId, aScrollId);
 
-  return widget->StartAsyncAutoscroll(
+  widget->StartAsyncAutoscroll(
       ViewAs<ScreenPixel>(
           anchor, PixelCastJustification::LayoutDeviceIsScreenForBounds),
       guid);
+  return true;
 }
 
 void CanonicalBrowsingContext::StopApzAutoscroll(nsViewID aScrollId,

gfx/layers/apz/public/IAPZCTreeManager.h

@@ -116,7 +116,7 @@ class IAPZCTreeManager {
   virtual void StartScrollbarDrag(const ScrollableLayerGuid& aGuid,
                                   const AsyncDragMetrics& aDragMetrics) = 0;
 
-  virtual bool StartAutoscroll(const ScrollableLayerGuid& aGuid,
+  virtual void StartAutoscroll(const ScrollableLayerGuid& aGuid,
                                const ScreenPoint& aAnchorLocation) = 0;
 
   virtual void StopAutoscroll(const ScrollableLayerGuid& aGuid) = 0;

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -1151,24 +1151,17 @@ void APZCTreeManager::StartScrollbarDrag(const ScrollableLayerGuid& aGuid,
   mInputQueue->ConfirmDragBlock(inputBlockId, apzc, aDragMetrics);
 }
 
-bool APZCTreeManager::StartAutoscroll(const ScrollableLayerGuid& aGuid,
+void APZCTreeManager::StartAutoscroll(const ScrollableLayerGuid& aGuid,
                                       const ScreenPoint& aAnchorLocation) {
   APZThreadUtils::AssertOnControllerThread();
 
   RefPtr<AsyncPanZoomController> apzc = GetTargetAPZC(aGuid);
   if (!apzc) {
-    if (XRE_IsGPUProcess()) {
-      // If we're in the compositor process, the "return false" will be
-      // ignored because the query comes over the PAPZCTreeManager protocol
-      // via an async message. In this case, send an explicit rejection
-      // message to content.
-      NotifyAutoscrollRejected(aGuid);
-    }
-    return false;
+    NotifyAutoscrollRejected(aGuid);
+    return;
   }
 
   apzc->StartAutoscroll(aAnchorLocation);
-  return true;
 }
 
 void APZCTreeManager::StopAutoscroll(const ScrollableLayerGuid& aGuid) {
@@ -1203,8 +1196,9 @@ void APZCTreeManager::NotifyAutoscrollRejected(
     const ScrollableLayerGuid& aGuid) const {
   RefPtr<GeckoContentController> controller =
       GetContentController(aGuid.mLayersId);
-  MOZ_ASSERT(controller);
-  controller->NotifyAsyncAutoscrollRejected(aGuid.mScrollId);
+  if (controller) {
+    controller->NotifyAsyncAutoscrollRejected(aGuid.mScrollId);
+  }
 }
 
 void SetHitTestData(HitTestingTreeNode* aNode,

gfx/layers/apz/src/APZCTreeManager.h

@@ -410,7 +410,7 @@ class APZCTreeManager : public IAPZCTreeManager, public APZInputBridge {
   void StartScrollbarDrag(const ScrollableLayerGuid& aGuid,
                           const AsyncDragMetrics& aDragMetrics) override;
 
-  bool StartAutoscroll(const ScrollableLayerGuid& aGuid,
+  void StartAutoscroll(const ScrollableLayerGuid& aGuid,
                        const ScreenPoint& aAnchorLocation) override;
 
   void StopAutoscroll(const ScrollableLayerGuid& aGuid) override;

gfx/layers/ipc/APZCTreeManagerChild.cpp

@@ -106,10 +106,10 @@ void APZCTreeManagerChild::StartScrollbarDrag(
   SendStartScrollbarDrag(aGuid, aDragMetrics);
 }
 
-bool APZCTreeManagerChild::StartAutoscroll(const ScrollableLayerGuid& aGuid,
+void APZCTreeManagerChild::StartAutoscroll(const ScrollableLayerGuid& aGuid,
                                            const ScreenPoint& aAnchorLocation) {
   MOZ_ASSERT(NS_IsMainThread());
-  return SendStartAutoscroll(aGuid, aAnchorLocation);
+  SendStartAutoscroll(aGuid, aAnchorLocation);
 }
 
 void APZCTreeManagerChild::StopAutoscroll(const ScrollableLayerGuid& aGuid) {

gfx/layers/ipc/APZCTreeManagerChild.h

@@ -57,7 +57,7 @@ class APZCTreeManagerChild final : public IAPZCTreeManager,
   void StartScrollbarDrag(const ScrollableLayerGuid& aGuid,
                           const AsyncDragMetrics& aDragMetrics) override;
 
-  bool StartAutoscroll(const ScrollableLayerGuid& aGuid,
+  void StartAutoscroll(const ScrollableLayerGuid& aGuid,
                        const ScreenPoint& aAnchorLocation) override;
 
   void StopAutoscroll(const ScrollableLayerGuid& aGuid) override;

toolkit/actors/AutoScrollChild.sys.mjs

@@ -219,17 +219,38 @@ export class AutoScrollChild extends JSWindowActorChild {
       // No view ID - leave this._scrollId as null. Receiving side will check.
     }
     let presShellId = domUtils.getPresShellId();
-    let { autoscrollEnabled, usingApz } = await this.sendQuery(
-      "Autoscroll:Start",
-      {
-        scrolldir: this._scrolldir,
-        screenXDevPx: event.screenX * content.devicePixelRatio,
-        screenYDevPx: event.screenY * content.devicePixelRatio,
-        scrollId: this._scrollId,
-        presShellId,
-      }
-    );
+
+    // APZ notifies us of a rejection over a different channel than the reply
+    // below, so the two are not ordered. Observe before asking, otherwise a
+    // rejection that wins the race is dropped and nothing scrolls at all.
+    // And before beginning observation, we have to initialize the coordinates.
+    this._startX = event.screenX;
+    this._startY = event.screenY;
+    this._screenX = event.screenX;
+    this._screenY = event.screenY;
+    this._scrollErrorX = 0;
+    this._scrollErrorY = 0;
+    this._autoscrollHandledByApz = true;
+    Services.obs.addObserver(this.observer, "autoscroll-rejected-by-apz");
+
+    let autoscrollEnabled, usingApz;
+    try {
+      ({ autoscrollEnabled, usingApz } = await this.sendQuery(
+        "Autoscroll:Start",
+        {
+          scrolldir: this._scrolldir,
+          screenXDevPx: event.screenX * content.devicePixelRatio,
+          screenYDevPx: event.screenY * content.devicePixelRatio,
+          scrollId: this._scrollId,
+          presShellId,
+        }
+      ));
+    } catch {
+      // The actor was destroyed before the reply arrived.
+      autoscrollEnabled = false;
+    }
     if (!autoscrollEnabled) {
+      this.stopObservingApzRejection();
       this._scrollable = null;
       return;
     }
@@ -244,23 +265,11 @@ export class AutoScrollChild extends JSWindowActorChild {
     });
     this.document.addEventListener("pagehide", this, true);
 
-    this._startX = event.screenX;
-    this._startY = event.screenY;
-    this._screenX = event.screenX;
-    this._screenY = event.screenY;
-    this._scrollErrorX = 0;
-    this._scrollErrorY = 0;
-    this._autoscrollHandledByApz = usingApz;
-
     if (!usingApz) {
-      // If the browser didn't hand the autoscroll off to APZ,
-      // scroll here in the main thread.
+      // The browser didn't hand the autoscroll off to APZ, so scroll here in
+      // the main thread.
+      this.stopObservingApzRejection();
       this.startMainThreadScroll();
-    } else {
-      // Even if the browser did hand the autoscroll to APZ,
-      // APZ might reject it in which case it will notify us
-      // and we need to take over.
-      Services.obs.addObserver(this.observer, "autoscroll-rejected-by-apz");
     }
 
     if (Cu.isInAutomation) {
@@ -268,6 +277,15 @@ export class AutoScrollChild extends JSWindowActorChild {
     }
   }
 
+  // Removes the "autoscroll-rejected-by-apz" observer if it is still
+  // registered. Safe to call more than once.
+  stopObservingApzRejection() {
+    if (this._autoscrollHandledByApz) {
+      this._autoscrollHandledByApz = false;
+      Services.obs.removeObserver(this.observer, "autoscroll-rejected-by-apz");
+    }
+  }
+
   startMainThreadScroll() {
     let content = this.document.defaultView;
     this._lastFrame = content.performance.now();
@@ -288,12 +306,7 @@ export class AutoScrollChild extends JSWindowActorChild {
         mozSystemGroup: true,
       });
       this.document.removeEventListener("pagehide", this, true);
-      if (this._autoscrollHandledByApz) {
-        Services.obs.removeObserver(
-          this.observer,
-          "autoscroll-rejected-by-apz"
-        );
-      }
+      this.stopObservingApzRejection();
     }
   }
 
@@ -452,9 +465,8 @@ export class AutoScrollChild extends JSWindowActorChild {
   rejectedByApz(data) {
     // The caller passes in the scroll id via 'data'.
     if (data == this._scrollId) {
-      this._autoscrollHandledByApz = false;
+      this.stopObservingApzRejection();
       this.startMainThreadScroll();
-      Services.obs.removeObserver(this.observer, "autoscroll-rejected-by-apz");
     }
   }
 }

widget/nsIWidget.cpp

@@ -2102,11 +2102,11 @@ void nsIWidget::StartAsyncScrollbarDrag(const AsyncDragMetrics& aDragMetrics) {
   mAPZC->StartScrollbarDrag(guid, aDragMetrics);
 }
 
-bool nsIWidget::StartAsyncAutoscroll(const ScreenPoint& aAnchorLocation,
+void nsIWidget::StartAsyncAutoscroll(const ScreenPoint& aAnchorLocation,
                                      const ScrollableLayerGuid& aGuid) {
   MOZ_ASSERT(XRE_IsParentProcess() && AsyncPanZoomEnabled());
 
-  return mAPZC->StartAutoscroll(aGuid, aAnchorLocation);
+  mAPZC->StartAutoscroll(aGuid, aAnchorLocation);
 }
 
 void nsIWidget::StopAsyncAutoscroll(const ScrollableLayerGuid& aGuid) {

widget/nsIWidget.h

@@ -1807,12 +1807,12 @@ class nsIWidget : public nsSupportsWeakReference {
   virtual void StartAsyncScrollbarDrag(const AsyncDragMetrics& aDragMetrics);
 
   /**
-   * Notify APZ to start autoscrolling.
+   * Notify APZ to start autoscrolling. APZ may still reject the autoscroll,
+   * in which case it notifies content itself.
    * @param aAnchorLocation the location of the autoscroll anchor
    * @param aGuid identifies the scroll frame to be autoscrolled
-   * @return true if APZ has been successfully notified
    */
-  bool StartAsyncAutoscroll(const ScreenPoint& aAnchorLocation,
+  void StartAsyncAutoscroll(const ScreenPoint& aAnchorLocation,
                             const ScrollableLayerGuid& aGuid);
 
   /**