IntersectionObserver: second cut.

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

Index: third_party/WebKit/Source/core/dom/ElementIntersectionObserverData.cpp
diff --git a/third_party/WebKit/Source/core/dom/ElementIntersectionObserverData.cpp b/third_party/WebKit/Source/core/dom/ElementIntersectionObserverData.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..97aa92552f86aa6e6a4266c2e7ae7107862bd554
--- /dev/null
+++ b/third_party/WebKit/Source/core/dom/ElementIntersectionObserverData.cpp
@@ -0,0 +1,102 @@
+// Copyright 2015 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/ElementIntersectionObserverData.h"
+
+#include "core/dom/Document.h"
+#include "core/dom/Element.h"
+#include "core/dom/IntersectionObservation.h"
+#include "core/dom/IntersectionObserver.h"
+#include "core/dom/IntersectionObserverController.h"
+
+namespace blink {
+
+ElementIntersectionObserverData::ElementIntersectionObserverData() { }
+
+ElementIntersectionObserverData::~ElementIntersectionObserverData() { }
+
+bool ElementIntersectionObserverData::hasIntersectionObserver() const
+{
+    return !m_intersectionObservers.isEmpty();
+}
+
+bool ElementIntersectionObserverData::hasIntersectionObservation() const

[esprehn, 2015/12/22 07:50:32]

I might rename these .hasObservation() and hasObserver(), no need to restate the
type of the thing so many times if you don't want

+{
+    return !m_intersectionObservations.isEmpty();
+}
+
+bool ElementIntersectionObserverData::hasObservationFor(IntersectionObserver& observer) const
+{
+    return m_intersectionObservations.contains(&observer);
+}
+
+void ElementIntersectionObserverData::addObservation(IntersectionObservation& observation)
+{
+    m_intersectionObservations.set(observation.observer(), &observation);

[haraken, 2015/12/22 01:13:29]

Add ASSERT(!m_intersectionObservations.contains(...));

[esprehn, 2015/12/22 07:50:32]

Since this is a HashMap this should be .add() not .set() I think? No reason to
check .contains()

[szager1, 2015/12/22 07:53:41]

Done.

[szager1, 2015/12/22 08:49:49]

Done.

+}
+
+void ElementIntersectionObserverData::removeObservation(IntersectionObserver& observer)
+{
+    const auto& it = m_intersectionObservations.find(&observer);
+    if (it != m_intersectionObservations.end())

[haraken, 2015/12/22 01:13:29]

Can we change this to:

  ASSERT(it != m_intersectionObservations.end());

?

[szager1, 2015/12/22 07:53:41]

This comes from an API call:

observer.unobserve(target);

It may fail with invalid target, but it should not ASSERT.

+        it->value->disconnect();
+}
+
+void ElementIntersectionObserverData::activateValidIntersectionObservers(Element& element)
+{
+    IntersectionObserverController& controller = element.document().intersectionObserverController();
+    for (auto& observer : m_intersectionObservers) {
+        controller.addTrackedObserver(*observer);
+        observer->setActive(true);
+    }
+    for (auto& observation : m_intersectionObservations)
+        observation.value->setActive(observation.key->isDescendantOfRoot(&element));
+}
+
+void ElementIntersectionObserverData::deactivateAllIntersectionObservers(Element& element)

[haraken, 2015/12/22 01:13:29]

Just to confirm: If the Element and the Document die together,
Element::removedFrom is not called. This means that
deactivateAllIntersectionObservers is not called. This is not an issue, right?

[esprehn, 2015/12/22 07:50:32]

That's only true when Oilpan is enabled I think, otherwise we always call
removedDetachedChildren. Are we sure that all the clean up in removedFrom is
really okay to skip with Oilpan? (in general)

[szager1, 2015/12/22 07:53:41]

That should be fine, regular garbage collection should cover that.

+{
+    element.document().intersectionObserverController().removeTrackedObserversForRoot(element);
+    for (auto& observer : m_intersectionObservers)
+        observer->setActive(false);
+    for (auto& observation : m_intersectionObservations)
+        observation.value->setActive(false);
+}
+
+#if !ENABLE(OILPAN)
+void ElementIntersectionObserverData::dispose()
+{
+    HeapVector<Member<IntersectionObservation>> toDisconnect;
+    copyValuesToVector(m_intersectionObservations, toDisconnect);
+    for (auto& observation : toDisconnect)
+        observation->disconnect();
+    ASSERT(m_intersectionObservations.isEmpty());
+
+    HeapVector<Member<IntersectionObserver>> toDispose;
+    copyToVector(m_intersectionObservers, toDispose);
+    for (auto& observer : toDispose)
+        observer->dispose();
+    ASSERT(m_intersectionObservers.isEmpty());
+
+    m_weakPointerFactory.clear();
+}
+#endif
+
+WeakPtr<Element> ElementIntersectionObserverData::createWeakPtr(Element* element)
+{
+#if ENABLE(OILPAN)
+    return element;

[esprehn, 2015/12/22 07:50:32]

does this actually work? The return type doesn't need to be something like
WeakPtrWillBeRawPtr?

[szager1, 2015/12/22 08:49:49]

Good catch!

+#else
+    if (!m_weakPointerFactory)
+        m_weakPointerFactory = adoptPtrWillBeNoop(new WeakPtrFactory<Element>(element));
+    return m_weakPointerFactory->createWeakPtr();
+#endif
+}
+
+DEFINE_TRACE(ElementIntersectionObserverData)
+{
+    visitor->trace(m_intersectionObservers);
+    visitor->trace(m_intersectionObservations);
+}
+
+} // namespace blink {