Bug 1357878 - Prevent hang on setting window size to existing size; r=maja_zf

When the window's size is being set to the window's existing size,
Marionette unconditionally listens for the DOM resize event.  When a
window is not resized, no such event fires.

This patch skips setting the window size when the window's current size
is the requested size.  This bypasses the problem of listening for an
event that never occurs.

It also combines the window position- and size tests into a
test_window_rect.py test, since they share many of the same
characteristics.

Fixes: https://github.com/mozilla/geckodriver/issues/643
MozReview-Commit-ID: IUtCFXwT1fh

--HG--
extra : rebase_source : 43c4d0e24cf1e0dc6102af48b8fe6f075b6dd4a2
This commit is contained in:
Andreas Tolfsen 2017-04-19 21:19:36 +01:00
parent e1e203f1f5
commit 5bd3bf699c
4 changed files with 108 additions and 118 deletions

View File

@ -1375,36 +1375,40 @@ GeckoDriver.prototype.getWindowRect = function (cmd, resp) {
* A modal dialog is open, blocking this operation.
*/
GeckoDriver.prototype.setWindowRect = function* (cmd, resp) {
assert.firefox();
assert.firefox()
const win = assert.window(this.getCurrentWindow());
assert.noUserPrompt(this.dialog);
let {x, y, height, width} = cmd.parameters;
let {x, y, width, height} = cmd.parameters;
if (height != null && width != null) {
assert.positiveInteger(height);
assert.positiveInteger(width);
yield new Promise(resolve => {
// When the DOM resize event claims that it fires _after_ the document
// view has been resized, it is lying.
//
// Because resize events fire at a high rate, DOM modifications
// such as updates to outerWidth/outerHeight are not guaranteed to
// have processed. To overcome this... abomination... of the web
// platform, we throttle the event using setTimeout. If everything
// was well in this world we would use requestAnimationFrame, but
// it does not seem to like our particular flavour of XUL.
const fps15 = 66;
const synchronousResize = () => win.setTimeout(resolve, fps15);
win.addEventListener("resize", synchronousResize, {once: true});
win.resizeTo(width, height);
});
if (win.outerWidth != width && win.outerHeight != height) {
yield new Promise(resolve => {
// When the DOM resize event claims that it fires _after_ the document
// view has been resized, it is lying.
//
// Because resize events fire at a high rate, DOM modifications
// such as updates to outerWidth/outerHeight are not guaranteed to
// have processed. To overcome this... abomination... of the web
// platform, we throttle the event using setTimeout. If everything
// was well in this world we would use requestAnimationFrame, but
// it does not seem to like our particular flavour of XUL.
const fps15 = 66;
const synchronousResize = () => win.setTimeout(resolve, fps15);
win.addEventListener("resize", synchronousResize, {once: true});
win.resizeTo(width, height);
});
}
}
if (x != null && y != null) {
assert.integer(x);
assert.integer(y);
let orig = {screenX: win.screenX, screenY: win.screenY};
const orig = {screenX: win.screenX, screenY: win.screenY};
win.moveTo(x, y);
yield wait.until((resolve, reject) => {
if ((x == win.screenX && y == win.screenY) ||
@ -1422,7 +1426,6 @@ GeckoDriver.prototype.setWindowRect = function* (cmd, resp) {
"width": win.outerWidth,
"height": win.outerHeight,
};
};
/**

View File

@ -1,84 +0,0 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
from marionette_harness import MarionetteTestCase
class TestSetWindowSize(MarionetteTestCase):
def setUp(self):
super(MarionetteTestCase, self).setUp()
self.start_size = self.marionette.window_size
self.max_width = self.marionette.execute_script("return window.screen.availWidth;")
self.max_height = self.marionette.execute_script("return window.screen.availHeight;")
def tearDown(self):
# WebDriver spec says a resize cannot result in window being maximized, an
# error is returned if that is the case; therefore if the window is maximized
# at the start of this test, returning to the original size via set_window_size
# size will result in error; so reset to original size minus 1 pixel width
if self.start_size['width'] == self.max_width and self.start_size['height'] == self.max_height:
self.start_size['width']-=1
self.marionette.set_window_size(self.start_size['width'], self.start_size['height'])
super(MarionetteTestCase, self).tearDown()
def test_that_we_can_get_and_set_window_size(self):
# event handler
self.marionette.execute_script("""
window.wrappedJSObject.rcvd_event = false;
window.onresize = function() {
window.wrappedJSObject.rcvd_event = true;
};
""")
# valid size
width = self.max_width - 100
height = self.max_height - 100
self.marionette.set_window_size(width, height)
self.wait_for_condition(lambda m: m.execute_script("return window.wrappedJSObject.rcvd_event;"))
size = self.marionette.window_size
self.assertEqual(size['width'], width,
"Window width is {0} but should be {1}".format(size['width'], width))
self.assertEqual(size['height'], height,
"Window height is {0} but should be {1}".format(size['height'], height))
def test_that_we_can_get_new_size_when_set_window_size(self):
actual = self.marionette.window_size
width = actual['width'] - 50
height = actual['height'] - 50
size = self.marionette.set_window_size(width, height)
self.assertIsNotNone(size, "Response is None")
self.assertEqual(size['width'], width,
"New width is {0} but should be {1}".format(size['width'], width))
self.assertEqual(size['height'], height,
"New height is {0} but should be {1}".format(size['height'], height))
def test_possible_to_request_window_larger_than_screen(self):
self.marionette.set_window_size(4 * self.max_width, 4 * self.max_height)
size = self.marionette.window_size
# In X the window size may be greater than the bounds of the screen
self.assertGreaterEqual(size["width"], self.max_width)
self.assertGreaterEqual(size["height"], self.max_height)
def test_that_we_can_maximise_the_window(self):
# valid size
width = self.max_width - 100
height = self.max_height - 100
self.marionette.set_window_size(width, height)
# event handler
self.marionette.execute_script("""
window.wrappedJSObject.rcvd_event = false;
window.onresize = function() {
window.wrappedJSObject.rcvd_event = true;
};
""")
self.marionette.maximize_window()
self.wait_for_condition(lambda m: m.execute_script("return window.wrappedJSObject.rcvd_event;"))
size = self.marionette.window_size
self.assertGreaterEqual(size['width'], self.max_width,
"Window width does not use availWidth, current width: {0}, max width: {1}".format(size['width'], self.max_width))
self.assertGreaterEqual(size['height'], self.max_height,
"Window height does not use availHeight. current width: {0}, max width: {1}".format(size['height'], self.max_height))

View File

@ -7,7 +7,7 @@ from marionette_driver.errors import InvalidArgumentException
from marionette_harness import MarionetteTestCase
class TestWindowPosition(MarionetteTestCase):
class TestPosition(MarionetteTestCase):
def setUp(self):
MarionetteTestCase.setUp(self)
@ -20,6 +20,8 @@ class TestWindowPosition(MarionetteTestCase):
def test_get_types(self):
position = self.marionette.get_window_position()
self.assertIn("x", position)
self.assertIn("y", position)
self.assertIsInstance(position["x"], int)
self.assertIsInstance(position["y"], int)
@ -43,17 +45,6 @@ class TestWindowPosition(MarionetteTestCase):
self.assertNotEqual(old_position["x"], new_position["x"])
self.assertNotEqual(old_position["y"], new_position["y"])
def test_set_size_with_rect(self):
actual = self.marionette.window_size
width = actual["width"] - 50
height = actual["height"] - 50
size = self.marionette.set_window_rect(width=width, height=height)
self.assertEqual(size["width"], width,
"New width is {0} but should be {1}".format(size["width"], width))
self.assertEqual(size["height"], height,
"New height is {0} but should be {1}".format(size["height"], height))
def test_move_to_new_position(self):
old_position = self.marionette.get_window_position()
new_position = {"x": old_position["x"] + 10, "y": old_position["y"] + 10}
@ -111,3 +102,85 @@ class TestWindowPosition(MarionetteTestCase):
elif os == "windows_nt":
self.assertEqual(-8, position["x"])
self.assertEqual(-8, position["y"])
class TestSize(MarionetteTestCase):
def setUp(self):
super(MarionetteTestCase, self).setUp()
self.max = self.marionette.execute_script("""
return {
width: window.screen.availWidth,
height: window.screen.availHeight,
}""", sandbox=None)
# WebDriver spec says a resize cannot result in window being
# maximised, an error is returned if that is the case; therefore if
# the window is maximised at the start of this test, returning to
# the original size via set_window_size size will result in error;
# so reset to original size minus 1 pixel width
start_size = self.marionette.window_size
if start_size["width"] == self.max["width"] and start_size["height"] == self.max["height"]:
self.start_size["width"] -= 1
self.start_size["height"] -= 1
self.marionette.set_window_size(start_size["width"], start_size["height"])
self.original_size = self.marionette.window_size
def tearDown(self):
self.marionette.set_window_size(
self.original_size["width"], self.original_size["height"])
super(MarionetteTestCase, self).tearDown()
def test_get_types(self):
size = self.marionette.window_size
self.assertIn("width", size)
self.assertIn("height", size)
self.assertIsInstance(size["width"], int)
self.assertIsInstance(size["height"], int)
def test_set_types(self):
for width, height in (["a", "b"], [1.2, 3.4], [True, False], [[], []], [{}, {}]):
print("testing invalid type size ({},{})".format(width, height))
with self.assertRaises(InvalidArgumentException):
self.marionette.set_window_size(width, height)
def test_setting_window_rect_with_nulls_errors(self):
with self.assertRaises(InvalidArgumentException):
self.marionette.set_window_rect(height=None, width=None,
x=None, y=None)
def test_set_size_with_rect(self):
actual = self.marionette.window_size
width = actual["width"] - 50
height = actual["height"] - 50
size = self.marionette.set_window_rect(width=width, height=height)
self.assertEqual(size["width"], width,
"New width is {0} but should be {1}".format(size["width"], width))
self.assertEqual(size["height"], height,
"New height is {0} but should be {1}".format(size["height"], height))
def test_resize_to_new_size(self):
old = self.marionette.window_size
new = {"width": old["width"] + 10, "height": old["height"] + 10}
self.marionette.set_window_size(new["width"], new["height"])
actual = self.marionette.window_size
self.assertEqual(actual["width"], new["width"])
self.assertEqual(actual["height"], new["height"])
def test_resize_to_existing_size(self):
old = self.marionette.window_size
self.marionette.set_window_size(old["width"], old["height"])
new = self.marionette.window_size
self.assertEqual(old["width"], new["width"])
self.assertEqual(old["height"], new["height"])
def test_resize_larger_than_screen(self):
self.marionette.set_window_size(
self.max["width"] * 2, self.max["height"] * 2)
new = self.marionette.window_size
# in X the window size may be greater than the bounds of the screen
self.assertGreaterEqual(new["width"], self.max["width"])
self.assertGreaterEqual(new["height"], self.max["height"])

View File

@ -72,7 +72,7 @@ skip-if = appname == 'fennec'
[test_window_close_chrome.py]
skip-if = appname == 'fennec'
[test_window_close_content.py]
[test_window_position.py]
[test_window_rect.py]
skip-if = appname == 'fennec'
[test_window_status_content.py]
[test_window_status_chrome.py]
@ -100,8 +100,6 @@ skip-if = true # Bug 925688
skip-if = manage_instance == false || appname == 'fennec' # Bug 1298921 and bug 1322993
[test_quit_restart.py]
skip-if = manage_instance == false || appname == 'fennec' # Bug 1298921 and bug 1322993
[test_set_window_size.py]
skip-if = os == "linux" || appname == 'fennec' # Bug 1085717
[test_with_using_context.py]
[test_modal_dialogs.py]