Bug 1332122 - Re-register the browser for frame script reloads.
With multi-processes for content reloads of the frame script
can happen at any time, and not only for remoteness changes.
As such the current browser has to be re-registered with the
new id of the listener.
MozReview-Commit-ID: 48MOZfuPTR9
--- a/testing/marionette/browser.js
+++ b/testing/marionette/browser.js
@@ -73,18 +73,18 @@ browser.getTabBrowser = function(win) {
* @param {nsIDOMWindow} win
* The window whose browser needs to be accessed.
* @param {GeckoDriver} driver
* Reference to the driver the browser is attached to.
*/
browser.Context = class {
/**
- * @param {<xul:browser>} win
- * Frame that is expected to contain the view of the web document.
+ * @param {ChromeWindow} win
+ * ChromeWindow that contains the top-level browsing context.
* @param {GeckoDriver} driver
* Reference to driver instance.
*/
constructor(win, driver) {
this.window = win;
this.driver = driver;
// In Firefox this is <xul:tabbrowser> (not <xul:browser>!)
@@ -101,29 +101,34 @@ browser.Context = class {
// A reference to the tab corresponding to the current window handle,
// if any. Specifically, this.tab refers to the last tab that Marionette
// switched to in this browser window. Note that this may not equal the
// currently selected tab. For example, if Marionette switches to tab
// A, and then clicks on a button that opens a new tab B in the same
// browser window, this.tab will still point to tab A, despite tab B
// being the currently selected tab.
this.tab = null;
+
+ // Commands which trigger a page load can cause the frame script to be
+ // reloaded. To not loose the currently active command, or any other
+ // already pushed following command, store them as long as they haven't
+ // been fully processed. The commands get flushed after a new browser
+ // has been registered.
this.pendingCommands = [];
+ this._needsFlushPendingCommands = false;
// We should have one frame.Manager per browser.Context so that we
// can handle modals in each <xul:browser>.
this.frameManager = new frame.Manager(driver);
this.frameRegsPending = 0;
// register all message listeners
this.frameManager.addMessageManagerListeners(driver.mm);
this.getIdForBrowser = driver.getIdForBrowser.bind(driver);
this.updateIdForBrowser = driver.updateIdForBrowser.bind(driver);
- this._browserWasRemote = null;
- this._hasRemotenessChange = false;
}
/**
* Returns the content browser for the currently selected tab.
* If there is no tab selected, null will be returned.
*/
get contentBrowser() {
if (this.tab) {
@@ -281,18 +286,17 @@ browser.Context = class {
* @param {string} uri
* URI to open.
*/
addTab(uri) {
return this.tabBrowser.addTab(uri, true);
}
/**
- * Set the current tab and update remoteness tracking if a tabbrowser
- * is available.
+ * Set the current tab.
*
* @param {number=} index
* Tab index to switch to. If the parameter is undefined,
* the currently selected tab will be used.
* @param {nsIDOMWindow=} win
* Switch to this window before selecting the tab.
* @param {boolean=} focus
* A boolean value which determins whether to focus
@@ -325,103 +329,71 @@ browser.Context = class {
// Firefox
this.tabBrowser.selectedTab = this.tab;
} else {
throw new UnsupportedOperationError("switchToTab() not supported");
}
}
}
-
- if (this.driver.appName == "Firefox") {
- this._browserWasRemote = this.contentBrowser.isRemoteBrowser;
- this._hasRemotenessChange = false;
- }
}
/**
* Registers a new frame, and sets its current frame id to this frame
* if it is not already assigned, and if a) we already have a session
* or b) we're starting a new session and it is the right start frame.
*
* @param {string} uid
* Frame uid for use by Marionette.
- * @param the XUL <browser> that was the target of the originating message.
+ * @param {xul:browser} target
+ * The <xul:browser> that was the target of the originating message.
*/
register(uid, target) {
- let remotenessChange = this.hasRemotenessChange();
- if (this.curFrameId === null || remotenessChange) {
- if (this.tabBrowser) {
- // If we're setting up a new session on Firefox, we only process the
- // registration for this frame if it belongs to the current tab.
- if (!this.tab) {
- this.switchToTab();
- }
+ if (this.tabBrowser) {
+ // If we're setting up a new session on Firefox, we only process the
+ // registration for this frame if it belongs to the current tab.
+ if (!this.tab) {
+ this.switchToTab();
+ }
- if (target === this.contentBrowser) {
- this.updateIdForBrowser(this.contentBrowser, uid);
- }
+ if (target === this.contentBrowser) {
+ this.updateIdForBrowser(this.contentBrowser, uid);
+ this._needsFlushPendingCommands = true;
}
}
// used to delete sessions
this.knownFrames.push(uid);
- return remotenessChange;
}
/**
- * When navigating between pages results in changing a browser's
- * process, we need to take measures not to lose contact with a listener
- * script. This function does the necessary bookkeeping.
- */
- hasRemotenessChange() {
- // None of these checks are relevant if we don't have a tab yet,
- // and may not apply on Fennec.
- if (this.driver.appName != "Firefox" ||
- this.tab === null ||
- this.contentBrowser === null) {
- return false;
- }
-
- if (this._hasRemotenessChange) {
- return true;
- }
-
- let currentIsRemote = this.contentBrowser.isRemoteBrowser;
- this._hasRemotenessChange = this._browserWasRemote !== currentIsRemote;
- this._browserWasRemote = currentIsRemote;
- return this._hasRemotenessChange;
- }
-
- /**
- * Flushes any pending commands queued when a remoteness change is being
- * processed and mark this remotenessUpdate as complete.
+ * Flushes any queued pending commands after a reload of the frame script.
*/
flushPendingCommands() {
- if (!this._hasRemotenessChange) {
+ if (!this._needsFlushPendingCommands) {
return;
}
- this._hasRemotenessChange = false;
this.pendingCommands.forEach(cb => cb());
this.pendingCommands = [];
+ this._needsFlushPendingCommands = false;
}
/**
* This function intercepts commands interacting with content and queues
* or executes them as needed.
*
* No commands interacting with content are safe to process until
* the new listener script is loaded and registers itself.
* This occurs when a command whose effect is asynchronous (such
- * as goBack) results in a remoteness change and new commands
+ * as goBack) results in a reload of the frame script and new commands
* are subsequently posted to the server.
*/
executeWhenReady(cb) {
- if (this.hasRemotenessChange()) {
+ if (this._needsFlushPendingCommands) {
this.pendingCommands.push(cb);
} else {
cb();
}
}
};
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -566,42 +566,39 @@ GeckoDriver.prototype.registerBrowser =
// Marionette:switchToFrame, so we'll acknowledge the switchToFrame
// message here.
//
// TODO: Should have a better way of determining that this message
// is from a remote frame.
this.curBrowser.frameManager.currentRemoteFrame.targetFrameId = id;
}
- let reg = {};
-
// We want to ignore frames that are XUL browsers that aren't in the "main"
// tabbrowser, but accept things on Fennec (which doesn't have a
// xul:tabbrowser), and accept HTML iframes (because tests depend on it),
// as well as XUL frames. Ideally this should be cleaned up and we should
// keep track of browsers a different way.
if (this.appName != "Firefox" || be.namespaceURI != XUL_NS ||
be.nodeName != "browser" || be.getTabBrowser()) {
// curBrowser holds all the registered frames in knownFrames
- reg.id = id;
- reg.remotenessChange = this.curBrowser.register(id, be);
+ this.curBrowser.register(id, be);
}
- this.wins.set(reg.id, listenerWindow);
+ this.wins.set(id, listenerWindow);
if (nullPrevious && (this.curBrowser.curFrameId !== null)) {
this.sendAsync(
"newSession",
this.capabilities.toJSON(),
this.newSessionCommandId);
if (this.curBrowser.isNewSession) {
this.newSessionCommandId = null;
}
}
- return [reg, this.capabilities.toJSON()];
+ return [id, this.capabilities.toJSON()];
};
GeckoDriver.prototype.registerPromise = function() {
const li = "Marionette:register";
return new Promise(resolve => {
let cb = msg => {
let wid = msg.json.value;
@@ -621,20 +618,23 @@ GeckoDriver.prototype.registerPromise =
return rv;
};
this.mm.addMessageListener(li, cb);
});
};
GeckoDriver.prototype.listeningPromise = function() {
const li = "Marionette:listenersAttached";
+
return new Promise(resolve => {
- let cb = () => {
- this.mm.removeMessageListener(li, cb);
- resolve();
+ let cb = msg => {
+ if (msg.json.listenerId === this.curBrowser.curFrameId) {
+ this.mm.removeMessageListener(li, cb);
+ resolve();
+ }
};
this.mm.addMessageListener(li, cb);
});
};
/** Create a new session. */
GeckoDriver.prototype.newSession = function* (cmd, resp) {
if (this.sessionId) {
@@ -973,19 +973,19 @@ GeckoDriver.prototype.get = function* (c
assert.content(this.context);
assert.window(this.getCurrentWindow());
assert.noUserPrompt(this.dialog);
let url = cmd.parameters.url;
let get = this.listener.get({url, pageTimeout: this.timeouts.pageLoad});
- // If a remoteness update interrupts our page load, this will never return
- // We need to re-issue this request to correctly poll for readyState and
- // send errors.
+ // If a reload of the frame script interrupts our page load, this will
+ // never return. We need to re-issue this request to correctly poll for
+ // readyState and send errors.
this.curBrowser.pendingCommands.push(() => {
let parameters = {
// TODO(ato): Bug 1242595
command_id: this.listener.activeMessageId,
pageTimeout: this.timeouts.pageLoad,
startTime: new Date().getTime(),
};
this.mm.broadcastAsyncMessage(
@@ -1092,19 +1092,19 @@ GeckoDriver.prototype.goBack = function*
// If there is no history, just return
if (!this.curBrowser.contentBrowser.webNavigation.canGoBack) {
return;
}
let lastURL = this.currentURL;
let goBack = this.listener.goBack({pageTimeout: this.timeouts.pageLoad});
- // If a remoteness update interrupts our page load, this will never return
- // We need to re-issue this request to correctly poll for readyState and
- // send errors.
+ // If a reload of the frame script interrupts our page load, this will
+ // never return. We need to re-issue this request to correctly poll for
+ // readyState and send errors.
this.curBrowser.pendingCommands.push(() => {
let parameters = {
// TODO(ato): Bug 1242595
command_id: this.listener.activeMessageId,
lastSeenURL: lastURL.toString(),
pageTimeout: this.timeouts.pageLoad,
startTime: new Date().getTime(),
};
@@ -1136,19 +1136,19 @@ GeckoDriver.prototype.goForward = functi
if (!this.curBrowser.contentBrowser.webNavigation.canGoForward) {
return;
}
let lastURL = this.currentURL;
let goForward = this.listener.goForward(
{pageTimeout: this.timeouts.pageLoad});
- // If a remoteness update interrupts our page load, this will never return
- // We need to re-issue this request to correctly poll for readyState and
- // send errors.
+ // If a reload of the frame script interrupts our page load, this will
+ // never return. We need to re-issue this request to correctly poll for
+ // readyState and send errors.
this.curBrowser.pendingCommands.push(() => {
let parameters = {
// TODO(ato): Bug 1242595
command_id: this.listener.activeMessageId,
lastSeenURL: lastURL.toString(),
pageTimeout: this.timeouts.pageLoad,
startTime: new Date().getTime(),
};
@@ -1171,17 +1171,35 @@ GeckoDriver.prototype.goForward = functi
* @throws {UnexpectedAlertOpenError}
* A modal dialog is open, blocking this operation.
*/
GeckoDriver.prototype.refresh = function* (cmd, resp) {
assert.content(this.context);
assert.window(this.getCurrentWindow());
assert.noUserPrompt(this.dialog);
- yield this.listener.refresh({pageTimeout: this.timeouts.pageLoad});
+ let refresh = this.listener.refresh(
+ {pageTimeout: this.timeouts.pageLoad})
+
+ // If a reload of the frame script interrupts our page load, this will
+ // never return. We need to re-issue this request to correctly poll for
+ // readyState and send errors.
+ this.curBrowser.pendingCommands.push(() => {
+ let parameters = {
+ // TODO(ato): Bug 1242595
+ command_id: this.listener.activeMessageId,
+ pageTimeout: this.timeouts.pageLoad,
+ startTime: new Date().getTime(),
+ };
+ this.mm.broadcastAsyncMessage(
+ "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId,
+ parameters);
+ });
+
+ yield refresh;
};
/**
* Forces an update for the given browser's id.
*/
GeckoDriver.prototype.updateIdForBrowser = function(browser, newId) {
this._browserIds.set(browser.permanentKey, newId);
};
@@ -2064,19 +2082,19 @@ GeckoDriver.prototype.clickElement = fun
// so we need to listen for it and then just send an error back.
// The person making the call should be aware something is not right
// and handle accordingly.
this.addFrameCloseListener("click");
let click = this.listener.clickElement(
{id, pageTimeout: this.timeouts.pageLoad});
- // If a remoteness update interrupts our page load, this will
- // never return We need to re-issue this request to correctly poll
- // for readyState and send errors.
+ // If a reload of the frame script interrupts our page load, this will
+ // never return. We need to re-issue this request to correctly poll for
+ // readyState and send errors.
this.curBrowser.pendingCommands.push(() => {
let parameters = {
// TODO(ato): Bug 1242595
command_id: this.listener.activeMessageId,
pageTimeout: this.timeouts.pageLoad,
startTime: new Date().getTime(),
};
this.mm.broadcastAsyncMessage(
@@ -3173,19 +3191,19 @@ GeckoDriver.prototype.receiveMessage = f
case "Marionette:register":
let wid = message.json.value;
let be = message.target;
let rv = this.registerBrowser(wid, be);
return rv;
case "Marionette:listenersAttached":
if (message.json.listenerId === this.curBrowser.curFrameId) {
- // If remoteness gets updated we need to call newSession. In the case
- // of desktop this just sets up a small amount of state that doesn't
- // change over the course of a session.
+ // If the frame script gets reloaded we need to call newSession.
+ // In the case of desktop this just sets up a small amount of state
+ // that doesn't change over the course of a session.
this.sendAsync("newSession", this.capabilities.toJSON());
this.curBrowser.flushPendingCommands();
}
break;
}
};
/* eslint-enable consistent-return */
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -49,17 +49,16 @@ Cu.import("chrome://marionette/content/s
Cu.importGlobalProperties(["URL"]);
var marionetteTestName;
var winUtil = content.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIDOMWindowUtils);
var listenerId = null; // unique ID of this listener
var curContainer = {frame: content, shadowRoot: null};
-var isRemoteBrowser = () => curContainer.frame.contentWindow !== null;
var previousContainer = null;
var seenEls = new element.Store();
var SUPPORTED_STRATEGIES = new Set([
element.Strategy.ClassName,
element.Strategy.Selector,
element.Strategy.ID,
element.Strategy.Name,
@@ -113,20 +112,20 @@ var modalHandler = function() {
curContainer = {frame: content, shadowRoot: null};
};
// sandbox storage and name of the current sandbox
var sandboxes = new Sandboxes(() => curContainer.frame);
var sandboxName = "default";
/**
- * The load listener singleton helps to keep track of active page
- * load activities, and can be used by any command which might cause a
- * navigation to happen. In the specific case of remoteness changes it
- * allows to continue observing the current page load.
+ * The load listener singleton helps to keep track of active page load
+ * activities, and can be used by any command which might cause a navigation
+ * to happen. In the specific case of a reload of the frame script it allows
+ * to continue observing the current page load.
*/
var loadListener = {
command_id: null,
seenBeforeUnload: false,
seenUnload: false,
timeout: null,
timerPageLoad: null,
timerPageUnload: null,
@@ -152,17 +151,17 @@ var loadListener = {
this.seenBeforeUnload = false;
this.seenUnload = false;
this.timerPageLoad = Cc["@mozilla.org/timer;1"]
.createInstance(Ci.nsITimer);
this.timerPageUnload = null;
- // In case of a remoteness change, only wait the remaining time
+ // In case the frame script has been reloaded, wait the remaining time
timeout = startTime + timeout - new Date().getTime();
if (timeout <= 0) {
this.notify(this.timerPageLoad);
return;
}
if (waitForUnloaded) {
@@ -207,18 +206,18 @@ var loadListener = {
curContainer.frame.removeEventListener("beforeunload", this);
curContainer.frame.removeEventListener("unload", this);
} catch (e) {
if (e.name != "TypeError") {
throw e;
}
}
- // In the case when the observer was added before a remoteness change,
- // it will no longer be available. Exceptions can be silently ignored.
+ // In case the observer was added before the frame script has been
+ // reloaded, it will no longer be available. Exceptions can be ignored.
try {
Services.obs.removeObserver(this, "outer-window-destroyed");
} catch (e) {}
},
/**
* Callback for registered DOM events.
*/
@@ -339,29 +338,29 @@ var loadListener = {
this.stop();
sendOk(this.command_id);
}
break;
}
},
/**
- * Continue to listen for page load events after a remoteness change
- * happened.
+ * Continue to listen for page load events after the frame script has been
+ * reloaded.
*
* @param {number} command_id
* ID of the currently handled message between the driver and
* listener.
* @param {number} timeout
* Timeout in milliseconds the method has to wait for the page
* being finished loading.
* @param {number} startTime
* Unix timestap when the navitation request got triggered.
*/
- waitForLoadAfterRemotenessChange(command_id, timeout, startTime) {
+ waitForLoadAfterFramescriptReload(command_id, timeout, startTime) {
this.start(command_id, timeout, startTime, false);
},
/**
* Use a trigger callback to initiate a page load, and attach listeners if
* a page load is expected.
*
* @param {function} trigger
@@ -425,28 +424,23 @@ var loadListener = {
*/
function registerSelf() {
let msg = {value: winUtil.outerWindowID};
logger.debug(`Register listener.js for window ${msg.value}`);
// register will have the ID and a boolean describing if this is the
// main process or not
let register = sendSyncMessage("Marionette:register", msg);
-
if (register[0]) {
- let {id, remotenessChange} = register[0][0];
+ listenerId = register[0][0];
capabilities = session.Capabilities.fromJSON(register[0][1]);
- listenerId = id;
- if (typeof id != "undefined") {
+ if (typeof listenerId != "undefined") {
startListeners();
- let rv = {};
- if (remotenessChange) {
- rv.listenerId = id;
- }
- sendAsyncMessage("Marionette:listenersAttached", rv);
+ sendAsyncMessage("Marionette:listenersAttached",
+ {"listenerId": listenerId});
}
}
}
// Eventually we will not have a closure for every single command, but
// use a generic dispatch for all listener commands.
//
// Perhaps one could even conceive having a separate instance of
@@ -1100,34 +1094,34 @@ function multiAction(args, maxLen) {
* current navigation request in case we're interupted by an onbeforeunload
* handler and navigation doesn't complete.
*/
function cancelRequest() {
loadListener.stop();
}
/**
- * This implements the latter part of a get request (for the case we
- * need to resume one when a remoteness update happens in the middle of a
+ * This implements the latter part of a get request (for the case we need
+ * to resume one when the frame script has been reloaded in the middle of a
* navigate request). This is most of of the work of a navigate request,
* but doesn't assume DOMContentLoaded is yet to fire.
*
* @param {number} command_id
* ID of the currently handled message between the driver and
* listener.
* @param {number} pageTimeout
* Timeout in seconds the method has to wait for the page being
* finished loading.
* @param {number} startTime
* Unix timestap when the navitation request got triggred.
*/
function waitForPageLoaded(msg) {
let {command_id, pageTimeout, startTime} = msg.json;
- loadListener.waitForLoadAfterRemotenessChange(
+ loadListener.waitForLoadAfterFramescriptReload(
command_id, pageTimeout, startTime);
}
/**
* Navigate to the given URL. The operation will be performed on the
* current browsing context, which means it handles the case where we
* navigate within an iframe. All other navigation is handled by the driver
* (in chrome space).