Add ScrollPageToElement to action_runner.

telemetry/telemetry/internal/actions/scroll_to_element.py

Index: telemetry/telemetry/internal/actions/scroll_to_element.py
diff --git a/telemetry/telemetry/internal/actions/scroll_to_element.py b/telemetry/telemetry/internal/actions/scroll_to_element.py
new file mode 100644
index 0000000000000000000000000000000000000000..35732e6afe5d793ee038d73a81950f3442b0c622
--- /dev/null
+++ b/telemetry/telemetry/internal/actions/scroll_to_element.py
@@ -0,0 +1,60 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

2016

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from telemetry.internal.actions import page_action
+from telemetry.internal.actions.scroll import ScrollAction
+
+
+class ScrollToElementAction(page_action.PageAction):
+
+
+  def __init__(self, selector=None, element_function=None, timeout=60,
+               speed=None):

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

What is |speed| in this case? The number of pixels scrolled per second? Probably
worth documenting. (similarly, not sure what |timeout| means in this context)

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+    super(ScrollToElementAction, self).__init__()
+    self._selector = selector
+    self._element_function = element_function
+    self._timeout = timeout
+    self._speed = speed
+    self._distance = None
+    self._direction = None
+    assert (self._selector or self._element_function), (
+        'Must have either selector or element function')
+
+  def WillRunAction(self, tab):
+    if self._selector:
+      element = 'document.querySelector("%s")' % self._selector
+    else:
+      element = self._element_function
+
+    js = '''

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

Maybe get_distance_js?

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+      f = (

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

can you just do:

(elem) => {
  // Your function code here...
}(%s);

? In this case, you're still polluting the global namespace with a function
named "f"

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+          (elem) => {
+        var rect = elem.getBoundingClientRect();
+        if (rect.bottom < 0) {
+          // The bottom of the element is above the viewport.
+          // Scroll up until the top of the element is on screen.
+          return rect.top + (window.innerHeight / 2);

[rnephew (Reviews Here), 2016/10/14 17:57:18]

rect.top is a negative number here, so need to subtract not add.

+        }
+        if (rect.top - window.innerHeight >= 0) {
+          // rect.top provides the pixel offset of the element from the
+          // top of the page. Because that exceeds the viewport's height,
+          // we know that the element is below the viewport.
+          return rect.top - (window.innerHeight / 2);
+        }
+        return 0

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

nit: needs a semicolon

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+      })
+      f(%s);

[nednguyen, 2016/10/13 23:16:12]

If we can compute the distance here, why not also call the js to scroll directly
(like in scroll.py). This would help avoid the extra round trip between
telemetry & the browser.

[rnephew (Reviews Here), 2016/10/14 00:13:18]

I was trying to reuse code to limit the complexity as much as possible. Does the
extra round trip cause problems that we should avoid?

[nednguyen, 2016/10/14 00:19:02]

The round trip mostly will cause more delay & extra js issued on the page.
Though in this case, your js is not that much, so I think this could be fine.

+    ''' % element
+
+    self._distance = tab.EvaluateJavaScript(js)
+    self._direction = 'down' if self._distance > 0 else 'up'
+    self._distance = abs(self._distance)
+
+
+  def RunAction(self, tab):
+    if self._distance == 0:  # Element is already in view.
+      return
+    scroller = ScrollAction(direction=self._direction, distance=self._distance)
+    scroller.WillRunAction(tab)

[charliea (OOO until 10-5), 2016/10/14 03:34:28]

I'm not 100% sure, but it seems like we might want to create the scroller in
WillRunAction so that all of the same exceptions that would get raised for a
ScrollAction will also get raised at the same time for our ScrollToElementAction

[rnephew (Reviews Here), 2016/10/14 17:57:18]

Done.

+    scroller.RunAction(tab)