IntersectionObserver: notify when root or target is removed.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/dom/IntersectionObservation.h"
 
 #include "core/dom/ElementRareData.h"
 #include "core/dom/IntersectionObserver.h"
 #include "core/frame/FrameView.h"
 #include "core/layout/LayoutBox.h"
@@ skipping 122 matching lines @@
     if (ancestorFrame != descendantFrame)
         return false;
 
     while (descendant && descendant != ancestor)
         descendant = descendant->containingBlock();
     return descendant;
 }
 
 bool IntersectionObservation::computeGeometry(IntersectionGeometry& geometry) const
 {
-    // Pre-oilpan, there will be a delay between the time when the target Element gets deleted
-    // (because its ref count dropped to zero) and when this IntersectionObservation gets
-    // deleted (during the next gc run, because the target Element is the only thing keeping
-    // the IntersectionObservation alive).  During that interval, we need to check that m_target
-    // hasn't been cleared.
+    // 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.
     Element* targetElement = target();
-    if (!targetElement || !targetElement->inShadowIncludingDocument())
+    if (!targetElement)
         return false;
+    if (!targetElement->inShadowIncludingDocument())
+        return true;

[ojan, 2016/04/15 18:19:08]

Nit: I think this code would be more self-documenting if the bool were an
out-param instead of a return value. As it is, I had a hard time figuring out
what the return value meant. I think it means shouldAddEntry or something like
that?

[szager1, 2016/04/15 19:47:23]

The return value means "should we *ever* send a notification."  It only returns
false when the observation is in an unrecoverable state (e.g., root or target
has been deleted, or root or target is an invalid node type).

[ojan, 2016/04/15 22:40:52]

All the more reason to use an out param with a clear name. :)

+    DCHECK(m_observer);
+    Element* rootElement = m_observer->root();
+    if (rootElement && !rootElement->inShadowIncludingDocument())
+        return true;
+
+    LayoutObject* rootLayoutObject = m_observer->rootLayoutObject();
+    if (!rootLayoutObject || !rootLayoutObject->isBoxModelObject())
+        return false;
+    // TODO(szager): Support SVG
     LayoutObject* targetLayoutObject = targetElement->layoutObject();
-    DCHECK(m_observer);
-    LayoutObject* rootLayoutObject = m_observer->rootLayoutObject();
-    // TODO(szager): Support SVG
     if (!targetLayoutObject)
         return false;
     if (!targetLayoutObject->isBoxModelObject() && !targetLayoutObject->isText())
         return false;
-    if (!rootLayoutObject || !rootLayoutObject->isBoxModelObject())
-        return false;
     if (!isContainingBlockChainDescendant(targetLayoutObject, rootLayoutObject))
-        return false;
+        return true;
 
     initializeGeometry(geometry);
 
     clipToRoot(geometry);
 
     mapTargetRectToTargetFrameCoordinates(geometry.targetRect);
 
     if (geometry.doesIntersect)
         mapRootRectToTargetFrameCoordinates(geometry.intersectionRect);
     else
@@ skipping 40 matching lines @@
         IntRect snappedRootBounds = pixelSnappedIntRect(geometry.rootRect);
         IntRect* rootBoundsPointer = m_shouldReportRootBounds ? &snappedRootBounds : nullptr;
         IntersectionObserverEntry* newEntry = new IntersectionObserverEntry(
             timestamp,
             newVisibleRatio,
             pixelSnappedIntRect(geometry.targetRect),
             rootBoundsPointer,
             pixelSnappedIntRect(geometry.intersectionRect),
             target());
         observer().enqueueIntersectionObserverEntry(*newEntry);
+        setLastThresholdIndex(newThresholdIndex);
     }
-    setLastThresholdIndex(newThresholdIndex);
 }
 
 void IntersectionObservation::disconnect()
 {
     IntersectionObserver* observer = m_observer;
     clearRootAndRemoveFromTarget();
     observer->removeObservation(*this);
 }
 
 void IntersectionObservation::clearRootAndRemoveFromTarget()
 {
     if (m_target)
         target()->ensureIntersectionObserverData().removeObservation(observer());
     m_observer.clear();
 }
 
 DEFINE_TRACE(IntersectionObservation)
 {
     visitor->trace(m_observer);
     visitor->trace(m_target);
 }
 
 } // namespace blink