IntersectionObserver: notify when root or target is removed.

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

Index: third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
diff --git a/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp b/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
index 94f9b6deb579dd1f04684ad394032e1be2ac631c..c8edc685b37ecdab67cf653bcef12374d76d1757 100644
--- a/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
+++ b/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
@@ -140,26 +140,32 @@ static bool isContainingBlockChainDescendant(LayoutObject* descendant, LayoutObj
 
 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;
-    LayoutObject* targetLayoutObject = targetElement->layoutObject();
+    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();
     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);
 
@@ -220,8 +226,8 @@ void IntersectionObservation::computeIntersectionObservations(DOMHighResTimeStam
             pixelSnappedIntRect(geometry.intersectionRect),
             target());
         observer().enqueueIntersectionObserverEntry(*newEntry);
+        setLastThresholdIndex(newThresholdIndex);
     }
-    setLastThresholdIndex(newThresholdIndex);
 }
 
 void IntersectionObservation::disconnect()