Fixed up root scroller API to be more webby

third_party/WebKit/Source/core/page/scrolling/RootScroller.h

Index: third_party/WebKit/Source/core/page/scrolling/RootScroller.h
diff --git a/third_party/WebKit/Source/core/page/scrolling/RootScroller.h b/third_party/WebKit/Source/core/page/scrolling/RootScroller.h
index e6ded8f45cd0d84cc57ef23fedd75c860e049ecc..516290cf0c18ba7ddbf3856341559754d09bd970 100644
--- a/third_party/WebKit/Source/core/page/scrolling/RootScroller.h
+++ b/third_party/WebKit/Source/core/page/scrolling/RootScroller.h
@@ -10,54 +10,89 @@
 
 namespace blink {
 
+class Document;
 class Element;
-class FrameHost;
+class OverscrollController;
+class TopControls;
 class ViewportScrollCallback;
 
-// This class manages the root scroller associated with the top-level document.
-// The root scroller causes top controls movement, overscroll effects and
-// prevents chaining up further in the DOM. It can be set using
-// document.setRootScroller. By default, the rootScroller is the
-// documentElement.
+// Manages the root scroller associated with a given document. The root scroller
+// causes top controls movement, overscroll effects and prevents chaining
+// scrolls up further in the DOM. It can be set from script using
+// document.setRootScroller.
 //
-// Not all elements can be set as the root scroller. A valid element must be
-// in the top-level document and must have a LayoutObject that is at least a
-// LayoutBlockFlow or LayoutIFrame. In addition, only the top-level document can
-// set a non-default root scroller.
+// There are two notions of a root scroller in this class: m_rootScroller and
+// m_effectiveRootScroller. The former is the Element that was set as the root
+// scroller using document.setRootScroller. If the page didn't set a root
+// scroller this will be nullptr. The "effective" root scroller is the current
+// element we're using internally to apply viewport scroll actions. i.e It's the
+// element with the ViewportScrollCallback set as its apply-scroll callback.
+// The effective root scroller will only be null during document initialization.
+//
+// If the root scroller element is a valid element to become the root scroller,
+// it will be promoted to the effective root scroller. If it is not valid, the
+// effective root scroller will fall back to a default Element (see
+// defaultEffectiveRootScroller()). The rules for what makes an element a valid
+// root scroller are set in isValidRootScroller(). The validity of the current
+// root scroller is re-checked after each layout.
 class CORE_EXPORT RootScroller : public GarbageCollected<RootScroller> {
 public:
-    static RootScroller* create(FrameHost& frameHost)
+    // Creates a RootScroller for the given document. An optional
+    // ViewportScrollCallback can be provided. If it is, RootScroller will
+    // ensure that the effectiveRootScroller element always has this set as the
+    // apply scroll callback.
+    static RootScroller* create(
+        Document& document,
+        ViewportScrollCallback* applyScrollCallback)
     {
-        return new RootScroller(frameHost);
+        return new RootScroller(document, applyScrollCallback);
     }
 
-    // This method returns true if the element was set as the root scroller. If
-    // the element isn't eligible to be the root scroller or we're in some bad
-    // state, the method returns false without changing the current root
-    // scroller.
-    bool set(Element&);
+    // Creates an apply scroll callback that handles viewport actions like
+    // TopControls movement and Overscroll.
+    static ViewportScrollCallback* createViewportApplyScroll(
+        TopControls&, OverscrollController&);
+
+    DECLARE_TRACE();
+
+    // Sets the element that will be used as the root scroller. This can be
+    // nullptr, in which case we'll use the default element (documentElement) as
+    // the effective root scroller.
+    void set(Element*);
 
-    // Returns the element currently set as the root scroller.
+    // Returns the element currently set as the root scroller from script. This
+    // differs from the effective root scroller since the set Element may not
+    // currently be a valid root scroller. e.g. If the page sets an Element
+    // with `display: none`, get() will return that element, even though the
+    // effective root scroller will remain the element returned by
+    // defaultEffectiveRootScroller().
     Element* get() const;
 
+    // This returns the Element that's actually being used to control viewport
+    // actions right now. This is different from get() if a root scroller hasn't
+    // been set, or if the set root scroller isn't currently a valid scroller.
+    Element* effectiveRootScroller() const;
+
     // This class needs to be informed of changes in layout so that it can
-    // determine if the current scroller is still valid or if it must be
+    // determine if the current root scroller is still valid or if it must be
     // replaced by the defualt root scroller.
-    void didUpdateTopDocumentLayout();
-
-    DECLARE_TRACE();
+    void didUpdateLayout();
 
 private:
-    RootScroller(FrameHost&);

[esprehn, 2016/05/11 17:37:05]

fyi for the future, even private constructors need explicit if they have a
single argument, otherwise you can call them by mistake inside your own class.
:)

[bokan, 2016/06/02 15:49:55]

Acknowledged.

+    RootScroller(Document&, ViewportScrollCallback*);
+
+    Element* defaultEffectiveRootScroller();
 
-    bool isValid(Element&) const;
-    void resetToDefault();
-    void createApplyScrollIfNeeded();
+    // Ensures the effective root scroller is currently valid and replaces it
+    // with the default if not.
+    void updateEffectiveRootScroller();
+    void moveViewportApplyScroll(Element* target);

[esprehn, 2016/05/11 17:37:05]

can this be null? reference?

[bokan, 2016/06/02 15:49:55]

It can be null though, if we're using the defaultEffectiveRootScroller and
there's no documentElement. (The m_document check inside should never be NULL
though so I turned that into a DCHECK instead).

 
-    WeakMember<FrameHost> m_frameHost;
+    WeakMember<Document> m_document;
     Member<ViewportScrollCallback> m_viewportApplyScroll;
 
     WeakMember<Element> m_rootScroller;
+    WeakMember<Element> m_effectiveRootScroller;
 };
 
 } // namespace blink