Implement Element.scrollIntoView for scroll-behavior: smooth.

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

Index: third_party/WebKit/Source/web/tests/SmoothScrollTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/SmoothScrollTest.cpp b/third_party/WebKit/Source/web/tests/SmoothScrollTest.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..4f20e2ed688f3d4ae450ff5d13def9c30feeb23d
--- /dev/null
+++ b/third_party/WebKit/Source/web/tests/SmoothScrollTest.cpp
@@ -0,0 +1,115 @@
+// Copyright 2017 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 "public/web/WebScriptSource.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "web/WebLocalFrameImpl.h"
+#include "web/tests/sim/SimCompositor.h"
+#include "web/tests/sim/SimDisplayItemList.h"
+#include "web/tests/sim/SimRequest.h"
+#include "web/tests/sim/SimTest.h"
+
+namespace blink {
+
+namespace {
+
+class SmoothScrollTest : public SimTest {};
+
+TEST_F(SmoothScrollTest, InstantScroll) {
+  v8::HandleScope handleScope(v8::Isolate::GetCurrent());
+  webView().resize(WebSize(800, 600));
+  SimRequest request("https://example.com/test.html", "text/html");
+  loadURL("https://example.com/test.html");
+  request.complete(
+      "<div id='space' style='height: 1000px'></div>"
+      "<div id='block' style='height: 1000px'></div>");
+
+  compositor().beginFrame();
+  ASSERT_EQ(0, window().scrollY());
+  mainFrame().executeScriptAndReturnValue(
+      WebScriptSource("document.getElementById('block').scrollIntoView();"));
+
+  compositor().beginFrame();
+  ASSERT_GE(window().scrollY(), 1000);
+}
+
+TEST_F(SmoothScrollTest, SmoothScroll) {
+  v8::HandleScope handleScope(v8::Isolate::GetCurrent());
+  webView().resize(WebSize(800, 600));
+  SimRequest request("https://example.com/test.html", "text/html");
+  loadURL("https://example.com/test.html");
+  request.complete(
+      "<div id='space' style='height: 1000px'></div>"
+      "<div id='block' style='height: 1000px'></div>");
+
+  compositor().beginFrame();
+  ASSERT_EQ(window().scrollY(), 0);
+
+  mainFrame().executeScriptAndReturnValue(
+      WebScriptSource("document.getElementById('block').scrollIntoView("
+                      "{block: 'start', behavior: 'smooth'});"));
+  compositor().beginFrame();
+  ASSERT_LT(window().scrollY(), 100);
+
+  // Scrolling the container
+  compositor().beginFrame();

[sunyunjia, 2017/04/07 13:53:21]

Seems I have to add this extra beginFrame() to start the animation.

[bokan, 2017/04/07 15:56:52]

Does this mean the ASSERT above should be scrollY == 0? I can understand why the
first frame might not move the animation (the next frame after the animation is
set is used to set the start time) but I'm a little confused why it needs two.
Could you do some debugging to determine why?

[sunyunjia, 2017/05/12 18:40:25]

I did some investigation. In the 1st BeginFrame(),
ProgrammaticScrollAnimator::run_state_ is set to kRunningOnMainThread from
kWaitingToSendToCompositor. In the 2nd BeginFrame(), TickAnimaion() got run and
the start_time_ is set to then, which means no elapsed_time yet thus no scroll.

[bokan, 2017/05/15 17:15:27]

Ahh, that makes sense - thanks for digging into it.

+  compositor().beginFrame(0.2);
+  ASSERT_GT(window().scrollY(), 250);
+  ASSERT_LT(window().scrollY(), 350);
+
+  // Finish scrolling the container
+  compositor().beginFrame(1);
+  ASSERT_GE(window().scrollY(), 1000);

[bokan, 2017/04/07 15:56:52]

This should be testing scrollY == 1000 right? if we scroll further we've
overshot.

[sunyunjia, 2017/05/12 18:40:25]

Done.

+}
+
+TEST_F(SmoothScrollTest, NestedContainer) {
+  v8::HandleScope handleScope(v8::Isolate::GetCurrent());
+  webView().resize(WebSize(800, 600));
+  SimRequest request("https://example.com/test.html", "text/html");
+  loadURL("https://example.com/test.html");
+  request.complete(
+      "<div id='space' style='height: 1000px'></div>"
+      "<div id='container' style='height: 600px; overflow: scroll'>"
+      "  <div id='space1' style='height: 1000px'></div>"
+      "  <div id='block' style='height: 1000px'></div>"
+      "</div>");
+
+  compositor().beginFrame();
+  Element* container = document().getElementById("container");
+
+  ASSERT_EQ(window().scrollY(), 0);
+  ASSERT_EQ(container->scrollTop(), 0);
+
+  mainFrame().executeScriptAndReturnValue(
+      WebScriptSource("document.getElementById('block').scrollIntoView("
+                      "{block: 'start', behavior: 'smooth'});"));
+  compositor().beginFrame();
+  ASSERT_LT(window().scrollY(), 100);
+
+  // Scrolling the outer container
+  compositor().beginFrame();
+  compositor().beginFrame(0.2);
+  ASSERT_GT(window().scrollY(), 250);
+  ASSERT_LT(window().scrollY(), 350);
+  ASSERT_EQ(container->scrollTop(), 0);
+
+  // Finish scrolling the outer container
+  compositor().beginFrame(1);
+  ASSERT_GE(window().scrollY(), 1000);
+  ASSERT_LE(container->scrollTop(), 20);
+
+  // Scrolling the inner container
+  compositor().beginFrame();
+  compositor().beginFrame(0.2);
+  ASSERT_GE(container->scrollTop(), 250);
+  ASSERT_LE(container->scrollTop(), 350);
+
+  // Finish scrolling the inner container
+  compositor().beginFrame(1);
+  ASSERT_GE(container->scrollTop(), 1000);
+}
+

[bokan, 2017/04/07 15:56:52]

If we start another programmatic smooth scroll (or even instant), should that
cancel the original one? Or just queue after it? We should have a test for that
case.

[sunyunjia, 2017/05/12 18:40:25]

Thanks! I would cancel the original one.

[bokan, 2017/05/15 17:15:27]

Correct me if I'm wrong but I think your current patch does this for another
scroll into view, but what about a programatic scroll that's not scroll into
view? (e.g. a smooth ScrollTo). In that case, I think
sequenced_for_smooth_scroll_ in the ProgrammticScrollAnimator will reset to
false but you'll still have all your other nested scrollers queued in the
sequencer. You should reset the queue from ProgrammaticScrollAnimator when an
animation is cancelled and sequenced_for_smooth_scroll_ is true.

+}  // namespace
+
+}  // namespace blink