Intersection Observer support for OOPIF

third_party/WebKit/Source/core/dom/IntersectionObserver.cpp

Index: third_party/WebKit/Source/core/dom/IntersectionObserver.cpp
diff --git a/third_party/WebKit/Source/core/dom/IntersectionObserver.cpp b/third_party/WebKit/Source/core/dom/IntersectionObserver.cpp
index 60be0837ef1f6001b6cdf4493d28ebe1097ff8b6..51a125d6729a0841994e3358d29c57150de8083b 100644
--- a/third_party/WebKit/Source/core/dom/IntersectionObserver.cpp
+++ b/third_party/WebKit/Source/core/dom/IntersectionObserver.cpp
@@ -132,11 +132,10 @@ void parseThresholds(const DoubleOrDoubleSequence& thresholdParameter,
 
 // Returns the root Node of a given Document to use as the IntersectionObserver
 // root when no root is given.
-// TODO(szager): it doesn't support RemoteFrames, see https://crbug.com/615156
 Node* getRootNode(Document* document) {
-  Frame* mainFrame = document->frame()->tree().top();
-  if (mainFrame && mainFrame->isLocalFrame())
-    return toLocalFrame(mainFrame)->document();
+  LocalFrame* rootFrame = document->frame()->localFrameRoot();
+  if (rootFrame)
+    return rootFrame->document();
   return nullptr;
 }
 
@@ -146,11 +145,17 @@ IntersectionObserver* IntersectionObserver::create(
     const IntersectionObserverInit& observerInit,
     IntersectionObserverCallback& callback,
     ExceptionState& exceptionState) {
+  bool intersectWithRemoteAncestors = false;
   Node* root = observerInit.root();
   if (!root) {
     ExecutionContext* context = callback.getExecutionContext();
     DCHECK(context->isDocument());
     root = getRootNode(toDocument(context));
+    if (root && (toDocument(context)->frame()->tree().top()->isRemoteFrame() ||
+                 root !=
+                     toLocalFrame(toDocument(context)->frame()->tree().top())
+                         ->document()))
+      intersectWithRemoteAncestors = true;
   }
   if (!root) {
     exceptionState.throwDOMException(
@@ -169,7 +174,8 @@ IntersectionObserver* IntersectionObserver::create(
   if (exceptionState.hadException())
     return nullptr;
 
-  return new IntersectionObserver(callback, *root, rootMargin, thresholds);
+  return new IntersectionObserver(callback, *root, rootMargin, thresholds,
+                                  intersectWithRemoteAncestors);
 }
 
 IntersectionObserver* IntersectionObserver::create(
@@ -179,6 +185,11 @@ IntersectionObserver* IntersectionObserver::create(
     std::unique_ptr<EventCallback> callback,
     ExceptionState& exceptionState) {
   Node* root = getRootNode(document);
+  bool intersectWithRemoteAncestors = false;
+  if (root &&
+      (document->frame()->tree().top()->isRemoteFrame() ||
+       root != toLocalFrame(document->frame()->tree().top())->document()))
+    intersectWithRemoteAncestors = true;
   if (!root) {
     exceptionState.throwDOMException(
         HierarchyRequestError,
@@ -189,14 +200,16 @@ IntersectionObserver* IntersectionObserver::create(
   IntersectionObserverCallbackImpl* intersectionObserverCallback =
       new IntersectionObserverCallbackImpl(document, std::move(callback));
   return new IntersectionObserver(*intersectionObserverCallback, *root,
-                                  rootMargin, thresholds);
+                                  rootMargin, thresholds,
+                                  intersectWithRemoteAncestors);
 }
 
 IntersectionObserver::IntersectionObserver(
     IntersectionObserverCallback& callback,
     Node& root,
     const Vector<Length>& rootMargin,
-    const Vector<float>& thresholds)
+    const Vector<float>& thresholds,
+    bool intersectWithRemoteAncestors)
     : m_callback(&callback),
       m_root(&root),
       m_thresholds(thresholds),
@@ -204,7 +217,8 @@ IntersectionObserver::IntersectionObserver(
       m_rightMargin(Fixed),
       m_bottomMargin(Fixed),
       m_leftMargin(Fixed),
-      m_initialState(InitialState::kHidden) {
+      m_initialState(InitialState::kHidden),
+      m_intersectWithRemoteAncestors(intersectWithRemoteAncestors) {
   switch (rootMargin.size()) {
     case 0:
       break;
@@ -270,7 +284,8 @@ void IntersectionObserver::observe(Element* target,
   LocalFrame* rootFrame = m_root->document().frame();
 
   if (target->document() == rootNode()->document()) {
-    shouldReportRootBounds = true;
+    if (!m_intersectWithRemoteAncestors)
+      shouldReportRootBounds = true;
     isDOMDescendant = rootNode()->isShadowIncludingInclusiveAncestorOf(target);
   } else if (targetFrame && rootFrame) {
     shouldReportRootBounds =
@@ -445,6 +460,18 @@ void IntersectionObserver::deliver() {
   m_callback->handleEvent(entries, *this);
 }
 
+void IntersectionObserver::intersectWithRemoteAncestorsIfNeeded(

[szager1, 2016/11/16 22:11:22]

I agree with Ojan's point that this should be handled by
LayoutView::mapToVisualRectInAncestorSpace without the need for an
intersectWithRemoteAncestors flag here.

[kenrb, 2016/11/21 19:37:35]

In the most recent PS I have modified mapToVisualRectInAncestorSpace as you
suggested, and removed this method (intersectWithRemoteAncestorsIfNeeded), but
the flag is still necessary.

IntersectionObserver still needs a reference to the local frame root for most of
the things it does with the root, but then it needs to know to call
mapToVisualRectInAncestorSpace with nullptr instead of the local root pointer.
The way I have implemented it now adds a new layout object accessor to
IntersectionObserver for the call to mapToVisualRectInAncestorSpace. If there is
a different way you'd prefer to have it done, then I can change it.

[ojan, 2016/11/23 19:14:00]

Can we always pass nullptr to mapToVisualRectInAncestorSpace if no root was
provided to IntersectionObserver::create? That way we won't need any
special-casing of code for remote frames outside of the couple lines in
mapToVisualRectInAncestorSpace that intersect with
frameView()->remoteViewportIntersection().

With IntersectionObserver, the only case where we need to cross frame boundaries
is when the root is null. We can add an assert to mapToVisualRectInAncestorSpace
that the provided ancestor is in the same Document as the LayoutView to ensure
other code doesn't try to cross frame boundaries to something other than the
top-most frame.

[kenrb, 2016/11/23 20:07:28]

We still need a boolean in the IntersectionObserver constructor though, right?
We need a reference to the local frame root for other reasons (there is no path
to the layout tree, otherwise), and the information that there was originally a
null root pointer passed to create() is lost. I had previously considered
passing a second pointer in the constructor for this purpose, but then realized
that it would always either be null or equal to |root|, which seems like an
over-complicated way of saying 'boolean'. Your suggestion seems to amount to us
changing the name of the boolean to m_intersectWithViewportOnly (or something
similar), and using it also for local cross-frame intersections.

However, does this create a problem for knowing whether or not to apply root
margins? Currently this CL only sets shouldReportRootBounds when intersecting
with the viewport and the main frame is local, which seems to correctly make IO
in an OOPIF process ignore margins from the local frame root. I don't know how
to handle that if we combine those situations.

+    LayoutRect& rect) {
+  if (m_intersectWithRemoteAncestors) {
+    Document* root = toDocument(rootNode());
+    if (root->view()) {
+      LayoutRect viewportIntersectionRect(
+          root->view()->remoteViewportIntersection());
+      rect.intersect(viewportIntersectionRect);
+    }
+  }
+}
+
 DEFINE_TRACE(IntersectionObserver) {
   visitor->template registerWeakMembers<
       IntersectionObserver, &IntersectionObserver::clearWeakMembers>(this);