Bug 1318351 - Ensure that window ids are internally handled as numbers. r=ato

Right now we retrieve window handles at different locations and as
such those are sometimes numbers but also strings. Given that we have
strict comparisons when checking for the id, checks can fail.

The patch also ensures that both close() and closeChromeWindow()
methods are returning the window ids as string.

MozReview-Commit-ID: ImOLe0Z2OE1

--HG--
extra : rebase_source : fe6e7857def2fde4e8253b1bfef8dd2a7d6bd954
This commit is contained in:
Henrik Skupin 2017-05-09 10:25:26 +02:00
Родитель e266de9045
Коммит 2ef12d4a3e
3 изменённых файлов: 88 добавлений и 26 удалений

Просмотреть файл

@ -505,8 +505,7 @@ GeckoDriver.prototype.registerBrowser = function (id, be) {
//
// TODO: Should have a better way of determining that this message
// is from a remote frame.
this.curBrowser.frameManager.currentRemoteFrame.targetFrameId =
this.generateFrameId(id);
this.curBrowser.frameManager.currentRemoteFrame.targetFrameId = id;
}
let reg = {};
@ -520,9 +519,8 @@ GeckoDriver.prototype.registerBrowser = function (id, be) {
if (this.appName != "Firefox" || be.namespaceURI != XUL_NS ||
be.nodeName != "browser" || be.getTabBrowser()) {
// curBrowser holds all the registered frames in knownFrames
let uid = this.generateFrameId(id);
reg.id = uid;
reg.remotenessChange = this.curBrowser.register(uid, be);
reg.id = id;
reg.remotenessChange = this.curBrowser.register(id, be);
}
// set to true if we updated mainContentId
@ -1233,7 +1231,6 @@ GeckoDriver.prototype.getIdForBrowser = function (browser) {
let winId = browser.outerWindowID;
if (winId) {
winId = winId.toString();
this._browserIds.set(permKey, winId);
return winId;
}
@ -1256,13 +1253,13 @@ GeckoDriver.prototype.getWindowHandle = function (cmd, resp) {
// curFrameId always holds the current tab.
if (this.curBrowser.curFrameId) {
resp.body.value = this.curBrowser.curFrameId;
resp.body.value = this.curBrowser.curFrameId.toString();
return;
}
for (let i in this.browsers) {
if (this.curBrowser == this.browsers[i]) {
resp.body.value = i;
resp.body.value = i.toString();
return;
}
}
@ -1439,14 +1436,20 @@ GeckoDriver.prototype.setWindowRect = function* (cmd, resp) {
* the window. Defaults to true.
*/
GeckoDriver.prototype.switchToWindow = function* (cmd, resp) {
let switchTo = cmd.parameters.name;
let focus = (cmd.parameters.focus !== undefined) ? cmd.parameters.focus : true;
let found;
// Window IDs are internally handled as numbers, but here it could also be the
// name of the window.
let switchTo = parseInt(cmd.parameters.name);
if (isNaN(switchTo)) {
switchTo = cmd.parameters.name;
}
let byNameOrId = function (name, windowId) {
return switchTo === name || switchTo === windowId;
};
let found;
let winEn = Services.wm.getEnumerator(null);
while (winEn.hasMoreElements()) {
let win = winEn.getNext();
@ -2608,7 +2611,7 @@ GeckoDriver.prototype.close = function (cmd, resp) {
this.mm.removeDelayedFrameScript(FRAME_SCRIPT);
}
return this.curBrowser.closeTab().then(() => this.windowHandles);
return this.curBrowser.closeTab().then(() => this.windowHandles.map(String));
};
/**
@ -2647,7 +2650,7 @@ GeckoDriver.prototype.closeChromeWindow = function (cmd, resp) {
this.mm.removeDelayedFrameScript(FRAME_SCRIPT);
}
return this.curBrowser.closeWindow().then(() => this.chromeWindowHandles);
return this.curBrowser.closeWindow().then(() => this.chromeWindowHandles.map(String));
};
/** Delete Marionette session. */
@ -3156,15 +3159,6 @@ GeckoDriver.prototype.uninstallAddon = function (cmd, resp) {
return addon.uninstall(id);
};
/**
* Helper function to convert an outerWindowID into a UID that Marionette
* tracks.
*/
GeckoDriver.prototype.generateFrameId = function (id) {
let uid = id + (this.appName == "B2G" ? "-b2g" : "");
return uid;
};
/** Receives all messages from content messageManager. */
GeckoDriver.prototype.receiveMessage = function (message) {
switch (message.name) {
@ -3433,9 +3427,7 @@ function copy (obj) {
* Returns the unique window ID.
*/
function getOuterWindowId(win) {
let id = win.QueryInterface(Ci.nsIInterfaceRequestor)
return win.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIDOMWindowUtils)
.outerWindowID;
return id.toString();
}

Просмотреть файл

@ -2,7 +2,9 @@
# 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_driver import By, Wait
import types
from marionette_driver import By, errors
from marionette_harness import MarionetteTestCase, WindowManagerMixin
@ -24,15 +26,30 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
super(TestWindowHandles, self).tearDown()
def assert_window_handles(self):
try:
self.assertIsInstance(self.marionette.current_chrome_window_handle, types.StringTypes)
self.assertIsInstance(self.marionette.current_window_handle, types.StringTypes)
except errors.NoSuchWindowException:
pass
for handle in self.marionette.chrome_window_handles:
self.assertIsInstance(handle, types.StringTypes)
for handle in self.marionette.window_handles:
self.assertIsInstance(handle, types.StringTypes)
def test_chrome_window_handles_with_scopes(self):
# Open a browser and a non-browser (about window) chrome window
self.open_window(
trigger=lambda: self.marionette.execute_script("window.open();"))
self.assert_window_handles()
self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1)
self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window)
self.open_window(
trigger=lambda: self.marionette.find_element(By.ID, "aboutName").click())
self.assert_window_handles()
self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 2)
self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window)
@ -53,11 +70,13 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# We open a new window but are actually interested in the new tab
new_win = self.open_window(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1)
self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window)
# Check that the new tab has the correct page loaded
self.marionette.switch_to_window(new_win)
self.assert_window_handles()
self.assertEqual(self.marionette.current_chrome_window_handle, new_win)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.empty_page)
@ -70,9 +89,11 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# Close the opened window and carry on in our original tab.
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows))
self.marionette.switch_to_window(self.start_window)
self.assert_window_handles()
self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.test_page)
@ -84,10 +105,12 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
link.click()
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.empty_page)
@ -99,12 +122,14 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
self.assertEqual(self.marionette.get_url(), other_page)
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.test_page)
self.marionette.switch_to_window(new_tab)
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
@ -118,11 +143,13 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# We open a new window but are actually interested in the new tab
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
# Check that the new tab has the correct page loaded
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.empty_page)
@ -135,9 +162,11 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# Close the opened window and carry on in our original tab.
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.test_page)
@ -149,13 +178,16 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
link.click()
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
with self.marionette.using_context("content"):
self.assertEqual(self.marionette.get_url(), self.empty_page)
@ -180,6 +212,7 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
menu_new_tab.click()
new_tab = self.open_tab(trigger=open_with_menu)
self.assert_window_handles()
# We still have the default tab set as our window handle. This
# get_url command should be sent immediately, and not be forever-queued.
@ -190,10 +223,17 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
def test_window_handles_after_closing_last_window(self):
self.close_all_windows()
self.assertEqual(self.marionette.close_chrome_window(), [])

