Scroll-bar browser test.

remoting/webapp/browser_test/scrollbar_browser_test.js

Index: remoting/webapp/browser_test/scrollbar_browser_test.js
diff --git a/remoting/webapp/browser_test/scrollbar_browser_test.js b/remoting/webapp/browser_test/scrollbar_browser_test.js
new file mode 100644
index 0000000000000000000000000000000000000000..bec8b7c6361622e3348955cb818a4ba303152eb2
--- /dev/null
+++ b/remoting/webapp/browser_test/scrollbar_browser_test.js
@@ -0,0 +1,196 @@
+// Copyright 2014 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.
+
+/**
+ * @suppress {checkTypes}
+ *
+ * @fileoverview
+ * Browser test for the scenario below:
+ * 1. Resize the client window to various sizes and verify the existence of
+ *    horizontal and/or vertical scroll-bars.
+ * 2. TODO(jamiewalch): Connect to a host and toggle various combinations of
+ *    scale and resize; repeat test 1.
+ * 3. TODO(jamiewalch): Disconnect; repeat test 1.
+ */
+
+'use strict';
+
+/** @constructor */
+browserTest.Scrollbars = function() {
+  this.scroller_ = document.getElementById('scroller');
+  this.kScrollBarWidth_ = 16;

[kelvinp, 2014/09/10 00:02:51]

According to style guide, constants should be named LIKE_THIS.
https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Naming

[Jamie, 2014/09/10 00:40:50]

Yuck :( Done

+  this.kBorderWidth_ = 1;
+
+  // The top border is already accounted for by getBoundingClientRect, but
+  // the bottom border is not.
+  var marker = document.getElementById('bottom-marker');
+  this.kContentHeight_ =
+      marker.getBoundingClientRect().top + this.kBorderWidth_;

[kelvinp, 2014/09/10 00:02:51]

why not just get the clientHeight(content + padding) of the home-screen div?
e.g. document.querySelector('.home-screen').clientHeight

[Jamie, 2014/09/10 00:40:50]

There are a couple of problems with this. Firstly, the home-screen div is marked
as full-height because otherwise my scroll-bar detection doesn't work
(elementFromPoint returns the scroller both if the content div is larger than
the scroller and if it's smaller). Secondly, I'd have to include the title-bar
in all the measurements.

I'm not particularly happy with needing a <div> to measure this, but I think
it's better than the alternatives I came up with. We could instead take the
bottom of the preceding element, but that feels more likely to be flaky as new
elements are added; at least with the approach I've taken there's a comment in
the source to guide people when adding new elements.

+
+  // The width of the content is computed from the width of a <section> (590px)

[kelvinp, 2014/09/10 00:02:51]

Is it a typo of 690px?

[Jamie, 2014/09/10 00:40:49]

Yes; done.

+  // plus the margin of the "inset" class (20px). There's no easy way to get
+  // that without hard-coding it. In fact, this is a bit simplistic because
+  // the horizontal space required by the header depends on the length of the

[kelvinp, 2014/09/10 00:02:51]

Would the test break if the clientWidth of the contents are changed? Would
document.querySelector('.home-screen').clientWidth (content + padding) work
better?

[Jamie, 2014/09/10 00:40:50]

Yes, the test will break in that case. I could measure the div to get rid of the
hard-coded 690px, but I can't see any easy way of getting rid of the hard-coded
margin width (short of parsing the CSS). Since the tests are very sensitive
(just 1px difference between test cases), any change to the CSS that affects
this will result in a test failure so I think it will fail safe, but if you can
think of a better approach, I'm open to changing it.

+  // product name.
+  this.kContentWidth_ = 690 + 20 + 2 * this.kBorderWidth_;
+
+};
+
+
+browserTest.Scrollbars.prototype.run = function(data) {
+  if (!base.isAppsV2()) {
+    browserTest.fail(
+        'Scroll-bar testing requires resizing the app window, which can ' +
+        'only be done programmatically in apps v2.');
+  }
+
+  // Verify that scrollbars are added/removed correctly on the home screen.
+  this.verifyHomeScreenScrollbars_()
+  .then(function(value) { return browserTest.pass(value); },

[kelvinp, 2014/09/10 00:02:51]

Nit: 
.then(browserTest.pass, browserTest.fail)

[Jamie, 2014/09/10 00:40:49]

Done.

+        function(error) { return browserTest.fail(error); });
+};
+
+
+/**
+ * Verify the test cases for the home-screen.
+ */
+browserTest.Scrollbars.prototype.verifyHomeScreenScrollbars_ = function() {
+  // Note that, due to crbug.com/240772, if the window already has
+  // scroll-bars, they will not be removed if the window size is
+  // increased by less than the scroll-bar width. We work around that
+  // when connected to a host because we know how big the content is
+  // (in fact, testing this work-around is the main motivation for
+  // writing this test), but it's not worth it for the home screen,
+  // so make the window large not to require scrollbars before each test.
+  var tooWide = this.kContentWidth_ + 100;
+  var tooTall = this.kContentHeight_ + 100;
+  var removeScrollbars = this.resize_.bind(this, tooWide, tooTall);
+
+  // Verify there are no scroll-bars if the window is as big as it needs
+  // to be.
+  return removeScrollbars()
+  .then(this.resizeAndVerifyScroll_(
+        this.kContentWidth_,
+        this.kContentHeight_,
+        false, false))
+
+  // Verify there is a vertical scroll-bar if the window is shorter than it
+  // needs to be.
+  .then(removeScrollbars)
+  .then(this.resizeAndVerifyScroll_.bind(
+        this,
+        this.kContentWidth_ + this.kScrollBarWidth_,
+        this.kContentHeight_ - 1,
+        false, true))
+
+  // Verify there is a horizontal scroll-bar if the window is narrow than it
+  // needs to be.
+  .then(removeScrollbars)
+  .then(this.resizeAndVerifyScroll_.bind(
+        this,
+        this.kContentWidth_ - 1,
+        this.kContentHeight_ + this.kScrollBarWidth_,
+        true, false))
+
+  // Verify there are both horizontal and vertical scroll-bars, even if one
+  // is only needed as a result of the space occupied by the other.
+  .then(removeScrollbars)
+  .then(this.resizeAndVerifyScroll_.bind(
+        this,
+        this.kContentWidth_,
+        this.kContentHeight_ - 1,
+        true, true))
+  .then(removeScrollbars)
+  .then(this.resizeAndVerifyScroll_.bind(
+        this,
+        this.kContentWidth_ - 1,
+        this.kContentHeight_,
+        true, true))
+
+  // Verify there are both horizontal and vertical scroll-bars, if both are
+  // required independently.
+  .then(removeScrollbars)
+  .then(this.resizeAndVerifyScroll_.bind(
+        this,
+        this.kContentWidth_ - 1,
+        this.kContentHeight_ - 1,
+        true, true));
+};
+
+
+/**
+ * Returns whether or not horizontal and vertical scroll-bars are expected
+ * and visible. To do this, it performs a hit-test close to the right and
+ * bottom edges of the scroller <div>; since the content of that <div> fills
+ * it completely, the hit-test will return the content unless there is a
+ * scroll-bar visible on the corresponding edge, in which case it will return
+ * the scroller <div> itself.
+ *
+ * @private
+ */
+browserTest.Scrollbars.prototype.getScrollbarState_ = function() {
+  var rect = this.scroller_.getBoundingClientRect();
+  var rightElement = document.elementFromPoint(
+      rect.right - 1, (rect.top + rect.bottom) / 2);
+  var bottomElement = document.elementFromPoint(
+      (rect.left + rect.right) / 2, rect.bottom - 1);
+  return {
+    horizontal: bottomElement === this.scroller_,
+    vertical: rightElement === this.scroller_
+  };
+};
+
+
+/**
+ * Returns a promise that resolves if the scroll-bar state is as expected, or
+ * rejects otherwise.
+ *
+ * @private
+ */
+browserTest.Scrollbars.prototype.verifyScrollbarState_ =
+    function(horizontalExpected, verticalExpected) {
+  var scrollbarState = this.getScrollbarState_();
+  if (scrollbarState.horizontal && !horizontalExpected) {
+    return Promise.reject(new Error(
+        'Horizontal scrollbar present but not expected.'));
+  } else if (!scrollbarState.horizontal && horizontalExpected) {
+    return Promise.reject(new Error(
+        'Horizontal scrollbar expected but not present.'));
+  } else if (scrollbarState.vertical && !verticalExpected) {
+    return Promise.reject(new Error(
+        'Vertical scrollbar present but not expected.'));
+  } else if (!scrollbarState.vertical && verticalExpected) {
+    return Promise.reject(new Error(
+        'Vertical scrollbar expected but not present.'));
+  }
+  return Promise.resolve();
+};
+
+
+/**
+ * @private
+ * @return {Promise} A promise that will be fulfilled when the window has
+ *     been resized and it's safe to test scroll-bar visibility.
+ */
+browserTest.Scrollbars.prototype.resize_ = function(width, height) {
+  var win = chrome.app.window.current();
+  win.resizeTo(width, height);
+  // Chrome takes a while to update the scroll-bars, so don't resolve
+  // immediately. Waiting for the onBoundsChanged event would be cleaner,
+  // but isn't reliable.
+  return base.Promise.sleep(500);
+};
+
+
+/**
+ * @private
+ * @return {Promise} A promise that will be fulfilled when the window has
+ *     been resized and it's safe to test scroll-bar visibility.
+ */
+browserTest.Scrollbars.prototype.resizeAndVerifyScroll_ =
+    function(width, height, horizontalExpected, verticalExpected) {
+  return this.resize_(width, height).then(
+      this.verifyScrollbarState_.bind(
+          this, horizontalExpected, verticalExpected));
+};