Initial ResizeObserver implementation

third_party/WebKit/Source/core/observer/ResizeObserver.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/observer/ResizeObserver.h"
+
+#include "core/observer/ResizeObservation.h"
+#include "core/observer/ResizeObserverCallback.h"
+#include "core/observer/ResizeObserverController.h"
+
+namespace blink {
+
+ResizeObserver* ResizeObserver::create(Document& document, ResizeObserverCallback* callback)
+{
+    auto observer = new ResizeObserver(callback);
+    document.ensureResizeObserverController().addObserver(*observer);
+    return observer;
+}
+
+ResizeObserver::ResizeObserver(ResizeObserverCallback* callback)
+    : m_callback(callback)
+{
+}
+
+bool compareObservationToElement(Member<blink::ResizeObservation> observation, Element* el)

[szager1, 2016/05/23 19:49:11]

This isn't used.

[atotic1, 2016/05/25 00:15:53]

Done.

+{
+    return observation->target() == el;
+}

[szager1, 2016/05/23 19:49:11]

Add blank line.

[atotic1, 2016/05/25 00:15:53]

Done.

+bool operator==(Member<blink::ResizeObservation> observation, Element* el)

[szager1, 2016/05/23 19:49:11]

meh, this is obfuscating.  Just use (observation->target() == el) where
necessary.

[atotic1, 2016/05/25 00:15:53]

Leftover from when I was using the std::find algorithm.

Done.

+{
+    return observation->target() == el;
+}
+
+void ResizeObserver::observe(Element* target)
+{
+    if (target && m_observations.find(target) == kNotFound)

[szager1, 2016/05/23 19:49:11]

This is why, for IntersectionObserver, I did this:

class NodeIntersectionObserverData {
public:
  IntersectionObservation* getObservationFor(IntersectionObserver&);
private:
  HeapHashMap<Member<IntersectionObserver>, Member<IntersectionObservation>>
m_observations;
}

... to enable this:

void IntersectionObserver::observe(Element* target)
{
  if (target->ensureIntersectionObserverData().getObservationFor(*this))
    return;
}

The HashMap lookup will be faster than the list lookup you're doing here.

[atotic1, 2016/05/25 00:15:53]

Done.

I was not optimizing this case because we traverse the entire list on every
observation.
As observe/unobserve are hopefully called much less often, having
them perform the same traversal did not seem like a significant performance hit.

This does introduce additional complexity, because knowledge of whether 
an Element has a ResizeObservation is held in two locations, that must
be kept in sync. Note the additional complexity in unobserve() and disconnect()

+        m_observations.append(new ResizeObservation(target, this));
+}
+
+void ResizeObserver::unobserve(Element* target)
+{
+    auto pos = m_observations.find(target);
+    if (pos != kNotFound)
+        m_observations.remove(pos);
+}
+
+void ResizeObserver::disconnect()
+{
+    m_observations.clear();
+    m_activeObservations.clear();

[szager1, 2016/05/23 19:49:11]

Also:

m_callback.clear();

[atotic1, 2016/05/25 00:15:53]

That'd violate the spec, because no elements could be observed afterwards. I
followed MutationObserver behavior, that allows observe() after disconnect

+}
+
+void ResizeObserver::gatherObservations()
+{
+    m_activeObservations.clear();

[szager1, 2016/05/23 19:49:11]

This should be either ASSERT(m_activeObservations->isEmpty()), or it should be
nothing.  Clearing out possible pending notifications can't be the desired
behavior.

Also, this would need to be m_activeObservations->clear(), not
m_activeObservations.clear().

[atotic1, 2016/05/25 00:15:53]

??? m_activeObservations->clear() does not compile.

Clearing old observations is the desired behavior. 
We always test all observations, so there is no value in keeping the old ones.

[szager1, 2016/06/02 20:52:40]

It should never happen that gatherObservations() is called when there are still
un-delivered observations.

+
+    for (auto observation : m_observations) {

[szager1, 2016/05/23 19:49:11]

I'm not sure if it makes a difference, but maybe it should be "auto&" rather
that "auto" ?  I honestly don't know if it matters.

[atotic1, 2016/05/25 00:15:53]

auto& means you can't have a null.

Done.

+        if (observation->hasResized())
+            m_activeObservations.append(*observation);
+    }
+}
+
+void ResizeObserver::deliverObservations()
+{
+    if (m_activeObservations.size() == 0)
+        return;
+
+    HeapVector<Member<ResizeObserverEntry>> entries;
+
+    for (size_t i = 0; i < m_activeObservations.size(); i++) {

[szager1, 2016/05/23 19:49:11]

for (auto& observation : m_activeObservations)

[atotic1, 2016/05/25 00:15:53]

Done.

+        auto observation = m_activeObservations.at(i);
+        auto entry = new ResizeObserverEntry(observation->target());
+        entries.append(entry);
+        observation->setBroadcastSize(entry->clientWidth(), entry->clientHeight());
+    }
+    m_callback->handleEvent(entries, this);

[szager1, 2016/05/23 19:49:11]

m_activeObservations->clear();

[atotic1, 2016/05/25 00:15:53]

Done.

+}
+
+ResizeObserver::ObservationList::iterator ResizeObserver::find(Element* target)
+{
+    for (ObservationList::iterator it = m_observations.begin(); it != m_observations.end(); ++it) {
+        if ((*it)->target() == target)
+            return it;
+    }
+
+    return m_observations.end();
+}
+
+DEFINE_TRACE(ResizeObserver)
+{
+    visitor->trace(m_callback);
+    visitor->trace(m_observations);
+    visitor->trace(m_activeObservations);
+}
+
+} // namespace blink