Просмотреть файл

@ -2,7 +2,9 @@
# 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_driver import By, Wait
import types
from marionette_driver import By, errors, Wait
from marionette_harness import MarionetteTestCase, skip_if_mobile, WindowManagerMixin
@ -21,16 +23,27 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
super(TestWindowHandles, self).tearDown()
def assert_window_handles(self):
try:
self.assertIsInstance(self.marionette.current_window_handle, types.StringTypes)
except errors.NoSuchWindowException:
pass
for handle in self.marionette.window_handles:
self.assertIsInstance(handle, types.StringTypes)
def test_window_handles_after_opening_new_tab(self):
def open_with_link():
link = self.marionette.find_element(By.ID, "new-tab")
link.click()
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
Wait(self.marionette, timeout=self.marionette.timeout.page_load).until(
lambda mn: mn.get_url() == self.empty_page,
@ -42,9 +55,11 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
self.marionette.switch_to_window(new_tab)
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
def test_window_handles_after_opening_new_browser_window(self):
@ -54,11 +69,13 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# We open a new window but are actually interested in the new tab
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
# Check that the new tab has the correct page loaded
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
Wait(self.marionette, self.marionette.timeout.page_load).until(
lambda _: self.marionette.get_url() == self.empty_page,
@ -71,9 +88,11 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# Close the opened window and carry on in our original tab.
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.assertEqual(self.marionette.get_url(), self.test_page)
@ -86,17 +105,21 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
# We open a new window but are actually interested in the new tab
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
# Close the opened window and carry on in our original tab.
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(self.start_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
def test_window_handles_after_closing_original_tab(self):
@ -105,14 +128,21 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase):
link.click()
new_tab = self.open_tab(trigger=open_with_link)
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1)
self.assertEqual(self.marionette.current_window_handle, self.start_tab)
self.marionette.close()
self.assert_window_handles()
self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs))
self.marionette.switch_to_window(new_tab)
self.assert_window_handles()
self.assertEqual(self.marionette.current_window_handle, new_tab)
Wait(self.marionette, self.marionette.timeout.page_load).until(
lambda _: self.marionette.get_url() == self.empty_page,
message="The expected page '{}' has not been loaded".format(self.empty_page))
def test_window_handles_after_closing_last_tab(self):
self.close_all_tabs()
self.assertEqual(self.marionette.close(), [])