IntersectionObserver: use nullptr for implicit root.

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

 // 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.
 
 #ifndef IntersectionObserver_h
 #define IntersectionObserver_h
 
 #include "bindings/core/v8/ExceptionStatePlaceholder.h"
 #include "bindings/core/v8/ScriptWrappable.h"
 #include "core/dom/IntersectionObservation.h"
 #include "core/dom/IntersectionObserverEntry.h"
 #include "platform/Length.h"
 #include "platform/heap/Handle.h"
 #include "wtf/HashSet.h"
 #include "wtf/Vector.h"
 
 namespace blink {
 
 class Document;
 class Element;
 class ExceptionState;
 class IntersectionObserverCallback;
 class IntersectionObserverInit;
+class LayoutObject;
 
 class CORE_EXPORT IntersectionObserver final
     : public GarbageCollectedFinalized<IntersectionObserver>,
       public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
   using EventCallback =
       Function<void(const HeapVector<Member<IntersectionObserverEntry>>&),
                WTF::SameThreadAffinity>;
 
   // Defines the assumed initial state of the observed element. If the actual
   // state is the same as the initial state, then no observation will be
   // delivered. kAuto means the initial observation will always get sent.
-  enum class InitialState {
+  enum InitialState {
     // TODO(skyostil): Add support for kVisible.
-    kAuto,
-    kHidden,
+    kAuto = 0,
+    kHidden = 1,
   };
 
   static IntersectionObserver* create(const IntersectionObserverInit&,
                                       IntersectionObserverCallback&,
                                       ExceptionState&);
   static IntersectionObserver* create(const Vector<Length>& rootMargin,
                                       const Vector<float>& thresholds,
                                       Document*,
                                       std::unique_ptr<EventCallback>,
                                       ExceptionState& = ASSERT_NO_EXCEPTION);
   static void resumeSuspendedObservers();
 
   // API methods.
   void observe(Element*, ExceptionState& = ASSERT_NO_EXCEPTION);
   void unobserve(Element*, ExceptionState& = ASSERT_NO_EXCEPTION);
   void disconnect(ExceptionState& = ASSERT_NO_EXCEPTION);
   HeapVector<Member<IntersectionObserverEntry>> takeRecords(ExceptionState&);
 
   // API attributes.
-  Element* root() const;
+  Element* root() const { return m_root.get(); }
   String rootMargin() const;
   const Vector<float>& thresholds() const { return m_thresholds; }
 
-  Node* rootNode() const { return m_root.get(); }
+  bool rootIsImplicit() const { return m_rootIsImplicit; }

[skobes, 2016/12/08 01:08:56]

Add comments explaining what this means.

[szager1, 2016/12/08 19:39:58]

Done.

+  Document& trackingDocument() const;

[skobes, 2016/12/08 01:08:56]

The observer does the tracking, right?  So this should be trackedDocument or
documentBeingTracked.

[szager1, 2016/12/08 19:39:58]

I added a comment to clarify this: "this is the document which is responsible
for running computeIntersectionObservations at frame generation time."  The
tracking document is not involved in the actual intersection calculation.

+  LayoutObject* rootLayoutObject() 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; }
   void computeIntersectionObservations();
   void enqueueIntersectionObserverEntry(IntersectionObserverEntry&);
   unsigned firstThresholdGreaterThan(float ratio) const;
   void deliver();
   void removeObservation(IntersectionObservation&);
   bool hasEntries() const { return m_entries.size(); }
   const HeapLinkedHashSet<WeakMember<IntersectionObservation>>& observations()
       const {
     return m_observations;
   }
 
   // Set the assumed initial state of the observed element. Note that this can
   // only be set before calling observe().
   // TODO(skyostil): Move this setting to IntersectionObserverInit once the API
   // is finalized.
   void setInitialState(InitialState);
 
   DECLARE_TRACE();
 
  private:
   explicit IntersectionObserver(IntersectionObserverCallback&,
-                                Node&,
+                                Element*,
                                 const Vector<Length>& rootMargin,
                                 const Vector<float>& thresholds);
   void clearWeakMembers(Visitor*);
 
+  // Did the explicit root Element get deleted?

[skobes, 2016/12/08 01:08:56]

The answer to this question is the inverse of the return value.  Perhaps begin
with "// True unless ..."

[szager1, 2016/12/08 19:39:58]

Done.

+  bool rootIsValid() const;
+
   Member<IntersectionObserverCallback> m_callback;
-  WeakMember<Node> m_root;
+  WeakMember<Element> m_root;

[skobes, 2016/12/08 01:08:56]

What happens if we want to track a Document in an iframe?

[szager1, 2016/12/08 19:39:58]

The only choices are to track an Element, or to track the top-level frame's
viewport (the "implict root").  Previously, when tracking the implicit root, we
would keep a reference to the top-level frame's Document node in m_root; but
that breaks down in the OOPIF case, because the top-level frame might be in a
different process.

[skobes, 2016/12/08 20:02:43]

So which element is m_root set to if we want to track an iframe's viewport?  Is
it its document.scrollingElement, the owning <iframe> element in the parent
document, or something else?

We're losing the ability for m_root to be a Document by making it an Element so
I'm just trying to understand how that use case is satisfied.

[szager1, 2016/12/08 20:12:11]

If you read through the code paths that get to the IntersectionObserver
constructor, the only way that m_root could be a Document is the implicit root
case.  This restriction is part of the IntersectionObserver spec:

"An IntersectionObserver with a root Element can observe any target Element that
is a descendant of the root in the containing block chain.

An IntersectionObserver with no root Element will automatically observe
intersections with the implicit root..."

If you want to track the intersection between a target in an iframe and that
iframe's viewport, then you would using iframe.contentDocument.scrollingElement
as the root.

   HeapLinkedHashSet<WeakMember<IntersectionObservation>> m_observations;
   HeapVector<Member<IntersectionObserverEntry>> m_entries;
   Vector<float> m_thresholds;
   Length m_topMargin;
   Length m_rightMargin;
   Length m_bottomMargin;
   Length m_leftMargin;
-  InitialState m_initialState;
+  unsigned m_rootIsImplicit : 1;
+  unsigned m_initialState : 1;

[skobes, 2016/12/08 01:08:56]

Why is the type not InitialState?

[szager1, 2016/12/08 19:39:58]

Just for efficient struct packing.  I think the enum values are still useful,
because they document what the different states mean.

[skobes, 2016/12/08 20:02:43]

C++ allows enum bitfields, so you should be able to do

  InitialState m_initialState : 1;

[szager1, 2016/12/08 20:12:11]

That gives a presubmit error:

$ git cl uplodad
Running presubmit upload checks ...

** Presubmit ERRORS **
check-webkit-style failed
  third_party/WebKit/Source/core/dom/IntersectionObserver.h:122:  Member
m_initialState of class CORE_EXPORT defined as a bitfield of type InitialiState.
Please declare all bitfields as unsigned.  [runtime/bitfields] [4]

[skobes, 2016/12/08 21:12:21]

"InitialiState" looks like a typo.

[szager1, 2016/12/08 21:30:58]

Same error.

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