1411307 - Rewrite element retrieval functions to use generators

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

The various strategy-specific element retrieval function in
testing/marionette/element.js return arrays or sequences of elements,
but this entails extra computation work when the underlying data
representation is an iterator.

This means we exhaust the iterable, storing each item in an array,
return the sequence, then extract the first item and throw away
the remainder, when retrieving individual elements.

Instead, each function can be made to consistently return an iterable
which can be expanded when necessary.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1411307 - Return iterators for element retrieval.

These functions currently return sequences or arrays of elements, which
wastes cycles and memory because we spend time constructing these data
collections in cases where we only need the first element.  This is
the case for WebDriver:FindElement and WebDriver:FindElementFromElement.

This patch changes the Marionette element retrieval functions to
consistently provide iterators instead of sequences when looking
up elements.  Making them generator functions means the iterable
can be expanded using [...iterable] when necessary.

Review commit: https://reviewboard.mozilla.org/r/192542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192542/

Comment 2

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8921514 [details]
Bug 1411307 - Return iterators for element retrieval.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192542/diff/1-2/

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8921514 [details]
Bug 1411307 - Return iterators for element retrieval.

https://reviewboard.mozilla.org/r/192542/#review198286

r=wc

::: testing/marionette/element.js:396
(Diff revision 2)
>  
>  /**
>   * Find a single element by XPath expression.
>   *
> - * @param {DOMElement} root
> - *     Document root
> + * @param {HTMLDocument} rootNode
> + *     Document root.

I don't want that we conflict with [1] by using the term `rootNode`. Can we simply name the parameter just `doc`? It would also apply to the other changes.

[1] https://developer.mozilla.org/en-US/docs/Web/API/Node/rootNode

Comment 4

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8921514 [details]
Bug 1411307 - Return iterators for element retrieval.

https://reviewboard.mozilla.org/r/192542/#review198286

> I don't want that we conflict with [1] by using the term `rootNode`. Can we simply name the parameter just `doc`? It would also apply to the other changes.
> 
> [1] https://developer.mozilla.org/en-US/docs/Web/API/Node/rootNode

Fine, I’ve had to make a series of more changes because it was
referred to as a rootNode in the entry-point element.findElement and
element.findElements functions as well, but I’ve largely replaced
rootNode with document now.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8921514 [details]
Bug 1411307 - Return iterators for element retrieval.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192542/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b993bccfeebb
Return iterators for element retrieval. r=whimboo

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b993bccfeebb