Refactor IntersectionGeometry class and move it to core/layout.

third_party/WebKit/Source/core/layout/IntersectionGeometry.cpp

Index: third_party/WebKit/Source/core/layout/IntersectionGeometry.cpp
diff --git a/third_party/WebKit/Source/core/dom/IntersectionGeometry.cpp b/third_party/WebKit/Source/core/layout/IntersectionGeometry.cpp
similarity index 45%
rename from third_party/WebKit/Source/core/dom/IntersectionGeometry.cpp
rename to third_party/WebKit/Source/core/layout/IntersectionGeometry.cpp
index 1956b7b62aeda753729dc3e157b826fb8666bd11..20b50bb0dab946872b53d77fd51e8315606e3107 100644
--- a/third_party/WebKit/Source/core/dom/IntersectionGeometry.cpp
+++ b/third_party/WebKit/Source/core/layout/IntersectionGeometry.cpp
@@ -2,12 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "core/dom/IntersectionGeometry.h"
+#include "core/layout/IntersectionGeometry.h"
 
-#include "core/dom/Element.h"
-#include "core/dom/ElementRareData.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
+#include "core/html/HTMLFrameOwnerElement.h"
 #include "core/layout/LayoutBox.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/api/LayoutAPIShim.h"
@@ -22,31 +21,44 @@ bool isContainingBlockChainDescendant(LayoutObject* descendant,
                                       LayoutObject* ancestor) {
   LocalFrame* ancestorFrame = ancestor->document().frame();
   LocalFrame* descendantFrame = descendant->document().frame();
+  if (!ancestorFrame)
+    return false;
 
-  if (ancestor->isLayoutView())
-    return descendantFrame && descendantFrame->tree().top() == ancestorFrame;
+  while (descendantFrame && descendantFrame != ancestorFrame) {

[ojan, 2016/12/10 01:18:26]

When is descendantFrame != ancestorFrame without the ancestor being the implicit
root?

[szager1, 2016/12/10 01:47:11]

If the observer was created with an explicit root, and then the target was
removed from the document and inserted into a different document.

[ojan, 2016/12/12 19:53:31]

Do we deliver intersection records for that? I thought we got rid of
same-origin, cross-frame record delivery entirely. As per
https://github.com/WICG/IntersectionObserver/issues/87. So, you should be able
to just return false for the non-implicit root case where
descendantFrame!=ancestorFrame (which is this code, right?).

[szager1, 2016/12/12 20:16:11]

As discussed in-person, this function should not walk the frame tree at all,
since it only gets called in the explicit-root case, which must be
same-document.  I removed all of the frame-walking code.

+    HTMLFrameOwnerElement* owner = descendant->document().localOwner();
+    if (!owner)
+      return false;
+    descendant = owner->layoutObject();
+    if (!descendant)
+      return false;
+    descendantFrame = descendant->document().frame();
+  }
 
-  if (ancestorFrame != descendantFrame)
+  if (!descendantFrame)
     return false;
 
+  if (ancestor->isLayoutView())
+    return true;
+
   while (descendant && descendant != ancestor)
     descendant = descendant->containingBlock();
   return descendant;
 }
 
 void mapRectUpToDocument(LayoutRect& rect,
-                         const LayoutObject& layoutObject,
+                         const LayoutObject& descendant,
                          const Document& document) {
-  FloatQuad mappedQuad = layoutObject.localToAbsoluteQuad(
-      FloatQuad(FloatRect(rect)), UseTransforms | ApplyContainerFlip);
+  FloatQuad mappedQuad = descendant.localToAncestorQuad(
+      FloatQuad(FloatRect(rect)), document.layoutView(),
+      UseTransforms | ApplyContainerFlip);
   rect = LayoutRect(mappedQuad.boundingBox());
 }
 
 void mapRectDownToDocument(LayoutRect& rect,
-                           LayoutBoxModelObject& layoutObject,
+                           LayoutBoxModelObject* ancestor,
                            const Document& document) {
   FloatQuad mappedQuad = document.layoutView()->ancestorToLocalQuad(
-      &layoutObject, FloatQuad(FloatRect(rect)),
+      ancestor, FloatQuad(FloatRect(rect)),
       UseTransforms | ApplyContainerFlip | TraverseDocumentBoundaries);
   rect = LayoutRect(mappedQuad.boundingBox());
 }
