From aa7d787c6ef191627d46d76b9aac5a52b2a999e9 Mon Sep 17 00:00:00 2001
From: Kartikaya Gupta <kgupta@mozilla.com>
Date: Thu, 9 Jul 2020 10:39:59 +0000
Subject: [PATCH] Bug 1651050 - Change how the zoomable flag is used in
 scrollframes. r=tnikkel

If the root scrollframe has the zoomable flag set, it automatically returns
true from WantAsyncScroll which makes it the "primary" scrollframe, and
automatically gets a displayport. However, this can be undesirable in cases
where the root scrollframe is not actually scrollable (i.e. document content
doesn't overflow) because we don't actually need that displayport on the root
scrollframe and instead want it on a scrollframe that is actually scrollable.

This patch removes the behaviour of WantAsyncScroll returning true for such
scrollframes, but maintains the layerization changes needed to support zooming.
This reduces the set of differences between running with apz.allow_zooming on
and off, which in turn eliminates a bunch of test failures when that pref is
enabled.

Depends on D82777

Differential Revision: https://phabricator.services.mozilla.com/D82778
---
 layout/generic/nsGfxScrollFrame.cpp | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp
index 73873be5b446..cb12c3c051db 100644
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -1442,8 +1442,15 @@ void ScrollFrameHelper::SetScrollableByAPZ(bool aScrollable) {
 }
 
 void ScrollFrameHelper::SetZoomableByAPZ(bool aZoomable) {
+  if (!nsLayoutUtils::UsesAsyncScrolling(mOuter)) {
+    // If APZ is disabled on this window, then we're never actually going to
+    // do any zooming. So we don't need to do any of the setup for it. Note
+    // that this function gets called from ZoomConstraintsClient even if APZ
+    // is disabled to indicate the zoomability of content.
+    aZoomable = false;
+  }
   if (mZoomableByAPZ != aZoomable) {
-    // We might be changing the result of WantAsyncScroll() so schedule a
+    // We might be changing the result of DecideScrollableLayer() so schedule a
     // paint to make sure we pick up the result of that change.
     mZoomableByAPZ = aZoomable;
     mOuter->SchedulePaint();
@@ -1455,12 +1462,6 @@ void ScrollFrameHelper::SetHasOutOfFlowContentInsideFilter() {
 }
 
 bool ScrollFrameHelper::WantAsyncScroll() const {
-  // If zooming is allowed, and this is a frame that's allowed to zoom, then
-  // we want it to be async-scrollable or zooming will not be permitted.
-  if (mZoomableByAPZ) {
-    return true;
-  }
-
   ScrollStyles styles = GetScrollStylesFromFrame();
   nscoord oneDevPixel =
       GetScrolledFrame()->PresContext()->AppUnitsPerDevPixel();
@@ -4136,8 +4137,9 @@ bool ScrollFrameHelper::DecideScrollableLayer(
   // the compositor can find the scrollable layer for async scrolling.
   // If the element is marked 'scrollgrab', also force building of a layer
   // so that APZ can implement scroll grabbing.
-  mWillBuildScrollableLayer =
-      usingDisplayPort || nsContentUtils::HasScrollgrab(content);
+  mWillBuildScrollableLayer = usingDisplayPort ||
+                              nsContentUtils::HasScrollgrab(content) ||
+                              mZoomableByAPZ;
 
   // The cached animated geometry root for the display builder is out of
   // date if we just introduced a new animated geometry root.