Simplify ElementVisibilityObserver implementation.

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

Index: third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp
diff --git a/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp
index 05f571387aa193c7b7269add45af7a5d4eb69c03..e109e1b5546b0a21daf2ba60f8064f8b08037af4 100644
--- a/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp
+++ b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp
@@ -5,27 +5,33 @@
 #include "core/dom/ElementVisibilityObserver.h"
 
 #include "core/dom/Element.h"
+#include "core/dom/IntersectionObserver.h"
 #include "core/dom/IntersectionObserverEntry.h"
 #include "wtf/Functional.h"
 
 namespace blink {
 
-ElementVisibilityObserver::ElementVisibilityObserver(Element* element, std::unique_ptr<VisibilityCallback> callback)
-    : m_element(element)
-    , m_callback(std::move(callback))
+ElementVisibilityObserver* ElementVisibilityObserver::create(Element* element, Client* client)
 {
+    ElementVisibilityObserver* observer = new ElementVisibilityObserver(client);
+    observer->start(element);
+    return observer;
 }
 
-ElementVisibilityObserver::~ElementVisibilityObserver() = default;
+ElementVisibilityObserver::ElementVisibilityObserver(Client* client)
+    : m_client(client)
+{
+    DCHECK(m_client);
+}
 
-void ElementVisibilityObserver::start()
+void ElementVisibilityObserver::start(Element* element)
 {
     DCHECK(!m_intersectionObserver);
     m_intersectionObserver = IntersectionObserver::create(
-        Vector<Length>(), Vector<float>({std::numeric_limits<float>::min()}), &m_element->document(),
+        Vector<Length>(), Vector<float>({std::numeric_limits<float>::min()}), &element->document(),
         WTF::bind(&ElementVisibilityObserver::onVisibilityChanged, wrapWeakPersistent(this)));

[sof, 2016/07/25 05:16:26]

I'd prefer not using a WeakPersistent<> here also.

[sof, 2016/07/25 08:16:33]

Now done - having two heap objects linked together by way of an off-heap
callback (that additionally requires an on-heap intermediary + a
WeakPersistent<>), seems unnecessary.

     DCHECK(m_intersectionObserver);
-    m_intersectionObserver->observe(m_element.release());
+    m_intersectionObserver->observe(element);
 }
 
 void ElementVisibilityObserver::stop()
@@ -34,18 +40,21 @@ void ElementVisibilityObserver::stop()
 
     m_intersectionObserver->disconnect();
     m_intersectionObserver = nullptr;
+    // Client will no longer be called upon, release.
+    m_client = nullptr;
 }
 
-DEFINE_TRACE(ElementVisibilityObserver)
+void ElementVisibilityObserver::onVisibilityChanged(const HeapVector<Member<IntersectionObserverEntry>>& entries)
 {
-    visitor->trace(m_element);
-    visitor->trace(m_intersectionObserver);
+    DCHECK(m_client);
+    bool isVisible = entries.last()->intersectionRatio() > 0.f;
+    m_client->onVisibilityChanged(isVisible);
 }
 
-void ElementVisibilityObserver::onVisibilityChanged(const HeapVector<Member<IntersectionObserverEntry>>& entries)
+DEFINE_TRACE(ElementVisibilityObserver)
 {
-    bool isVisible = entries.last()->intersectionRatio() > 0.f;
-    (*m_callback.get())(isVisible);
+    visitor->trace(m_client);
+    visitor->trace(m_intersectionObserver);
 }
 
 } // namespace blink