@@ -60,62 +72,64 @@ LayoutUnit computeMargin(const Length& length, LayoutUnit referenceLength) {
   return LayoutUnit(length.intValue());
 }
 
+LayoutView* localRootView(Element& element) {
+  LocalFrame* frame = element.document().frame();
+  frame = frame ? frame->localFrameRoot() : nullptr;

[ojan, 2016/12/10 01:18:26]

Can you create a new local variable here, e.g. frameRoot? That would make this
code more clear at a glance.

[szager1, 2016/12/10 01:47:11]

Done.

+  return frame ? frame->contentLayoutObject() : nullptr;
+}
+
 }  // namespace
 
-IntersectionGeometry::IntersectionGeometry(
-    Node* root,
-    Element* target,
-    const Vector<Length>& rootMargin,
-    ReportRootBounds shouldReportRootBounds)
-    : m_root(root),
-      m_target(target),
+IntersectionGeometry::IntersectionGeometry(Element* root,
+                                           Element& target,
+                                           const Vector<Length>& rootMargin,
+                                           bool shouldReportRootBounds)
+    : m_root(root ? root->layoutObject() : localRootView(target)),
+      m_target(target.layoutObject()),
       m_rootMargin(rootMargin),
-      m_shouldReportRootBounds(shouldReportRootBounds) {
-  DCHECK(m_target);
-  DCHECK(rootMargin.isEmpty() || rootMargin.size() == 4);
-}
+      m_doesIntersect(0),
+      m_shouldReportRootBounds(shouldReportRootBounds),
+      m_rootIsImplicit(root ? 0 : 1),

[ojan, 2016/12/10 01:18:26]

Does !root work? I feel like I haven't seen this ternary pattern to set bools
elsewhere.

[szager1, 2016/12/10 01:47:10]

Switched to !root.

+      m_isValid(initializeGeometry(root, target)) {}
 
 IntersectionGeometry::~IntersectionGeometry() {}
 
