Make document.rootScroller work properly across iframes.

third_party/WebKit/Source/web/tests/RootScrollerTest.cpp

Index: third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp b/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
index 1ca7e9f3a0b896a2088f93f5ce35a0cef8612ad0..703f4fed3dc380fe4c33d38597416f777622e6c8 100644
--- a/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
+++ b/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
@@ -8,6 +8,7 @@
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/page/Page.h"
 #include "core/page/scrolling/RootScrollerController.h"
+#include "core/paint/PaintLayerScrollableArea.h"
 #include "platform/testing/URLTestHelpers.h"
 #include "platform/testing/UnitTestHelpers.h"
 #include "public/platform/Platform.h"
@@ -459,6 +460,143 @@ TEST_F(RootScrollerTest, TestRootScrollerWithinIframe)
     }
 }
 
+// Tests that setting an iframe as the root scroller makes the iframe the
+// effective root scroller in the parent frame.
+TEST_F(RootScrollerTest, SetRootScrollerIframeBecomesEffective)
+{
+    initialize("root-scroller-iframe.html");
+    ASSERT_EQ(nullptr, mainFrame()->document()->rootScroller());

[tdresser, 2016/08/25 19:46:37]

Throughput this file, we're ASSERT'ing when we should be EXPECT'ing.

[bokan, 2016/08/26 19:35:05]

ASSERTs will stop the test when they fail whereas EXPECTs won't. I think in all
these cases, if the ASSERT fails, our assumptions following it are violated and
so they'll fail trivially which will just add noise to the failure and make it
harder to track down. I'll leave it as is, if you disagree I can change it
later/we can discuss.

[tdresser, 2016/08/29 15:07:50]

I think there are lots of cases here where seeing multiple expectation failures
at once would be better here, but I don't feel strongly about it.

[bokan, 2016/08/29 17:04:35]

Personally, I find multiple failures to be distracting but that's a personal
preference and EXPECT is more common so I've changed most checks in this file to
EXPECT.

+
+    {
+        NonThrowableExceptionState nonThrow;
+
+        // Try to set the root scroller in the main frame to be the iframe
+        // element.
+        HTMLFrameOwnerElement* iframe = toHTMLFrameOwnerElement(
+            mainFrame()->document()->getElementById("iframe"));
+
+        mainFrame()->document()->setRootScroller(iframe, nonThrow);
+
+        ASSERT_EQ(iframe, mainFrame()->document()->rootScroller());
+        ASSERT_EQ(iframe,
+            mainFrame()->document()->rootScrollerController()
+                ->effectiveRootScroller());
+
+        Element* container =
+            iframe->contentDocument()->getElementById("container");
+
+        iframe->contentDocument()->setRootScroller(container, nonThrow);
+
+        ASSERT_EQ(container, iframe->contentDocument()->rootScroller());
+        ASSERT_EQ(container,
+            iframe->contentDocument()->rootScrollerController()
+                ->effectiveRootScroller());
+        ASSERT_EQ(iframe, mainFrame()->document()->rootScroller());
+        ASSERT_EQ(iframe,
+            mainFrame()->document()->rootScrollerController()
+                ->effectiveRootScroller());
+    }
+}
+
+// Tests that the global root scroller is correctly calculated when getting the
+// root scroller layer and that the viewport apply scroll is set on it.
+TEST_F(RootScrollerTest, SetRootScrollerIframeUsesCorrectLayerAndCallback)
+{
+    initialize("root-scroller-iframe.html");
+    ASSERT_EQ(nullptr, mainFrame()->document()->rootScroller());
+
+    HTMLFrameOwnerElement* iframe = toHTMLFrameOwnerElement(
+        mainFrame()->document()->getElementById("iframe"));
+    Element* container =
+        iframe->contentDocument()->getElementById("container");
+
+    RootScrollerController* mainController =
+        mainFrame()->document()->rootScrollerController();
+
+    NonThrowableExceptionState nonThrow;
+
+    // No root scroller set, the documentElement should be the effective root
+    // and the main FrameView's scroll layer should be the layer to use.
+    {
+        ASSERT_EQ(
+            mainController->rootScrollerLayer(),
+            mainFrameView()->layerForScrolling());
+        ASSERT_TRUE(mainController->isViewportScrollCallback(
+            mainFrame()->document()->documentElement()->getApplyScroll()));
+    }
+
+    // Set a root scroller in the iframe. Since the main document didn't set a
+    // root scroller, the global root scroller shouldn't change.
+    {
+
+        iframe->contentDocument()->setRootScroller(container, nonThrow);
+
+        ASSERT_EQ(
+            mainController->rootScrollerLayer(),
+            mainFrameView()->layerForScrolling());
+        ASSERT_TRUE(mainController->isViewportScrollCallback(
+            mainFrame()->document()->documentElement()->getApplyScroll()));
+    }
+
+    // Setting the iframe as the root scroller in the main frame should now
+    // link the root scrollers so the container should now be the global root
+    // scroller.
+    {
+        mainFrame()->document()->setRootScroller(iframe, nonThrow);
+
+        ScrollableArea* containerScroller =
+            static_cast<PaintInvalidationCapableScrollableArea*>(
+                toLayoutBox(container->layoutObject())->getScrollableArea());
+
+        ASSERT_EQ(
+            mainController->rootScrollerLayer(),
+            containerScroller->layerForScrolling());
+        ASSERT_FALSE(mainController->isViewportScrollCallback(
+            mainFrame()->document()->documentElement()->getApplyScroll()));

[tdresser, 2016/08/25 19:46:37]

This is redundant with the ASSERT below, isn't it?

[bokan, 2016/08/26 19:35:05]

If everything's working correctly but it's a bit of a sanity check. i.e. Make
sure the apply scroll isn't set on both Elements.

[tdresser, 2016/08/29 15:07:50]

Acknowledged.

+        ASSERT_TRUE(mainController->isViewportScrollCallback(
+            container->getApplyScroll()));
+    }
+
+    // Unsetting the root scroller in the iframe should reset its effective
+    // root scroller to the iframe's documentElement and thus the iframe's
+    // documentElement becomes the global root scroller.
+    {
+        iframe->contentDocument()->setRootScroller(nullptr, nonThrow);
+        ASSERT_EQ(
+            mainController->rootScrollerLayer(),
+            iframe->contentDocument()->view()->layerForScrolling());
+        ASSERT_FALSE(mainController->isViewportScrollCallback(
+            container->getApplyScroll()));
+        ASSERT_FALSE(mainController->isViewportScrollCallback(
+            mainFrame()->document()->documentElement()->getApplyScroll()));

[tdresser, 2016/08/25 19:46:37]

Aren't the two ASSERTs above unneeded?

[bokan, 2016/08/26 19:35:05]

Same as above, just making sure it's removed from the old Elements..

[tdresser, 2016/08/29 15:07:51]

Acknowledged.

+        ASSERT_TRUE(mainController->isViewportScrollCallback(
+            iframe->contentDocument()->documentElement()->getApplyScroll()));
+    }
+
+    // Finally, unsetting the main frame's root scroller should reset it to the
+    // documentElement and corresponding layer.
+    {
+        mainFrame()->document()->setRootScroller(nullptr, nonThrow);
+        ASSERT_EQ(
+            mainController->rootScrollerLayer(),
+            mainFrameView()->layerForScrolling());
+        ASSERT_TRUE(mainController->isViewportScrollCallback(
+            mainFrame()->document()->documentElement()->getApplyScroll()));
+        ASSERT_FALSE(mainController->isViewportScrollCallback(
+            container->getApplyScroll()));
+        ASSERT_FALSE(mainController->isViewportScrollCallback(
+            iframe->contentDocument()->documentElement()->getApplyScroll()));
+    }
+}
+
+TEST_F(RootScrollerTest, TestSetRootScrollerCausesViewportLayerChange)
+{
+    // TODO(bokan): Need a test that changing root scrollers actually sets the
+    // outer viewport layer on the compositor, even in the absence of other
+    // compositing changes. crbug.com/505516

[bokan, 2016/08/25 17:25:55]

I thought of this so I added the TODO so I don't forget. My next patch will deal
with compositing issues so I'll fill this in there.

+}
+
+
 // Tests that trying to set an element as the root scroller of a document inside
 // an iframe fails when that element belongs to the parent document.
 // TODO(bokan): Recent changes mean this is now possible but should be fixed.