IntersectionObserver: second cut.

third_party/WebKit/Source/core/dom/IntersectionObserver.h

+// 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.
+
+#ifndef IntersectionObserver_h
+#define IntersectionObserver_h
+
+#include "bindings/core/v8/ScriptWrappable.h"
+#include "core/dom/Element.h"
+#include "core/dom/IntersectionObservation.h"
+#include "core/dom/IntersectionObserverEntry.h"
+#include "platform/heap/Handle.h"
+#include "wtf/HashSet.h"
+#include "wtf/RefCounted.h"
+#include "wtf/Vector.h"
+
+namespace blink {
+
+class ExceptionState;
+class IntersectionObserverCallback;
+class IntersectionObserverInit;
+
+typedef HeapVector<Member<IntersectionObserverEntry>> IntersectionObserverEntryVector;

[esprehn, 2015/12/12 00:14:16]

remove the typedefs, the name is nearly as long as the actual type and just
obscures the code, you can use auto in loops to avoid typing it

[szager1, 2015/12/16 19:15:36]

Done.

+
+class IntersectionObserver final : public GarbageCollectedFinalized<IntersectionObserver>, public ScriptWrappable {
+    DEFINE_WRAPPERTYPEINFO();
+public:
+    typedef HeapHashSet<Member<IntersectionObserver>> HashSet;
+    typedef HeapHashSet<WeakMember<IntersectionObserver>> WeakHashSet;

[esprehn, 2015/12/12 00:14:16]

I'd remove these and just type out the types in the right spots, this also
prevents forward decls in places.

[szager1, 2015/12/16 19:15:37]

Done.

+
+    static IntersectionObserver* create(const IntersectionObserverInit&, IntersectionObserverCallback*, ExceptionState&);
+    static void resumeSuspendedObservers();
+
+    // API methods
+    void observe(Element*, ExceptionState&);
+    void unobserve(Element*, ExceptionState&);
+    void disconnect();
+    IntersectionObserverEntryVector takeRecords();
+
+    Element* root() { return m_root.get(); }
+    void computeIntersectionObservations(int);
+    void disconnect(IntersectionObservation&);
+    bool isDescendantOfRoot(Element*) const;

[esprehn, 2015/12/12 00:14:16]

reference

[szager1, 2015/12/16 19:15:36]

I'd argue that in this particular case, using a reference doesn't make sense. 
The body of the method reassigns to the Element* parameter as it walks up the
frame tree, so the method is already doing null checks.

+    void enqueueIntersectionObserverEntry(IntersectionObserverEntry*);

[esprehn, 2015/12/12 00:14:16]

reference

[szager1, 2015/12/16 19:15:36]

Done.

+    void applyRootMargin(LayoutRect&) const;
+    size_t firstThresholdGreaterThan(float ratio) const;
+    bool shouldBeSuspended() const;
+    void deliver();
+    void setActive(bool);
+    bool hasPercentMargin() const;
+    const Length& topMargin() const { return m_topMargin; }
+    const Length& rightMargin() const { return m_rightMargin; }
+    const Length& bottomMargin() const { return m_bottomMargin; }
+    const Length& leftMargin() const { return m_leftMargin; }
+
+    DECLARE_TRACE();
+
+private:
+    explicit IntersectionObserver(IntersectionObserverCallback*, Element*, const Vector<Length>&, const Vector<float>&);

[esprehn, 2015/12/12 00:14:16]

reference for most of these I think? Also the two vectors need argument names

[szager1, 2015/12/16 19:15:36]

Done.

+
+    void checkRootAndDetachIfNecessary();
+

[esprehn, 2015/12/12 00:14:16]

Needed() not Necessary.

[ojan, 2015/12/12 01:06:29]

Lets just call this checkRootAndDetach. We have too many gratuitous "IfNeeded"
throughout the code. It's gotten to a point where it's meaningless.

[esprehn, 2015/12/12 01:49:52]

Actually looking at this function just call it detachRoot(), if there's a root,
it always "detaches", and an early return when there's no room is very sensible.

[szager1, 2015/12/16 19:15:36]

That's not actually what it does.  Rather, it checks to see if the root has been
gc'ed, and if so, it basically immolates the observer and -- critically --
releases its reference to the callback execution context.

I did s/Necessary/Needed/, but I didn't rename it to detachRoot, because I think
that's misleading.

+    Member<IntersectionObserverCallback> m_callback;
+    WeakPtrWillBeWeakMember<Element> m_root;
+    Member<IntersectionObservation::WeakHashSet> m_observations;

[esprehn, 2015/12/12 00:14:16]

just write the types

[szager1, 2015/12/16 19:15:37]

Done.

+    Member<IntersectionObserverEntryVector> m_entries;
+    Vector<float> m_thresholds;
+    Length m_topMargin;
+    Length m_rightMargin;
+    Length m_bottomMargin;
+    Length m_leftMargin;

[esprehn, 2015/12/12 00:14:16]

wow these things are big... you definitely won't want an observer per widget in
your page

[szager1, 2015/12/16 19:15:36]

That would be a very strange use case.  I agree that IntersectionObserver can be
added to the long list of "things you wouldn't want to do for every widget in
your page."

+};
+
+} // namespace blink
+
+#endif // IntersectionObserver_h