-Element* IntersectionGeometry::root() const {
-  if (m_root && !m_root->isDocumentNode())
-    return toElement(m_root);
-  return nullptr;
-}
-
-LayoutObject* IntersectionGeometry::getRootLayoutObject() const {
-  DCHECK(m_root);
-  if (m_root->isDocumentNode()) {
-    return LayoutAPIShim::layoutObjectFrom(
-        toDocument(m_root)->layoutViewItem());
-  }
-  return toElement(m_root)->layoutObject();
-}
-
-void IntersectionGeometry::initializeGeometry() {
+bool IntersectionGeometry::initializeGeometry(Element* root, Element& target) {

[ojan, 2016/12/10 01:18:26]

This bool return value isn't super clear.

How would you feel about splitting this up into hasValidGeometry and
initializeGeometry and only calling the latter if m_isValid?

[szager1, 2016/12/10 01:47:11]

Done.

+  DCHECK(m_rootMargin.isEmpty() || m_rootMargin.size() == 4);
+  if (root && !root->isConnected())
+    return false;
+  if (!m_root || !m_root->isBox())
+    return false;
+  if (!target.isConnected())
+    return false;
+  if (!m_target || (!m_target->isBoxModelObject() && !m_target->isText()))
+    return false;
+  if (root && !isContainingBlockChainDescendant(m_target, m_root))
+    return false;
   initializeTargetRect();
   m_intersectionRect = m_targetRect;
   initializeRootRect();
-  m_doesIntersect = true;
+  return true;
 }
 
 void IntersectionGeometry::initializeTargetRect() {
-  LayoutObject* targetLayoutObject = m_target->layoutObject();
-  DCHECK(targetLayoutObject && targetLayoutObject->isBoxModelObject());
-  m_targetRect = LayoutRect(
-      toLayoutBoxModelObject(targetLayoutObject)->borderBoundingBox());
+  m_targetRect =
+      LayoutRect(toLayoutBoxModelObject(target())->borderBoundingBox());
 }
 
 void IntersectionGeometry::initializeRootRect() {
-  LayoutObject* rootLayoutObject = getRootLayoutObject();
-  if (rootLayoutObject->isLayoutView()) {
-    m_rootRect = LayoutRect(
-        toLayoutView(rootLayoutObject)->frameView()->visibleContentRect());
-  } else if (rootLayoutObject->isBox() && rootLayoutObject->hasOverflowClip()) {
-    m_rootRect = LayoutRect(toLayoutBox(rootLayoutObject)->contentBoxRect());
+  // TODO(szager): In OOPIF, m_root will be the LayoutView of the
+  // localFrameRoot().  Once viewport intersection support lands,
+  // add a call to mapToVisualRectInAncestorSpace to map the rect up to
+  // top-level frame coordinates.
+  if (m_root->isLayoutView()) {
+    m_rootRect =
+        LayoutRect(toLayoutView(m_root)->frameView()->visibleContentRect());
+  } else if (m_root->isBox() && m_root->hasOverflowClip()) {
+    m_rootRect = LayoutRect(toLayoutBox(m_root)->contentBoxRect());
   } else {
-    m_rootRect = LayoutRect(
-        toLayoutBoxModelObject(rootLayoutObject)->borderBoundingBox());
+    m_rootRect =
+        LayoutRect(toLayoutBoxModelObject(m_root)->borderBoundingBox());
   }
   applyRootMargin();
 }
@@ -140,102 +154,71 @@ void IntersectionGeometry::applyRootMargin() {
 void IntersectionGeometry::clipToRoot() {
   // Map and clip rect into root element coordinates.
   // TODO(szager): the writing mode flipping needs a test.
-  LayoutBox* rootLayoutObject = toLayoutBox(getRootLayoutObject());
-  LayoutObject* targetLayoutObject = m_target->layoutObject();
-
-  m_doesIntersect = targetLayoutObject->mapToVisualRectInAncestorSpace(
-      rootLayoutObject, m_intersectionRect, EdgeInclusive);
-  if (rootLayoutObject->hasOverflowClip())
-    m_intersectionRect.move(-rootLayoutObject->scrolledContentOffset());
-
+  // TODO(szager): Once the OOPIF viewport intersection code lands,
+  // use nullptr for ancestor to map to the top frame.
+  LayoutBox* ancestor = toLayoutBox(m_root);
+  m_doesIntersect = m_target->mapToVisualRectInAncestorSpace(
+      ancestor, m_intersectionRect, EdgeInclusive);
+  if (ancestor && ancestor->hasOverflowClip())
+    m_intersectionRect.move(-ancestor->scrolledContentOffset());
   if (!m_doesIntersect)
     return;
   LayoutRect rootClipRect(m_rootRect);
-  rootLayoutObject->flipForWritingMode(rootClipRect);
+  if (ancestor)
+    ancestor->flipForWritingMode(rootClipRect);
   m_doesIntersect &= m_intersectionRect.inclusiveIntersect(rootClipRect);
 }
 
 void IntersectionGeometry::mapTargetRectToTargetFrameCoordinates() {
-  LayoutObject& targetLayoutObject = *m_target->layoutObject();
   Document& targetDocument = m_target->document();
   LayoutSize scrollPosition =
       LayoutSize(targetDocument.view()->getScrollOffset());
-  mapRectUpToDocument(m_targetRect, targetLayoutObject, targetDocument);
+  mapRectUpToDocument(m_targetRect, *m_target, targetDocument);
   m_targetRect.move(-scrollPosition);
 }
 
 void IntersectionGeometry::mapRootRectToRootFrameCoordinates() {
-  LayoutObject& rootLayoutObject = *getRootLayoutObject();
-  Document& rootDocument = rootLayoutObject.document();
+  Document& rootDocument = m_root->document();
+  if (!rootIsImplicit())
+    mapRectUpToDocument(m_rootRect, *m_root, rootDocument);
+  // TODO(szager): When OOPIF support lands, this scroll offset adjustment
+  // will probably be wrong.
   LayoutSize scrollPosition =
       LayoutSize(rootDocument.view()->getScrollOffset());
-  mapRectUpToDocument(m_rootRect, rootLayoutObject,
-                      rootLayoutObject.document());
   m_rootRect.move(-scrollPosition);
 }
 
-void IntersectionGeometry::mapRootRectToTargetFrameCoordinates() {
-  LayoutObject& rootLayoutObject = *getRootLayoutObject();
+void IntersectionGeometry::mapIntersectionRectToTargetFrameCoordinates() {
   Document& targetDocument = m_target->document();
-  LayoutSize scrollPosition =
-      LayoutSize(targetDocument.view()->getScrollOffset());
-
-  if (&targetDocument == &rootLayoutObject.document()) {
-    mapRectUpToDocument(m_intersectionRect, rootLayoutObject, targetDocument);
+  if (rootIsImplicit()) {
+    LocalFrame* targetFrame = targetDocument.frame();
+    Frame* rootFrame = targetFrame->tree().top();
+    LayoutSize scrollPosition =
+        LayoutSize(targetDocument.view()->getScrollOffset());
+    if (targetFrame != rootFrame)
+      mapRectDownToDocument(m_intersectionRect, nullptr, targetDocument);
+    m_intersectionRect.move(-scrollPosition);
   } else {
-    mapRectDownToDocument(m_intersectionRect,
-                          toLayoutBoxModelObject(rootLayoutObject),
-                          targetDocument);
+    LayoutSize scrollPosition =
+        LayoutSize(targetDocument.view()->getScrollOffset());
+    mapRectUpToDocument(m_intersectionRect, *m_root, m_root->document());
+    m_intersectionRect.move(-scrollPosition);
   }
-
-  m_intersectionRect.move(-scrollPosition);
 }
 
 void IntersectionGeometry::computeGeometry() {
-  // In the first few lines here, before initializeGeometry is called, "return
-  // true" effectively means "if the previous observed state was that root and
-  // target were intersecting, then generate a notification indicating that they
-  // are no longer intersecting."  This happens, for example, when root or
-  // target is removed from the DOM tree and not reinserted before the next
-  // frame is generated, or display:none is set on the root or target.
-  if (!m_target->isConnected())
-    return;
-  Element* rootElement = root();
-  if (rootElement && !rootElement->isConnected())
-    return;
-
-  LayoutObject* rootLayoutObject = getRootLayoutObject();
-  if (!rootLayoutObject || !rootLayoutObject->isBoxModelObject())
-    return;
-  // TODO(szager): Support SVG
-  LayoutObject* targetLayoutObject = m_target->layoutObject();
-  if (!targetLayoutObject)
+  if (!isValid())
     return;
-  if (!targetLayoutObject->isBoxModelObject() && !targetLayoutObject->isText())
-    return;
-  if (!isContainingBlockChainDescendant(targetLayoutObject, rootLayoutObject))
-    return;
-
-  initializeGeometry();
-
   clipToRoot();
-
   mapTargetRectToTargetFrameCoordinates();
-
   if (m_doesIntersect)
-    mapRootRectToTargetFrameCoordinates();
+    mapIntersectionRectToTargetFrameCoordinates();
   else
     m_intersectionRect = LayoutRect();
-
   // Small optimization: if we're not going to report root bounds, don't bother
   // transforming them to the frame.
-  if (m_shouldReportRootBounds == ReportRootBounds::kShouldReportRootBounds)
+  if (shouldReportRootBounds())
     mapRootRectToRootFrameCoordinates();
 }
 
-DEFINE_TRACE(IntersectionGeometry) {
-  visitor->trace(m_root);
-  visitor->trace(m_target);
-}
-
 }  // namespace blink