Last active
June 7, 2017 17:11
-
-
Save florentbr/b9a878a0a9ee116ee3c59ff419e7c810 to your computer and use it in GitHub Desktop.
Patch for marionette issue #729
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--- testing/marionette/driver.js 2017-06-07 12:34:43.000000000 +0100 | |
+++ testing/marionette/driver.js 2017-06-07 16:08:25.014583900 +0100 | |
@@ -887,7 +887,8 @@ | |
} | |
opts.timeout = timeout; | |
- let wargs = evaluate.fromJSON(args, this.curBrowser.seenEls, sb.window); | |
+ let container = {frame: sb.window, shadowRoot: undefined}; | |
+ let wargs = evaluate.fromJSON(args, this.curBrowser.seenEls, container); | |
let evaluatePromise = evaluate.sandbox(sb, script, wargs, opts); | |
return evaluatePromise.then(res => evaluate.toJSON(res, this.curBrowser.seenEls)); | |
} | |
@@ -913,7 +914,8 @@ | |
switch (this.context) { | |
case Context.CHROME: | |
- let wargs = evaluate.fromJSON(args, this.curBrowser.seenEls, win); | |
+ let container = {frame: win, shadowRoot: undefined}; | |
+ let wargs = evaluate.fromJSON(args, this.curBrowser.seenEls, container); | |
let harness = new simpletest.Harness( | |
win, | |
Context.CHROME, | |
@@ -1914,7 +1916,7 @@ | |
}; | |
switch (this.context) { | |
- case Context.CHROME: | |
+ case Context.CHROME: { | |
if (!SUPPORTED_STRATEGIES.has(strategy)) { | |
throw new InvalidSelectorError(`Strategy not supported: ${strategy}`); | |
} | |
@@ -1929,13 +1931,15 @@ | |
resp.body.value = webEl; | |
break; | |
+ } | |
- case Context.CONTENT: | |
- resp.body.value = yield this.listener.findElementContent( | |
- strategy, | |
- expr, | |
- opts); | |
+ case Context.CONTENT: { | |
+ let elRef = yield this.listener.findElementContent(strategy, expr, opts); | |
+ let webEl = element.makeWebElement(elRef); | |
+ | |
+ resp.body.value = webEl; | |
break; | |
+ } | |
} | |
}; | |
@@ -1959,7 +1963,7 @@ | |
}; | |
switch (this.context) { | |
- case Context.CHROME: | |
+ case Context.CHROME: { | |
if (!SUPPORTED_STRATEGIES.has(strategy)) { | |
throw new InvalidSelectorError(`Strategy not supported: ${strategy}`); | |
} | |
@@ -1974,13 +1978,18 @@ | |
let webEls = elRefs.map(element.makeWebElement); | |
resp.body = webEls; | |
break; | |
+ } | |
- case Context.CONTENT: | |
- resp.body = yield this.listener.findElementsContent( | |
+ case Context.CONTENT: { | |
+ let elRefs = yield this.listener.findElementsContent( | |
cmd.parameters.using, | |
cmd.parameters.value, | |
opts); | |
+ | |
+ let webEls = elRefs.map(element.makeWebElement); | |
+ resp.body = webEls; | |
break; | |
+ } | |
} | |
}; | |
--- testing/marionette/element.js 2017-06-07 12:34:43.000000000 +0100 | |
+++ testing/marionette/element.js 2017-06-07 16:09:13.255744300 +0100 | |
@@ -22,13 +22,13 @@ | |
* A web element is an abstraction used to identify an element when it | |
* is transported across the protocol, between remote- and local ends. | |
* | |
- * Each element has an associated web element reference (a UUID) that | |
- * uniquely identifies the the element across all browsing contexts. The | |
+ * Each element has an associated web element reference (a UID) that | |
+ * uniquely identifies the element across all browsing contexts. The | |
* web element reference for every element representing the same element | |
* is the same. | |
* | |
* The @code{element.Store} provides a mapping between web element | |
- * references and DOM elements for each browsing context. It also provides | |
+ * references and DOM elements for each browsing context. It also provides | |
* functionality for looking up and retrieving elements. | |
*/ | |
@@ -64,15 +64,27 @@ | |
* | |
* Elements are added by calling |add(el)| or |addAll(elements)|, and | |
* may be queried by their web element reference using |get(element)|. | |
+ * | |
+ * A reference or unique indentifier (UID) includes the timestamp of | |
+ * this store to prevent the usage between sessions and a number | |
+ * incremented for each new reference. | |
+ * | |
+ * Uses a double linked dictionary holding weak references and | |
+ * integers to keep a low complexity and to reduce the memory | |
+ * footprint. | |
*/ | |
element.Store = class { | |
+ | |
constructor() { | |
- this.els = {}; | |
- this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); | |
+ this.els = new Map(); // {<Number>: <Weak HTMLElement>, ...} | |
+ this.ids = new WeakMap(); // {<Weak HTMLElement>: <Number>, ...} | |
+ this.storeId = Date.now().toString(36).slice(-6) + '-'; | |
+ this.lowerId = 0; | |
+ this.upperId = -1; | |
} | |
clear() { | |
- this.els = {}; | |
+ this.constructor(); | |
} | |
/** | |
@@ -84,13 +96,13 @@ | |
* @param {NodeList} els | |
* Sequence of elements to add to set of seen elements. | |
* | |
- * @return {Array.<WebElement>} | |
+ * @return {Array.<String>} | |
* List of the web element references associated with each element | |
* from |els|. | |
*/ | |
addAll(els) { | |
- let add = this.add.bind(this); | |
- return [...els].map(add); | |
+ let uids = Array.map(els, this.add, this); | |
+ return uids; | |
} | |
/** | |
@@ -100,52 +112,61 @@ | |
* Element to add to set of seen elements. | |
* | |
* @return {string} | |
- * Web element reference associated with element. | |
+ * Web element reference associated with element. | |
*/ | |
add(el) { | |
- for (let i in this.els) { | |
- let foundEl; | |
- try { | |
- foundEl = this.els[i].get(); | |
- } catch (e) {} | |
- if (foundEl) { | |
- if (new XPCNativeWrapper(foundEl) == new XPCNativeWrapper(el)) { | |
- return i; | |
- } | |
+ // use XPCNativeWrapper to store the element; see bug 834266 | |
+ let wrappedEl = new XPCNativeWrapper(el); | |
+ let id = this.ids.get(wrappedEl); | |
- // cleanup reference to gc'd element | |
- } else { | |
- delete this.els[i]; | |
+ if (id == undefined) { | |
+ | |
+ id = ++this.upperId; | |
+ | |
+ this.ids.set(wrappedEl, id); | |
+ this.els.set(id, Cu.getWeakReference(wrappedEl)); | |
+ | |
+ // cleanup first-in gc'd elements. | |
+ while (!this.els.get(this.lowerId).get()) { | |
+ this.els.delete(this.lowerId++); | |
} | |
} | |
- let id = element.generateUUID(); | |
- this.els[id] = Cu.getWeakReference(el); | |
- return id; | |
+ let uid = this.storeId + id.toString(36); | |
+ return uid; | |
} | |
/** | |
* Determine if the provided web element reference has been seen | |
* before/is in the element store. | |
* | |
- * @param {string} uuid | |
+ * @param {string} uid | |
* Element's associated web element reference. | |
* | |
* @return {boolean} | |
* True if element is in the store, false otherwise. | |
*/ | |
- has(uuid) { | |
- return Object.keys(this.els).includes(uuid); | |
+ has(uid) { | |
+ | |
+ if (uid && uid.startsWith(this.storeId)) { | |
+ | |
+ let id = parseInt(uid.substring(this.storeId.length), 36); | |
+ let elWeakRef = this.els.get(id); | |
+ | |
+ return !!(elWeakRef && elWeakRef.get()); | |
+ } | |
+ | |
+ return false; | |
} | |
/** | |
- * Retrieve a DOM element by its unique web element reference/UUID. | |
+ * Retrieve a DOM element by its unique web element reference/UID. | |
* | |
- * @param {string} uuid | |
- * Web element reference, or UUID. | |
+ * @param {string} uid | |
+ * Web element reference, or UID. | |
* @param {(nsIDOMWindow|ShadowRoot)} container | |
- * Window and an optional shadow root that contains the element. | |
+ * Window and an optional shadow root that contains the element. | |
* | |
* @returns {nsIDOMElement} | |
* Element associated with reference. | |
@@ -156,40 +177,51 @@ | |
* If element has gone stale, indicating it is no longer attached to | |
* the DOM provided in the container. | |
*/ | |
- get(uuid, container) { | |
- let el = this.els[uuid]; | |
- if (!el) { | |
- throw new JavaScriptError(`Element reference not seen before: ${uuid}`); | |
- } | |
+ get(uid, container) { | |
- try { | |
- el = el.get(); | |
- } catch (e) { | |
- el = null; | |
- delete this.els[id]; | |
+ if (!uid.startsWith(this.storeId)) { | |
+ throw new JavaScriptError(`Element reference not seen before: ${uid}`); | |
} | |
- // use XPCNativeWrapper to compare elements (see bug 834266) | |
- let wrappedFrame = new XPCNativeWrapper(container.frame); | |
- let wrappedShadowRoot; | |
- if (container.shadowRoot) { | |
- wrappedShadowRoot = new XPCNativeWrapper(container.shadowRoot); | |
- } | |
- let wrappedEl = new XPCNativeWrapper(el); | |
- let wrappedContainer = { | |
- frame: wrappedFrame, | |
- shadowRoot: wrappedShadowRoot, | |
- }; | |
- if (!el || | |
- !(wrappedEl.ownerDocument == wrappedFrame.document) || | |
- element.isDisconnected(wrappedEl, wrappedContainer)) { | |
- throw new StaleElementReferenceError( | |
- error.pprint`The element reference of ${el} stale: ` + | |
- "either the element is no longer attached to the DOM " + | |
- "or the page has been refreshed"); | |
+ let id = parseInt(uid.substring(this.storeId.length), 36); | |
+ let elWeakRef = this.els.get(id); | |
+ let wrappedEl = elWeakRef && elWeakRef.get(); | |
+ | |
+ if (wrappedEl) { | |
+ | |
+ // fast stale evaluation for the most favorable case. | |
+ if (container.frame.document.contains(wrappedEl)) { | |
+ return wrappedEl; | |
+ } | |
+ | |
+ // fallback on a slow stale evaluation. | |
+ else { | |
+ // use XPCNativeWrapper to compare elements (see bug 834266) | |
+ let wrappedFrame = new XPCNativeWrapper(container.frame); | |
+ | |
+ let wrappedShadowRoot; | |
+ if (container.shadowRoot) { | |
+ wrappedShadowRoot = new XPCNativeWrapper(container.shadowRoot); | |
+ } | |
+ | |
+ let wrappedContainer = { | |
+ frame: wrappedFrame, | |
+ shadowRoot: wrappedShadowRoot | |
+ }; | |
+ | |
+ if (wrappedEl.ownerDocument == wrappedFrame.document && | |
+ !element.isDisconnected(wrappedEl, wrappedContainer)) { | |
+ return el; | |
+ } | |
+ | |
+ } | |
+ | |
} | |
- return el; | |
+ throw new StaleElementReferenceError( | |
+ error.pprint`The element reference of ${uid} stale: ` + | |
+ "either the element is no longer attached to the DOM " + | |
+ "or the page has been refreshed"); | |
} | |
}; | |
@@ -568,24 +600,6 @@ | |
} | |
} | |
-/** Determines if |obj| is an HTML or JS collection. */ | |
-element.isCollection = function (seq) { | |
- switch (Object.prototype.toString.call(seq)) { | |
- case "[object Arguments]": | |
- case "[object Array]": | |
- case "[object FileList]": | |
- case "[object HTMLAllCollection]": | |
- case "[object HTMLCollection]": | |
- case "[object HTMLFormControlsCollection]": | |
- case "[object HTMLOptionsCollection]": | |
- case "[object NodeList]": | |
- return true; | |
- | |
- default: | |
- return false; | |
- } | |
-}; | |
- | |
element.makeWebElement = function (uuid) { | |
return { | |
[element.Key]: uuid, | |
--- testing/marionette/evaluate.js 2017-06-07 12:34:43.000000000 +0100 | |
+++ testing/marionette/evaluate.js 2017-06-07 16:10:08.675638600 +0100 | |
@@ -182,49 +182,50 @@ | |
* Arbitrary object containing web elements. | |
* @param {element.Store} seenEls | |
* Element store to use for lookup of web element references. | |
- * @param {Window} win | |
- * Window. | |
- * @param {ShadowRoot} shadowRoot | |
- * Shadow root. | |
+ * @param {(nsIDOMWindow|ShadowRoot)} container | |
+ * Window and an optional shadow root that contains the element. | |
* | |
* @return {?} | |
* Same object as provided by |obj| with the web elements replaced | |
* by DOM elements. | |
*/ | |
-evaluate.fromJSON = function (obj, seenEls, win, shadowRoot = undefined) { | |
+evaluate.fromJSON = function fromJSON(obj, seenEls, container) { | |
+ | |
switch (typeof obj) { | |
+ | |
case "boolean": | |
case "number": | |
case "string": | |
return obj; | |
case "object": | |
+ | |
if (obj === null) { | |
- return obj; | |
+ return null; | |
} | |
// arrays | |
- else if (Array.isArray(obj)) { | |
- return obj.map(e => evaluate.fromJSON(e, seenEls, win, shadowRoot)); | |
+ if (Array.isArray(obj)) { | |
+ return obj.map(e => fromJSON(e, seenEls, container)); | |
} | |
// web elements | |
- else if (Object.keys(obj).includes(element.Key) || | |
- Object.keys(obj).includes(element.LegacyKey)) { | |
- let uuid = obj[element.Key] || obj[element.LegacyKey]; | |
- let el = seenEls.get(uuid, {frame: win, shadowRoot: shadowRoot}); | |
- if (!el) { | |
- throw new WebDriverError(`Unknown element: ${uuid}`); | |
+ { | |
+ let uid = obj[element.Key] || obj[element.LegacyKey]; | |
+ | |
+ if (typeof uid == "string") { | |
+ return seenEls.get(uid, container); | |
} | |
- return el; | |
} | |
// arbitrary objects | |
- else { | |
+ { | |
let rv = {}; | |
+ | |
for (let prop in obj) { | |
- rv[prop] = evaluate.fromJSON(obj[prop], seenEls, win, shadowRoot); | |
+ rv[prop] = fromJSON(obj[prop], seenEls, container); | |
} | |
+ | |
return rv; | |
} | |
} | |
@@ -246,47 +247,87 @@ | |
* Same object as provided by |obj| with the elements replaced by | |
* web elements. | |
*/ | |
-evaluate.toJSON = function (obj, seenEls) { | |
- const t = Object.prototype.toString.call(obj); | |
+evaluate.toJSON = function toJSON(obj, seenEls) { | |
- // null | |
- if (t == "[object Undefined]" || t == "[object Null]") { | |
- return null; | |
- } | |
+ switch (typeof obj) { | |
- // literals | |
- else if (t == "[object Boolean]" || t == "[object Number]" || t == "[object String]") { | |
- return obj; | |
- } | |
+ case "undefined": | |
+ return null; | |
- // Array, NodeList, HTMLCollection, et al. | |
- else if (element.isCollection(obj)) { | |
- return [...obj].map(el => evaluate.toJSON(el, seenEls)); | |
- } | |
+ case "boolean": | |
+ case "number": | |
+ case "string": | |
+ return obj; | |
- // HTMLElement | |
- else if ("nodeType" in obj && obj.nodeType == obj.ELEMENT_NODE) { | |
- let uuid = seenEls.add(obj); | |
- return element.makeWebElement(uuid); | |
- } | |
+ case "object": | |
- // custom JSON representation | |
- else if (typeof obj["toJSON"] == "function") { | |
- let unsafeJSON = obj.toJSON(); | |
- return evaluate.toJSON(unsafeJSON, seenEls); | |
- } | |
+ // null object | |
+ if (obj === null) { | |
+ return null; | |
+ } | |
+ | |
+ // array | |
+ if (Array.isArray(obj)) { | |
+ return obj.map(e => toJSON(e, seenEls)); | |
+ } | |
+ | |
+ // HTMLElement | |
+ if (typeof obj.focus == "function" | |
+ && typeof obj.cloneNode == "function") { | |
+ let uid = seenEls.add(obj); | |
+ return element.makeWebElement(uid); | |
+ } | |
- // arbitrary objects + files | |
- else { | |
- let rv = {}; | |
- for (let prop in obj) { | |
- try { | |
- rv[prop] = evaluate.toJSON(obj[prop], seenEls); | |
- } catch (e if (e.result == Cr.NS_ERROR_NOT_IMPLEMENTED)) { | |
- logger.debug(`Skipping ${prop}: ${e.message}`); | |
+ // custom JSON representation | |
+ if (typeof obj.toJSON == "function") { | |
+ let unsafeJSON = obj.toJSON(); | |
+ return toJSON(unsafeJSON, seenEls); | |
+ } | |
+ | |
+ switch (Object.prototype.toString.call(obj)) { | |
+ | |
+ // literals | |
+ case "[object Boolean]": | |
+ case "[object Number]": | |
+ case "[object String]": | |
+ return obj; | |
+ | |
+ // collection | |
+ case "[object Arguments]": | |
+ case "[object Array]": | |
+ case "[object FileList]": | |
+ return Array.map(obj, e => toJSON(e, seenEls)); | |
+ | |
+ // collection of DOM HTML elements | |
+ case "[object HTMLAllCollection]": | |
+ case "[object HTMLCollection]": | |
+ case "[object HTMLFormControlsCollection]": | |
+ case "[object HTMLOptionsCollection]": { | |
+ let elRefs = seenEls.addAll(obj); | |
+ return elRefs.map(element.makeWebElement); | |
+ } | |
+ | |
+ // collection of DOM nodes | |
+ case "[object NodeList]": { | |
+ let els = Array.filter(obj, node => node.nodeType == 1); | |
+ let elRefs = seenEls.addAll(els); | |
+ return elRefs.map(element.makeWebElement); | |
+ } | |
+ | |
+ } | |
+ | |
+ // arbitrary objects + files | |
+ { | |
+ let rv = {}; | |
+ for (let prop in obj) { | |
+ try { | |
+ rv[prop] = toJSON(obj[prop], seenEls); | |
+ } catch (e if (e.result == Cr.NS_ERROR_NOT_IMPLEMENTED)) { | |
+ logger.debug(`Skipping ${prop}: ${e.message}`); | |
+ } | |
+ } | |
+ return rv; | |
} | |
- } | |
- return rv; | |
} | |
}; | |
--- testing/marionette/legacyaction.js 2017-06-07 12:34:43.000000000 +0100 | |
+++ testing/marionette/legacyaction.js 2017-06-07 16:10:16.640714400 +0100 | |
@@ -60,8 +60,7 @@ | |
this.seenEls = seenEls; | |
this.container = container; | |
- let commandArray = evaluate.fromJSON( | |
- args, seenEls, container.frame, container.shadowRoot); | |
+ let commandArray = evaluate.fromJSON(args, seenEls, container); | |
if (touchId == null) { | |
touchId = this.nextTouchId++; | |
--- testing/marionette/listener.js 2017-06-07 12:34:43.000000000 +0100 | |
+++ testing/marionette/listener.js 2017-06-07 16:10:34.365677300 +0100 | |
@@ -734,8 +734,7 @@ | |
opts.timeout = timeout; | |
let sb = sandbox.createMutable(curContainer.frame); | |
- let wargs = evaluate.fromJSON( | |
- args, seenEls, curContainer.frame, curContainer.shadowRoot); | |
+ let wargs = evaluate.fromJSON(args, seenEls, curContainer); | |
let res = yield evaluate.sandbox(sb, script, wargs, opts); | |
return evaluate.toJSON(res, seenEls); | |
@@ -750,8 +749,7 @@ | |
sb = sandbox.augment(sb, new logging.Adapter(contentLog)); | |
} | |
- let wargs = evaluate.fromJSON( | |
- args, seenEls, curContainer.frame, curContainer.shadowRoot); | |
+ let wargs = evaluate.fromJSON(args, seenEls, curContainer); | |
let evaluatePromise = evaluate.sandbox(sb, script, wargs, opts); | |
let res = yield evaluatePromise; | |
@@ -775,8 +773,7 @@ | |
// TODO(ato): Not sure this is needed: | |
sb = sandbox.augment(sb, new logging.Adapter(contentLog)); | |
- let wargs = evaluate.fromJSON( | |
- args, seenEls, curContainer.frame, curContainer.shadowRoot); | |
+ let wargs = evaluate.fromJSON(args, seenEls, curContainer); | |
let evaluatePromise = evaluate.sandbox(sb, script, wargs, opts); | |
let res = yield evaluatePromise; | |
@@ -1062,8 +1059,7 @@ | |
*/ | |
function multiAction(args, maxLen) { | |
// unwrap the original nested array | |
- let commandArray = evaluate.fromJSON( | |
- args, seenEls, curContainer.frame, curContainer.shadowRoot); | |
+ let commandArray = evaluate.fromJSON(args, seenEls, curContainer); | |
let concurrentEvent = []; | |
let temp; | |
for (let i = 0; i < maxLen; i++) { | |
@@ -1246,8 +1242,7 @@ | |
let el = yield element.find(curContainer, strategy, selector, opts); | |
let elRef = seenEls.add(el); | |
- let webEl = element.makeWebElement(elRef); | |
- return webEl; | |
+ return elRef; | |
} | |
/** | |
@@ -1266,8 +1261,7 @@ | |
let els = yield element.find(curContainer, strategy, selector, opts); | |
let elRefs = seenEls.addAll(els); | |
- let webEls = elRefs.map(element.makeWebElement); | |
- return webEls; | |
+ return elRefs; | |
} | |
/** Find and return the active element on the page. */ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment