From ec7ae63ad0e8009c3452a0f18f9cc204be1b6400 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Sun, 14 Feb 2021 14:09:22 +0000 Subject: [PATCH] Bug 1669172 - [marionette] Remove references to Element Store. r=marionette-reviewers,jdescottes Differential Revision: https://phabricator.services.mozilla.com/D100945 --- testing/marionette/browser.js | 2 - testing/marionette/doc/internals/element.rst | 6 +- testing/marionette/element.js | 169 +----------------- testing/marionette/evaluate.js | 56 ++---- testing/marionette/test/unit/test_evaluate.js | 88 --------- 5 files changed, 22 insertions(+), 299 deletions(-) diff --git a/testing/marionette/browser.js b/testing/marionette/browser.js index 60b402e395d0..80723a16e36b 100644 --- a/testing/marionette/browser.js +++ b/testing/marionette/browser.js @@ -165,8 +165,6 @@ browser.Context = class { // Used to set curFrameId upon new session this.newSession = true; - this.seenEls = new element.Store(); - // 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 diff --git a/testing/marionette/doc/internals/element.rst b/testing/marionette/doc/internals/element.rst index 5fabfc2c34dd..622eb5639942 100644 --- a/testing/marionette/doc/internals/element.rst +++ b/testing/marionette/doc/internals/element.rst @@ -1,9 +1,9 @@ element module ============== -element.Store -------------- -.. js:autoclass:: element.Store +element.ReferenceStore +---------------------- +.. js:autoclass:: element.ReferenceStore :members: element.find diff --git a/testing/marionette/element.js b/testing/marionette/element.js index 8ce9e5f46379..7e466fcad9bb 100644 --- a/testing/marionette/element.js +++ b/testing/marionette/element.js @@ -68,9 +68,10 @@ const XUL_SELECTED_ELS = new Set([ * web element reference for every element representing the same element * is the same. * - * The {@link element.Store} provides a mapping between web element - * references and DOM elements for each browsing context. It also provides - * functionality for looking up and retrieving elements. + * The {@link element.ReferenceStore} provides a mapping between web element + * references and the ContentDOMReference of DOM elements for each browsing + * context. It also provides functionality for looking up and retrieving + * elements. * * @namespace */ @@ -87,167 +88,6 @@ element.Strategy = { XPath: "xpath", }; -/** - * Stores known/seen elements and their associated web element - * references. - * - * Elements are added by calling {@link #add()} or {@link addAll()}, - * and may be queried by their web element reference using {@link get()}. - * - * @class - * @memberof element - */ -element.Store = class { - constructor() { - this.els = {}; - } - - clear() { - this.els = {}; - } - - /** - * Make a collection of elements seen. - * - * The order of the returned web element references is guaranteed to - * match that of the collection passed in. - * - * @param {NodeList} els - * Sequence of elements to add to set of seen elements. - * - * @return {Array.} - * List of the web element references associated with each element - * from els. - */ - addAll(els) { - let add = this.add.bind(this); - return [...els].map(add); - } - - /** - * Make an element seen. - * - * @param {(Element|WindowProxy|XULElement)} el - * Element to add to set of seen elements. - * - * @return {WebElement} - * Web element reference associated with element. - * - * @throws {TypeError} - * If el is not an {@link Element} or a {@link XULElement}. - */ - add(el) { - const isDOMElement = element.isDOMElement(el); - const isDOMWindow = element.isDOMWindow(el); - const isXULElement = element.isXULElement(el); - const context = element.isInXULDocument(el) ? "chrome" : "content"; - - if (!(isDOMElement || isDOMWindow || isXULElement)) { - throw new TypeError( - "Expected an element or WindowProxy, " + pprint`got: ${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 WebElement.fromUUID(i, context); - } - - // cleanup reference to gc'd element - } else { - delete this.els[i]; - } - } - - let webEl = WebElement.from(el); - this.els[webEl.uuid] = Cu.getWeakReference(el); - return webEl; - } - - /** - * Determine if the provided web element reference has been seen - * before/is in the element store. - * - * Unlike when getting the element, a staleness check is not - * performed. - * - * @param {WebElement} webEl - * Element's associated web element reference. - * - * @return {boolean} - * True if element is in the store, false otherwise. - * - * @throws {TypeError} - * If webEl is not a {@link WebElement}. - */ - has(webEl) { - if (!(webEl instanceof WebElement)) { - throw new TypeError(pprint`Expected web element, got: ${webEl}`); - } - return Object.keys(this.els).includes(webEl.uuid); - } - - /** - * Retrieve a DOM {@link Element} or a {@link XULElement} by its - * unique {@link WebElement} reference. - * - * @param {WebElement} webEl - * Web element reference to find the associated {@link Element} - * of. - * @param {WindowProxy} win - * Current window global, which may differ from the associated - * window global of el. - * - * @returns {(Element|XULElement)} - * Element associated with reference. - * - * @throws {TypeError} - * If webEl is not a {@link WebElement}. - * @throws {NoSuchElementError} - * If the web element reference uuid has not been - * seen before. - * @throws {StaleElementReferenceError} - * If the element has gone stale, indicating it is no longer - * attached to the DOM, or its node document is no longer the - * active document. - */ - get(webEl, win) { - if (!(webEl instanceof WebElement)) { - throw new TypeError(pprint`Expected web element, got: ${webEl}`); - } - if (!this.has(webEl)) { - throw new error.NoSuchElementError( - "Web element reference not seen before: " + webEl.uuid - ); - } - - let el; - let ref = this.els[webEl.uuid]; - try { - el = ref.get(); - } catch (e) { - delete this.els[webEl.uuid]; - } - - if (element.isStale(el, win)) { - throw new error.StaleElementReferenceError( - pprint`The element reference of ${el || webEl.uuid} is stale; ` + - "either the element is no longer attached to the DOM, " + - "it is not in the current frame context, " + - "or the document has been refreshed" - ); - } - - return el; - } -}; - /** * Stores known/seen web element references and their associated * ContentDOMReference ElementIdentifiers. @@ -260,7 +100,6 @@ element.Store = class { * webElRef: {element-6066-11e4-a52e-4f735466cecf: } } * * For use in parent process in conjunction with ContentDOMReference in content. - * Implements all `element.Store` methods for duck typing. * * @class * @memberof element diff --git a/testing/marionette/evaluate.js b/testing/marionette/evaluate.js index 5dd22d401091..9e18fef73f33 100644 --- a/testing/marionette/evaluate.js +++ b/testing/marionette/evaluate.js @@ -177,18 +177,17 @@ evaluate.sandbox = function( }; /** - * Convert any web elements in arbitrary objects to DOM elements by - * looking them up in the seen element store. For ElementIdentifiers a new - * entry in the seen element reference store gets added when running in the + * Convert any web elements in arbitrary objects to a ContentDOMReference by + * looking them up in the seen element reference store. For ElementIdentifiers a + * new entry in the seen element reference store gets added when running in the * parent process, otherwise ContentDOMReference is used to retrieve the DOM * node. * * @param {Object} obj * Arbitrary object containing web elements or ElementIdentifiers. - * @param {(element.Store|element.ReferenceStore)=} seenEls + * @param {element.ReferenceStore=} seenEls * Known element store to look up web elements from. If `seenEls` is an - * instance of `element.ReferenceStore`, return WebElement. If `seenEls` - * is an instance of `element.Store`, return Element. If `seenEls` is + * instance of `element.ReferenceStore`, return WebElement. If `seenEls` is * `undefined` the Element from the ContentDOMReference cache is returned * when executed in the child process, in the parent process the WebElement * is passed-through. @@ -200,12 +199,12 @@ evaluate.sandbox = function( * replaced by DOM elements. * * @throws {NoSuchElementError} - * If `seenEls` is an `element.Store` and the web element reference has not - * been seen before. + * If `seenEls` is an `element.ReferenceStore` and the web element reference + * has not been seen before. * @throws {StaleElementReferenceError} - * If `seenEls` is an `element.ReferenceStore` or `element.Store` and the - * element has gone stale, indicating it is no longer attached to the DOM, - * or its node document is no longer the active document. + * If `seenEls` is an `element.ReferenceStore` and the element has gone + * stale, indicating it is no longer attached to the DOM, or its node + * document is no longer the active document. */ evaluate.fromJSON = function(obj, seenEls = undefined, win = undefined) { switch (typeof obj) { @@ -233,18 +232,6 @@ evaluate.fromJSON = function(obj, seenEls = undefined, win = undefined) { return element.resolveElement(obj, win); } throw new TypeError("seenEls is not an instance of ReferenceStore"); - - // WebElement and Store (used by framescript) - } else if (WebElement.isReference(obj)) { - const webEl = WebElement.fromJSON(obj); - if (seenEls instanceof element.Store) { - // Child: Get web element from the store - return seenEls.get(webEl, win); - } else if (!seenEls) { - // Parent: No conversion. Just return the web element - return webEl; - } - throw new TypeError("seenEls is not an instance of Store"); } // arbitrary objects @@ -267,8 +254,8 @@ evaluate.fromJSON = function(obj, seenEls = undefined, win = undefined) { * - Collections, such as `Array<`, `NodeList`, `HTMLCollection` * et al. are expanded to arrays and then recursed. * - * - Elements that are not known web elements are added to the `seenEls` element - * store, or the ContentDOMReference registry. Once known, the elements' + * - Elements that are not known web elements are added to the + * ContentDOMReference registry. Once known, the elements' * associated web element representation is returned. * * - WebElements are transformed to the corresponding ElementIdentifier @@ -284,7 +271,7 @@ evaluate.fromJSON = function(obj, seenEls = undefined, win = undefined) { * @param {Object} obj * Object to be marshaled. * - * @param {(element.Store|element.ReferenceStore)=} seenEls + * @param {element.ReferenceStore=} seenEls * Element store to use for lookup of web element references. * * @return {Object} @@ -317,31 +304,18 @@ evaluate.toJSON = function(obj, seenEls) { // WebElement } else if (WebElement.isReference(obj)) { // Parent: Convert to ElementIdentifier for use in child actor - if (seenEls instanceof element.ReferenceStore) { - return seenEls.get(WebElement.fromJSON(obj)); - } - - return obj; + return seenEls.get(WebElement.fromJSON(obj)); // ElementIdentifier } else if (WebElement.isReference(obj.webElRef)) { // Parent: Pass-through ElementIdentifiers to the child - if (seenEls instanceof element.ReferenceStore) { - return obj; - } - - // Parent: Otherwise return the web element - return WebElement.fromJSON(obj.webElRef); + return obj; // Element (HTMLElement, SVGElement, XULElement, et al.) } else if (element.isElement(obj)) { // Parent if (seenEls instanceof element.ReferenceStore) { throw new TypeError(`ReferenceStore can't be used with Element`); - - // Child: Add element to the Store, return as WebElement - } else if (seenEls instanceof element.Store) { - return seenEls.add(obj); } // If no storage has been specified assume we are in a child process. diff --git a/testing/marionette/test/unit/test_evaluate.js b/testing/marionette/test/unit/test_evaluate.js index 6b426e13fa96..2f72a5ecefa2 100644 --- a/testing/marionette/test/unit/test_evaluate.js +++ b/testing/marionette/test/unit/test_evaluate.js @@ -65,7 +65,6 @@ const domElId = { id: 1, browsingContextId: 4, webElRef: domWebEl.toJSON() }; const svgElId = { id: 2, browsingContextId: 5, webElRef: svgWebEl.toJSON() }; const xulElId = { id: 3, browsingContextId: 6, webElRef: xulWebEl.toJSON() }; -const seenEls = new element.Store(); const elementIdCache = new element.ReferenceStore(); add_test(function test_toJSON_types() { @@ -81,11 +80,6 @@ add_test(function test_toJSON_types() { // collections deepEqual([], evaluate.toJSON([])); - // elements - ok(evaluate.toJSON(domEl, seenEls) instanceof WebElement); - ok(evaluate.toJSON(svgEl, seenEls) instanceof WebElement); - ok(evaluate.toJSON(xulEl, seenEls) instanceof WebElement); - // toJSON equal( "foo", @@ -125,31 +119,6 @@ add_test(function test_toJSON_types_ReferenceStore() { }); add_test(function test_toJSON_sequences() { - const input = [ - null, - true, - [], - domEl, - { - toJSON() { - return "foo"; - }, - }, - { bar: "baz" }, - ]; - const actual = evaluate.toJSON(input, seenEls); - - equal(null, actual[0]); - equal(true, actual[1]); - deepEqual([], actual[2]); - ok(actual[3] instanceof WebElement); - equal("foo", actual[4]); - deepEqual({ bar: "baz" }, actual[5]); - - run_next_test(); -}); - -add_test(function test_toJSON_sequences_ReferenceStore() { const input = [ null, true, @@ -186,33 +155,6 @@ add_test(function test_toJSON_sequences_ReferenceStore() { }); add_test(function test_toJSON_objects() { - const input = { - null: null, - boolean: true, - array: [], - webElement: domEl, - elementId: domElId, - toJSON: { - toJSON() { - return "foo"; - }, - }, - object: { bar: "baz" }, - }; - const actual = evaluate.toJSON(input, seenEls); - - equal(null, actual.null); - equal(true, actual.boolean); - deepEqual([], actual.array); - ok(actual.webElement instanceof WebElement); - ok(actual.elementId instanceof WebElement); - equal("foo", actual.toJSON); - deepEqual({ bar: "baz" }, actual.object); - - run_next_test(); -}); - -add_test(function test_toJSON_objects_ReferenceStore() { const input = { null: null, boolean: true, @@ -266,41 +208,11 @@ add_test(function test_fromJSON_ReferenceStore() { deepEqual(webEl, domWebEl); deepEqual(elementIdCache.get(webEl), domElId); - // Store doesn't contain ElementIdentifiers - Assert.throws( - () => evaluate.fromJSON(domElId, seenEls), - /TypeError/, - "Expected element.ReferenceStore" - ); - elementIdCache.clear(); run_next_test(); }); -add_test(function test_fromJSON_Store() { - // Pass-through WebElements without adding it to the element store - let webEl = evaluate.fromJSON(domWebEl.toJSON()); - deepEqual(webEl, domWebEl); - ok(!seenEls.has(domWebEl)); - - // Find element in the element store - webEl = seenEls.add(domEl); - const el = evaluate.fromJSON(webEl.toJSON(), seenEls); - deepEqual(el, domEl); - - // Reference store doesn't contain web elements - Assert.throws( - () => evaluate.fromJSON(domWebEl.toJSON(), elementIdCache), - /TypeError/, - "Expected element.Store" - ); - - seenEls.clear(); - - run_next_test(); -}); - add_test(function test_isCyclic_noncyclic() { for (let type of [true, 42, "foo", [], {}, null, undefined]) { ok(!evaluate.isCyclic(type));