From 7689f9b0abe0278459e79eaebc39dd5eddbd3c85 Mon Sep 17 00:00:00 2001 From: jamiewalch Date: Wed, 10 Sep 2014 18:51:18 -0700 Subject: [PATCH] Scroll-bar browser test. This doesn't yet do the most interesting test--whether or not scroll-bars work correctly when connected to a host--but that should be simple to add once a ClientPlugin mock is implemented. Review URL: https://codereview.chromium.org/552923002 Cr-Commit-Position: refs/heads/master@{#294290} --- chrome/chrome_tests.gypi | 1 + chrome/test/remoting/scrollbar_browsertest.cc | 26 +++ remoting/remoting_webapp_files.gypi | 1 + .../webapp/browser_test/scrollbar_browser_test.js | 195 +++++++++++++++++++++ remoting/webapp/html/template_main.html | 9 +- remoting/webapp/window_frame.css | 7 +- 6 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 chrome/test/remoting/scrollbar_browsertest.cc create mode 100644 remoting/webapp/browser_test/scrollbar_browser_test.js diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 37ec9f592757..b8987c197000 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1645,6 +1645,7 @@ 'test/remoting/qunit_browser_test_runner.cc', 'test/remoting/remote_desktop_browsertest.cc', 'test/remoting/remote_desktop_browsertest.h', + 'test/remoting/scrollbar_browsertest.cc', 'test/remoting/waiter.cc', 'test/remoting/waiter.h', 'test/remoting/webapp_javascript_unittest.cc', diff --git a/chrome/test/remoting/scrollbar_browsertest.cc b/chrome/test/remoting/scrollbar_browsertest.cc new file mode 100644 index 000000000000..325105c4089f --- /dev/null +++ b/chrome/test/remoting/scrollbar_browsertest.cc @@ -0,0 +1,26 @@ +// 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. + +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "chrome/test/remoting/remote_desktop_browsertest.h" +#include "chrome/test/remoting/waiter.h" + +namespace remoting { + +class ScrollbarBrowserTest : public RemoteDesktopBrowserTest { +}; + +IN_PROC_BROWSER_TEST_F(ScrollbarBrowserTest, MANUAL_Scrollbar_Visibility) { + SetUpTestForMe2Me(); + + content::WebContents* content = app_web_content(); + LoadScript(content, FILE_PATH_LITERAL("scrollbar_browser_test.js")); + + RunJavaScriptTest(content, "Scrollbars", "{}"); + + Cleanup(); +} + +} // namespace remoting diff --git a/remoting/remoting_webapp_files.gypi b/remoting/remoting_webapp_files.gypi index 9a784fe926ff..08c885a08de4 100644 --- a/remoting/remoting_webapp_files.gypi +++ b/remoting/remoting_webapp_files.gypi @@ -138,6 +138,7 @@ 'webapp/browser_test/bump_scroll_browser_test.js', 'webapp/browser_test/cancel_pin_browser_test.js', 'webapp/browser_test/invalid_pin_browser_test.js', + 'webapp/browser_test/scrollbar_browser_test.js', 'webapp/browser_test/update_pin_browser_test.js', ], # These product files are excluded from our JavaScript unittest 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 000000000000..2424c8ce5764 --- /dev/null +++ b/remoting/webapp/browser_test/scrollbar_browser_test.js @@ -0,0 +1,195 @@ +// 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.SCROLLBAR_WIDTH_ = 16; + this.BORDER_WIDTH_ = 1; + + // The top border is already accounted for by getBoundingClientRect, but + // the bottom border is not. + var marker = document.getElementById('bottom-marker'); + this.CONTENT_HEIGHT_ = + marker.getBoundingClientRect().top + this.BORDER_WIDTH_; + + // The width of the content is computed from the width of a
(690px) + // 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 + // product name. + this.CONTENT_WIDTH_ = 690 + 20 + 2 * this.BORDER_WIDTH_; + +}; + + +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(browserTest.pass, browserTest.fail); +}; + + +/** + * 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.CONTENT_WIDTH_ + 100; + var tooTall = this.CONTENT_HEIGHT_ + 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.CONTENT_WIDTH_, + this.CONTENT_HEIGHT_, + 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.CONTENT_WIDTH_ + this.SCROLLBAR_WIDTH_, + this.CONTENT_HEIGHT_ - 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.CONTENT_WIDTH_ - 1, + this.CONTENT_HEIGHT_ + this.SCROLLBAR_WIDTH_, + 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.CONTENT_WIDTH_, + this.CONTENT_HEIGHT_ - 1, + true, true)) + .then(removeScrollbars) + .then(this.resizeAndVerifyScroll_.bind( + this, + this.CONTENT_WIDTH_ - 1, + this.CONTENT_HEIGHT_, + true, true)) + + // Verify there are both horizontal and vertical scroll-bars, if both are + // required independently. + .then(removeScrollbars) + .then(this.resizeAndVerifyScroll_.bind( + this, + this.CONTENT_WIDTH_ - 1, + this.CONTENT_HEIGHT_ - 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
; since the content of that
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
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)); +}; diff --git a/remoting/webapp/html/template_main.html b/remoting/webapp/html/template_main.html index 72f84699ff85..981f7947d663 100644 --- a/remoting/webapp/html/template_main.html +++ b/remoting/webapp/html/template_main.html @@ -44,13 +44,19 @@ found in the LICENSE file. hidden>
- diff --git a/remoting/webapp/window_frame.css b/remoting/webapp/window_frame.css index a138921cd40c..838d5c64c6f0 100644 --- a/remoting/webapp/window_frame.css +++ b/remoting/webapp/window_frame.css @@ -81,7 +81,7 @@ html.apps-v2 body:not(.fullscreen) { position: relative; } -html.apps-v2 #scroller { +html.apps-v2 .window-body { height: calc(100% - 32px); /* Allow space for the title-bar */ } @@ -144,10 +144,13 @@ body:not(.connected) .window-options { */ html.apps-v2 body.fullscreen #scroller { - height: 100%; overflow: hidden; } +html.apps-v2 body.fullscreen .window-body { + height: 100%; +} + body.fullscreen .title-bar { border: 1px solid #a6a6a6; } -- 2.11.4.GIT