Bug 1668156 - Fix some IntersectionObserver edge cases, and enable the assertion for good. r=hiro

This patch fixes two issues, described below:

First, the GetTopLevelDocument function was looking at the browsing
context tree. It should look at the window context tree, as looking at
the browsing context tree means that if you're in a discarded or
about-to-get-discarded document, you can end up with a document from a
different tree. Computing intersections between those of course makes no
sense and triggers the assertion we're enabling.

Second, this patch fixes an issue when you have fission enabled, and a
setup such as:

  A1 -> B1 -> A2

If you try to use IntersectionObserver from A2 with the implicit root,
we'd end up with:

  * rootRect: A1's root scrollport rect (this is fine, because it's only
              used to compute the root margin and bounds and so on, not
              to compute geometry).

  * rootFrame: A1's root scroll frame (this is _not_ fine, see below).

Then, we'd try to map rects from A2's target to A1's viewport, and we
can't really do that sensibly with the existing nsLayoutUtils functions,
because we're not accounting for all the OOP iframe transforms that may
be going on. This also triggers the assertion that this patch enables in
same-origin-grand-child-iframe.sub.html.

To fix it, for the A2 case, use the same code that we have for other OOP
iframes. The test tweaks fails with fission enabled without the patch
(because we don't account for the OOP iframe clip).

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

docshell/base/WindowContext.cpp

@@ -74,6 +74,11 @@ Document* WindowContext::GetDocument() const {
   return innerWindow ? innerWindow->GetDocument() : nullptr;
 }
 
+Document* WindowContext::GetExtantDoc() const {
+  nsGlobalWindowInner* innerWindow = GetInnerWindow();
+  return innerWindow ? innerWindow->GetExtantDoc() : nullptr;
+}
+
 WindowContext* WindowContext::GetParentWindowContext() {
   return mBrowsingContext->GetParentWindowContext();
 }

docshell/base/WindowContext.h

@@ -105,6 +105,7 @@ class WindowContext : public nsISupports, public nsWrapperCache {
 
   nsGlobalWindowInner* GetInnerWindow() const;
   Document* GetDocument() const;
+  Document* GetExtantDoc() const;
 
   // Get the parent WindowContext of this WindowContext, taking the BFCache into
   // account. This will not cross chrome/content <browser> boundaries.

dom/base/DOMIntersectionObserver.cpp

@@ -272,7 +272,7 @@ static Maybe<nsRect> EdgeInclusiveIntersection(const nsRect& aRect,
   return Some(nsRect(left, top, right - left, bottom - top));
 }
 
-enum class BrowsingContextOrigin { Similar, Different, Unknown };
+enum class BrowsingContextOrigin { Similar, Different };
 
 // FIXME(emilio): The whole concept of "units of related similar-origin browsing
 // contexts" is gone, but this is still in the spec, see
@@ -280,7 +280,7 @@ enum class BrowsingContextOrigin { Similar, Different, Unknown };
 static BrowsingContextOrigin SimilarOrigin(const Element& aTarget,
                                            const nsINode* aRoot) {
   if (!aRoot) {
-    return BrowsingContextOrigin::Unknown;
+    return BrowsingContextOrigin::Different;
   }
   nsIPrincipal* principal1 = aTarget.NodePrincipal();
   nsIPrincipal* principal2 = aRoot->NodePrincipal();
@@ -300,20 +300,11 @@ static BrowsingContextOrigin SimilarOrigin(const Element& aTarget,
                                     : BrowsingContextOrigin::Different;
 }
 
-// NOTE: This returns nullptr if |aDocument| is in a cross process.
-static Document* GetTopLevelDocument(const Document& aDocument) {
-  BrowsingContext* browsingContext = aDocument.GetBrowsingContext();
-  if (!browsingContext) {
-    return nullptr;
-  }
-
-  nsPIDOMWindowOuter* topWindow = browsingContext->Top()->GetDOMWindow();
-  if (!topWindow) {
-    // If we don't have a DOMWindow, We are not in same origin.
-    return nullptr;
-  }
-
-  return topWindow->GetExtantDoc();
+// NOTE: This returns nullptr if |aDocument| is in another process from the top
+// level content document.
+static Document* GetTopLevelContentDocumentInThisProcess(Document& aDocument) {
+  auto* wc = aDocument.GetTopLevelWindowContext();
+  return wc ? wc->GetExtantDoc() : nullptr;
 }
 
 // https://w3c.github.io/IntersectionObserver/#compute-the-intersection
@@ -411,7 +402,7 @@ static Maybe<nsRect> ComputeTheIntersection(
   }
 
   // In out-of-process iframes we need to take an intersection with the remote
-  // document visble rect which was already clipped by ancestor document's
+  // document visible rect which was already clipped by ancestor document's
   // viewports.
   if (aRemoteDocumentVisibleRect) {
     MOZ_ASSERT(aRoot->PresContext()->IsRootContentDocumentInProcess() &&
@@ -434,9 +425,25 @@ struct OopIframeMetrics {
   nsRect mRemoteDocumentVisibleRect;
 };
 
-static Maybe<OopIframeMetrics> GetOopIframeMetrics(Document& aDocument) {
+static Maybe<OopIframeMetrics> GetOopIframeMetrics(Document& aDocument,
+                                                   Document* aRootDocument) {
   Document* rootDoc = nsContentUtils::GetRootDocument(&aDocument);
-  MOZ_ASSERT(rootDoc && !rootDoc->IsTopLevelContentDocument());
+  MOZ_ASSERT(rootDoc);
+
+  if (rootDoc->IsTopLevelContentDocument()) {
+    return Nothing();
+  }
+
+  if (aRootDocument &&
+      rootDoc == nsContentUtils::GetRootDocument(aRootDocument)) {
+    // aRootDoc, if non-null, is either the implicit root
+    // (top-level-content-document) or a same-origin document passed explicitly.
+    //
+    // In the former case, we should've returned above if there are no iframes
+    // in between. This condition handles the explicit, same-origin root
+    // document, when both are embedded in an OOP iframe.
+    return Nothing();
+  }
 
   PresShell* rootPresShell = rootDoc->GetPresShell();
   if (!rootPresShell || rootPresShell->IsDestroying()) {
@@ -509,21 +516,36 @@ void DOMIntersectionObserver::Update(Document* aDocument,
   } else {
     MOZ_ASSERT(!mRoot || mRoot->IsDocument());
     Document* rootDocument =
-        mRoot ? mRoot->AsDocument() : GetTopLevelDocument(*aDocument);
+        mRoot ? mRoot->AsDocument()
+              : GetTopLevelContentDocumentInThisProcess(*aDocument);
+    root = rootDocument;
+
     if (rootDocument) {
+      // We're in the same process as the root document, though note that there
+      // could be an out-of-process iframe in between us and the root. Grab the
+      // root frame and the root rect.
+      //
+      // Note that the root rect is always good (we assume no DPI changes in
+      // between the two documents, and we don't need to convert coordinates).
+      //
+      // The root frame however we may need to tweak in the block below, if
+      // there's any OOP iframe in between `rootDocument` and `aDocument`, to
+      // handle the OOP iframe positions.
       if (PresShell* presShell = rootDocument->GetPresShell()) {
         rootFrame = presShell->GetRootScrollFrame();
         if (rootFrame) {
-          root = rootFrame->GetContent()->AsElement();
           nsIScrollableFrame* scrollFrame = do_QueryFrame(rootFrame);
           rootRect = scrollFrame->GetScrollPortRect();
         }
       }
-    } else if (Maybe<OopIframeMetrics> metrics =
-                   GetOopIframeMetrics(*aDocument)) {
-      // `implicit root` case in an out-of-process iframe.
+    }
+
+    if (Maybe<OopIframeMetrics> metrics =
+            GetOopIframeMetrics(*aDocument, rootDocument)) {
       rootFrame = metrics->mInProcessRootFrame;
-      rootRect = metrics->mInProcessRootRect;
+      if (!rootDocument) {
+        rootRect = metrics->mInProcessRootRect;
+      }
       remoteDocumentVisibleRect = Some(metrics->mRemoteDocumentVisibleRect);
     }
   }
@@ -555,8 +577,6 @@ void DOMIntersectionObserver::Update(Document* aDocument,
     }
 
     BrowsingContextOrigin origin = SimilarOrigin(*target, root);
-    MOZ_ASSERT_IF(remoteDocumentVisibleRect,
-                  origin != BrowsingContextOrigin::Similar);
     if (origin == BrowsingContextOrigin::Similar) {
       rootBounds.Inflate(rootMargin);
     }

layout/base/nsLayoutUtils.cpp

@@ -3197,10 +3197,7 @@ nsRect nsLayoutUtils::TransformFrameRectToAncestor(
     Maybe<Matrix4x4Flagged>* aMatrixCache /* = nullptr */,
     bool aStopAtStackingContextAndDisplayPortAndOOFFrame /* = false */,
     nsIFrame** aOutAncestor /* = nullptr */) {
-  // FIXME(emilio, bug 1668156): The pres context check shouldn't be needed, it
-  // should hold regardless, but there are some existing bogus callers...
-  MOZ_ASSERT(aAncestor.mFrame->PresContext() != aFrame->PresContext() ||
-                 IsAncestorFrameCrossDoc(aAncestor.mFrame, aFrame),
+  MOZ_ASSERT(IsAncestorFrameCrossDoc(aAncestor.mFrame, aFrame),
              "Fix the caller");
 
   SVGTextFrame* text = GetContainingSVGTextFrame(aFrame);

testing/web-platform/tests/intersection-observer/resources/cross-origin-child-iframe.sub.html

@@ -1,6 +1,6 @@
 <!DOCTYPE html>
 <script src="/common/get-host-info.sub.js"></script>
-<iframe id="iframe"></iframe>
+<iframe scrolling="no" frameborder="0" id="iframe"></iframe>
 <script>
 iframe.src =
   get_host_info().ORIGIN + "/intersection-observer/resources/same-origin-grand-child-iframe.html";

testing/web-platform/tests/intersection-observer/resources/same-origin-grand-child-iframe.html

@@ -1,8 +1,15 @@
 <!DOCTYPE html>
-<div id="target"></div>
+<!--
+  target should be fully vertically in-viewport as 100px is way less than the
+  default iframe height.
+
+  right: 0 makes sure that we're occluded by 8px by the intermediate iframe.
+-->
+<div id="target" style="width: 100px; height: 100px; position: absolute; right: 0"></div>
 <script>
 const observer = new IntersectionObserver(records => {
-  window.top.postMessage(records[0].rootBounds, "*");
+  let { rootBounds, intersectionRect } = records[0];
+  window.top.postMessage({ rootBounds, intersectionRect }, "*");
 }, {});
-observer.observe(target);
+observer.observe(document.getElementById("target"));
 </script>

testing/web-platform/tests/intersection-observer/same-origin-grand-child-iframe.sub.html

@@ -5,16 +5,25 @@
 <script src="/resources/testharnessreport.js"></script>
 <script src="/common/get-host-info.sub.js"></script>
 <script src="./resources/intersection-observer-test-utils.js"></script>
-<iframe id="iframe"></iframe>
+<iframe scrolling="no" frameborder="0" id="iframe"></iframe>
 <script>
 promise_test(async t => {
   iframe.src =
     get_host_info().HTTP_NOTSAMESITE_ORIGIN + "/intersection-observer/resources/cross-origin-child-iframe.sub.html";
 
-  const rootBounds = await new Promise(resolve => {
+  const { rootBounds, intersectionRect } = await new Promise(resolve => {
     window.addEventListener("message", event => resolve(event.data));
   }, { once: true } );
 
+  // 300px = iframe viewport width
+  // 8px = default body margin
+  // (intersectionRect is in the coordinate space of the target iframe)
+  assert_equals(intersectionRect.top, 8);
+  assert_equals(intersectionRect.left, 200);
+  assert_equals(intersectionRect.right, 300 - 8);
+  assert_equals(intersectionRect.width, 100 - 8);
+  assert_equals(intersectionRect.height, 100);
+
   assert_equals(rootBounds.left, 0);
   assert_equals(rootBounds.top, 0);
   assert_equals(rootBounds.right, document.documentElement.clientWidth);