Bug 1715179 - Allow double-tap-to-zoom to zoom in contents inside OOP iframes. r=botond

Depends on D186325 Differential Revision: https://phabricator.services.mozilla.com/D186326
2023-12-04 09:31:23 +00:00 · 2023-12-04 09:31:23 +00:00 · 80e4683e9e
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@ -2440,8 +2440,12 @@ void APZCTreeManager::ZoomToRect(const ScrollableLayerGuid& aGuid,
    }
    return;
  }
+
  if (apzc) {
-    apzc->ZoomToRect(aZoomTarget, aFlags);
+    apzc = FindZoomableApzc(apzc);
+    if (apzc) {
+      apzc->ZoomToRect(aZoomTarget, aFlags);
+    }
  }
 }

--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@ -1230,19 +1230,11 @@ nsEventStatus AsyncPanZoomController::HandleGestureEvent(
          rv = OnSingleTapConfirmed(tapGestureInput);
          break;
        case TapGestureInput::TAPGESTURE_DOUBLE:
-          // This means that double tapping on an oop iframe "works" in that we
-          // don't try (and fail) to zoom the oop iframe. But it also means it
-          // is impossible to zoom to some content inside that oop iframe.
-          // Instead the best we can do is zoom to the oop iframe itself. This
-          // is consistent with what Chrome and Safari currently do. Allowing
-          // zooming to content inside an oop iframe would be decently
-          // complicated and it doesn't seem worth it. Bug 1715179 is on file
-          // for this.
          if (!IsRootContent()) {
            if (APZCTreeManager* treeManagerLocal = GetApzcTreeManager()) {
-              if (RefPtr<AsyncPanZoomController> root =
-                      treeManagerLocal->FindZoomableApzc(this)) {
-                rv = root->OnDoubleTap(tapGestureInput);
+              if (AsyncPanZoomController* apzc =
+                      treeManagerLocal->FindRootApzcFor(GetLayersId())) {
+                rv = apzc->OnDoubleTap(tapGestureInput);
              }
            }
            break;
@ -3193,7 +3185,7 @@ nsEventStatus AsyncPanZoomController::OnDoubleTap(

  RefPtr<GeckoContentController> controller = GetGeckoContentController();
  if (controller) {
-    if (ZoomConstraintsAllowDoubleTapZoom() &&
+    if (rootContentApzc->ZoomConstraintsAllowDoubleTapZoom() &&
        (!GetCurrentTouchBlock() ||
         GetCurrentTouchBlock()->TouchActionAllowsDoubleTapZoom())) {
      if (Maybe<LayoutDevicePoint> geckoScreenPoint =
--- a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html
+++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html
@ -0,0 +1,142 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=2100,initial-scale=0.4"/>
+  <title>Tests that double tap to zoom in iframe works regardless whether cross-origin or not</title>
+  <script src="apz_test_native_event_utils.js"></script>
+  <script src="apz_test_utils.js"></script>
+  <script src="/tests/SimpleTest/paint_listener.js"></script>
+  <style>
+  html {
+    /* To avoid bug 1865573 */
+    scrollbar-width: none;
+  }
+  iframe {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+    border: none;
+  }
+  </style>
+
+  <script>
+
+async function setupIframe(aURL) {
+  const iframe = document.querySelector("iframe");
+  const iframeLoadPromise = promiseOneEvent(iframe, "load", null);
+  iframe.src = aURL;
+  await iframeLoadPromise;
+  info(`${aURL} loaded`);
+
+  await SpecialPowers.spawn(iframe, [], async () => {
+    await content.wrappedJSObject.waitUntilApzStable();
+    await SpecialPowers.contentTransformsReceived(content);
+  });
+}
+
+async function targetElementPosition() {
+  const iframe = document.querySelector("iframe");
+  return SpecialPowers.spawn(iframe, [], async () => {
+    return content.document.querySelector("#target").getBoundingClientRect();
+  });
+}
+
+function cloneVisualViewport() {
+  return {
+    offsetLeft: visualViewport.offsetLeft,
+    offsetTop:  visualViewport.offsetTop,
+    pageLeft: visualViewport.pageLeft,
+    pageTop: visualViewport.pageTop,
+    width: visualViewport.width,
+    height: visualViewport.height,
+    scale: visualViewport.scale,
+  };
+}
+
+function compareVisualViewport(aVisualViewportValue1, aVisualViewportValue2, aMessage) {
+  for (let p in aVisualViewportValue1) {
+    // Due to the method difference of the calculation for double-tap-zoom in
+    // OOP iframes, we allow 1.0 difference in each visualViewport value.
+    // NOTE: Because of our layer pixel snapping (bug 1774315 and bug 1852884)
+    // the visual viewport metrics can have one more pixel difference so we
+    // allow it here.
+    const tolerance = 1.0 + 1.0;
+    isfuzzy(aVisualViewportValue1[p], aVisualViewportValue2[p],
+            aVisualViewportValue1.scale > 1.0 ? tolerance : tolerance / aVisualViewportValue1.scale,
+            `${p} should be same on ${aMessage}`);
+  }
+}
+
+const useTouchpad = (location.search == "?touchpad");
+
+async function test(aTestFile) {
+  let iframeURL = SimpleTest.getTestFileURL(aTestFile);
+
+  // Load the test document in the same origin.
+  await setupIframe(iframeURL);
+
+  let resolution = await getResolution();
+  ok(resolution > 0,
+     "The initial_resolution is " + resolution + ", which is some sane value");
+
+  let pos = await targetElementPosition();
+  const iframe = document.querySelector("iframe");
+  await doubleTapOn(iframe, pos.x + 10, pos.y + 10, useTouchpad);
+
+  let prev_resolution = resolution;
+  resolution = await getResolution();
+  ok(resolution > prev_resolution, "The first double-tap has increased the resolution to " + resolution);
+
+  let zoomedInState = cloneVisualViewport();
+
+  await doubleTapOn(iframe, pos.x + 10, pos.y + 10, useTouchpad);
+  prev_resolution = resolution;
+  resolution = await getResolution();
+  ok(resolution < prev_resolution, "The second double-tap has decreased the resolution to " + resolution);
+
+  let zoomedOutState = cloneVisualViewport();
+
+  // Now load the document in an OOP iframe.
+  iframeURL = iframeURL.replace(window.location.origin, "https://example.com");
+  await setupIframe(iframeURL);
+
+  await doubleTapOn(iframe, pos.x + 10, pos.y + 10, useTouchpad);
+  prev_resolution = resolution;
+  resolution = await getResolution();
+  ok(resolution > prev_resolution, "The first double-tap has increased the resolution to " + resolution);
+
+  compareVisualViewport(zoomedInState, cloneVisualViewport(), "zoomed-in state");
+
+  await doubleTapOn(iframe, pos.x + 10, pos.y + 10, useTouchpad);
+  compareVisualViewport(zoomedOutState, cloneVisualViewport(), "zoomed-out state");
+}
+
+async function moveIframe() {
+  const iframe = document.querySelector("iframe");
+  iframe.style.top = "500vh";
+
+  // Scroll to the bottom to make the layout scroll offset non-zero.
+  window.scrollTo(0, document.documentElement.scrollHeight);
+  ok(window.scrollY > 0, "The root scroll position should be non-zero");
+
+  await SpecialPowers.spawn(iframe, [], async () => {
+    await SpecialPowers.contentTransformsReceived(content);
+  });
+}
+
+waitUntilApzStable()
+.then(async () => test("helper_doubletap_zoom_oopif_subframe-1.html"))
+// A test case where the layout scroll offset isn't zero.
+.then(async () => moveIframe())
+.then(async () => test("helper_doubletap_zoom_oopif_subframe-1.html"))
+// A test case where the layout scroll offset in the iframe isn't zero.
+.then(async () => test("helper_doubletap_zoom_oopif_subframe-2.html#target"))
+.then(subtestDone, subtestFailed);
+
+  </script>
+</head>
+<body>
+<iframe width="500" height="500"></iframe>
+</body>
+</html>
--- a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html
+++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html
@ -0,0 +1,21 @@
+<!DOCTYPE HTML>
+<html>
+<script src="apz_test_native_event_utils.js"></script>
+<script src="apz_test_utils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/paint_listener.js"></script>
+<style>
+html, body {
+  margin: 0;
+  padding: 0;
+}
+</style>
+<div id="target" style="width: 100px; height: 100px; position: absolute; top: 300px; left: 100px; background: blue;"></div>
+<script>
+  // Silence SimpleTest warning about missing assertions by having it wait
+  // indefinitely. We don't need to give it an explicit finish because the
+  // entire window this test runs in will be closed after the main browser test
+  // finished.
+  SimpleTest.waitForExplicitFinish();
+</script>
+</html>
--- a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html
+++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html
@ -0,0 +1,21 @@
+<!DOCTYPE HTML>
+<html>
+<script src="apz_test_native_event_utils.js"></script>
+<script src="apz_test_utils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/paint_listener.js"></script>
+<style>
+html, body {
+  margin: 0;
+  padding: 0;
+}
+</style>
+<div id="target" style="width: 100px; height: 100px; position: absolute; top: 300vh; left: 100px; background: blue;"></div>
+<script>
+  // Silence SimpleTest warning about missing assertions by having it wait
+  // indefinitely. We don't need to give it an explicit finish because the
+  // entire window this test runs in will be closed after the main browser test
+  // finished.
+  SimpleTest.waitForExplicitFinish();
+</script>
+</html>
--- a/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html
+++ b/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html
@ -49,6 +49,10 @@ var subtests = [
  {"file": "helper_doubletap_zoom_noscroll.html", "prefs": doubletap_prefs},
  {"file": "helper_doubletap_zoom_square.html", "prefs": doubletap_prefs},
  {"file": "helper_doubletap_zoom_oopif.html", "prefs": doubletap_prefs},
+  {"file": "helper_doubletap_zoom_oopif-2.html", "prefs": [
+    ...meta_viewport_and_doubletap_prefs,
+    ["layout.scroll.disable-pixel-alignment", true],
+    ]},
  {"file": "helper_disallow_doubletap_zoom_inside_oopif.html", "prefs": meta_viewport_and_doubletap_prefs},
  {"file": "helper_doubletap_zoom_nothing_listener.html", "prefs": doubletap_prefs},
  {"file": "helper_doubletap_zoom_touch_action_manipulation.html", "prefs": meta_viewport_and_doubletap_prefs},
@ -69,8 +73,11 @@ if (getPlatform() == "mac") {
    {"file": "helper_doubletap_zoom_noscroll.html?touchpad", "prefs": doubletap_prefs},
    {"file": "helper_doubletap_zoom_square.html?touchpad", "prefs": doubletap_prefs},
    {"file": "helper_doubletap_zoom_oopif.html?touchpad", "prefs": doubletap_prefs},
+    {"file": "helper_doubletap_zoom_oopif-2.html?touchpad", "prefs": doubletap_prefs},
    {"file": "helper_disallow_doubletap_zoom_inside_oopif.html?touchpad", "prefs": meta_viewport_and_doubletap_prefs},
-    {"file": "helper_doubletap_zoom_nothing_listener.html?touchpad", "prefs": doubletap_prefs},
+    {"file": "helper_doubletap_zoom_nothing_listener.html?touchpad", "prefs": [
+       ...doubletap_prefs,
+       ["layout.scroll.disable-pixel-alignment", true]]},
    {"file": "helper_doubletap_zoom_touch_action_manipulation.html?touchpad", "prefs": doubletap_prefs},
    {"file": "helper_doubletap_zoom_touch_action_manipulation_in_iframe.html?touchpad", "prefs": doubletap_prefs},
  );
--- a/gfx/layers/apz/util/ChromeProcessController.cpp
+++ b/gfx/layers/apz/util/ChromeProcessController.cpp
@ -146,14 +146,8 @@ void ChromeProcessController::HandleDoubleTap(
  ZoomTarget zoomTarget =
      CalculateRectToZoomTo(document, aPoint, aDoubleTapToZoomMetrics);

-  uint32_t presShellId;
-  ScrollableLayerGuid::ViewID viewId;
-  if (APZCCallbackHelper::GetOrCreateScrollIdentifiers(
-          document->GetDocumentElement(), &presShellId, &viewId)) {
-    mAPZCTreeManager->ZoomToRect(
-        ScrollableLayerGuid(aGuid.mLayersId, presShellId, viewId), zoomTarget,
-        ZoomToRectBehavior::DEFAULT_BEHAVIOR);
-  }
+  mAPZCTreeManager->ZoomToRect(aGuid, zoomTarget,
+                               ZoomToRectBehavior::DEFAULT_BEHAVIOR);
 }

 void ChromeProcessController::HandleTap(
--- a/gfx/layers/apz/util/DoubleTapToZoom.cpp
+++ b/gfx/layers/apz/util/DoubleTapToZoom.cpp
@ -100,11 +100,11 @@ static dom::Element* GetNearbyTableCell(
 // level content document coordinates.
 static CSSRect GetBoundingContentRect(
    const dom::Element* aElement,
-    const RefPtr<dom::Document>& aRootContentDocument,
+    const RefPtr<dom::Document>& aInProcessRootContentDocument,
    const nsIScrollableFrame* aRootScrollFrame,
    const DoubleTapToZoomMetrics& aMetrics,
    mozilla::Maybe<CSSRect>* aOutNearestScrollClip = nullptr) {
-  if (aRootContentDocument->IsTopLevelContentDocument()) {
+  if (aInProcessRootContentDocument->IsTopLevelContentDocument()) {
    return nsLayoutUtils::GetBoundingContentRect(aElement, aRootScrollFrame,
                                                 aOutNearestScrollClip);
  }
@ -124,7 +124,7 @@ static CSSRect GetBoundingContentRect(

 static bool ShouldZoomToElement(
    const nsCOMPtr<dom::Element>& aElement,
-    const RefPtr<dom::Document>& aRootContentDocument,
+    const RefPtr<dom::Document>& aInProcessRootContentDocument,
    nsIScrollableFrame* aRootScrollFrame,
    const DoubleTapToZoomMetrics& aMetrics) {
  if (nsIFrame* frame = aElement->GetPrimaryFrame()) {
@ -139,7 +139,7 @@ static bool ShouldZoomToElement(
  // Trying to zoom to the html element will just end up scrolling to the start
  // of the document, return false and we'll run out of elements and just
  // zoomout (without scrolling to the start).
-  if (aElement->OwnerDoc() == aRootContentDocument &&
+  if (aElement->OwnerDoc() == aInProcessRootContentDocument &&
      aElement->IsHTMLElement(nsGkAtoms::html)) {
    return false;
  }
@ -153,8 +153,8 @@ static bool ShouldZoomToElement(
  // don't want to zoom to them. This heuristic is quite naive and leaves a lot
  // to be desired.
  if (dom::Element* tableCell = GetNearbyTableCell(aElement)) {
-    CSSRect rect = GetBoundingContentRect(tableCell, aRootContentDocument,
-                                          aRootScrollFrame, aMetrics);
+    CSSRect rect = GetBoundingContentRect(
+        tableCell, aInProcessRootContentDocument, aRootScrollFrame, aMetrics);
    if (rect.width < 0.3 * aMetrics.mRootScrollableRect.width) {
      return false;
    }
@ -235,15 +235,15 @@ static bool HasNonPassiveWheelListenerOnAncestor(nsIContent* aContent) {
 }

 ZoomTarget CalculateRectToZoomTo(
-    const RefPtr<dom::Document>& aRootContentDocument, const CSSPoint& aPoint,
-    const DoubleTapToZoomMetrics& aMetrics) {
+    const RefPtr<dom::Document>& aInProcessRootContentDocument,
+    const CSSPoint& aPoint, const DoubleTapToZoomMetrics& aMetrics) {
  // Ensure the layout information we get is up-to-date.
-  aRootContentDocument->FlushPendingNotifications(FlushType::Layout);
+  aInProcessRootContentDocument->FlushPendingNotifications(FlushType::Layout);

  // An empty rect as return value is interpreted as "zoom out".
  const CSSRect zoomOut;

-  RefPtr<PresShell> presShell = aRootContentDocument->GetPresShell();
+  RefPtr<PresShell> presShell = aInProcessRootContentDocument->GetPresShell();
  if (!presShell) {
    return ZoomTarget{zoomOut, CantZoomOutBehavior::ZoomIn};
  }
@ -255,7 +255,7 @@ ZoomTarget CalculateRectToZoomTo(
  }

  CSSPoint documentRelativePoint =
-      aRootContentDocument->IsTopLevelContentDocument()
+      aInProcessRootContentDocument->IsTopLevelContentDocument()
          ? CSSPoint::FromAppUnits(ViewportUtils::VisualToLayout(
                CSSPoint::ToAppUnits(aPoint), presShell)) +
                CSSPoint::FromAppUnits(rootScrollFrame->GetScrollPosition())
@ -272,7 +272,7 @@ ZoomTarget CalculateRectToZoomTo(
          ? CantZoomOutBehavior::Nothing
          : CantZoomOutBehavior::ZoomIn;

-  while (element && !ShouldZoomToElement(element, aRootContentDocument,
+  while (element && !ShouldZoomToElement(element, aInProcessRootContentDocument,
                                         rootScrollFrame, aMetrics)) {
    element = element->GetFlattenedTreeParentElement();
  }
@ -284,8 +284,8 @@ ZoomTarget CalculateRectToZoomTo(

  Maybe<CSSRect> nearestScrollClip;
  CSSRect rect =
-      GetBoundingContentRect(element, aRootContentDocument, rootScrollFrame,
-                             aMetrics, &nearestScrollClip);
+      GetBoundingContentRect(element, aInProcessRootContentDocument,
+                             rootScrollFrame, aMetrics, &nearestScrollClip);

  // In some cases, like overflow: visible and overflowing content, the bounding
  // client rect of the targeted element won't contain the point the user double
--- a/gfx/layers/apz/util/DoubleTapToZoom.h
+++ b/gfx/layers/apz/util/DoubleTapToZoom.h
@ -72,10 +72,11 @@ struct DoubleTapToZoomMetrics {
 * For a double tap at |aPoint|, return a ZoomTarget struct with contains a rect
 * to which the browser should zoom in response (see ZoomTarget definition for
 * more details). An empty rect means the browser should zoom out. |aDocument|
- * should be the root content document for the content that was tapped.
+ * should be the in-process root content document for the content that was
+ * tapped.
 */
 ZoomTarget CalculateRectToZoomTo(
-    const RefPtr<mozilla::dom::Document>& aRootContentDocument,
+    const RefPtr<mozilla::dom::Document>& aInProcessRootContentDocument,
    const CSSPoint& aPoint, const DoubleTapToZoomMetrics& aMetrics);

 }  // namespace layers