1650800 - Use ContentDOMReference.jsm as replacement for our custom WebElement store

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, task, P3)

Description

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

There is the following module that can be used to create a mapping of element references in content and get these transferred as json to the parent process:

https://searchfox.org/mozilla-central/source/toolkit/modules/ContentDOMReference.jsm

As such we could replace our custom map for WebElement references, which is also leaky. Further benefit is that commands like getElementProperty could be fully implemented in the parent process, and won't need a command equivalent in the framescript/JSWindowActorChild.

The only downside is that it doesn't generate UUIDs as requested by the WebDriver spec, but does Math.random() to generate a unique id (see bug 1560166 for the uuid removal).

Comment 1

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

As James mentioned on Matrix we can make use of this module but would have to maintain our own map referencing those elements via the uuid.

Comment 2

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

As the prototype using the JSWindowActor has been shown we clearly want to make use of this Javascript module, and it even works in chrome scope, so not only in content as the name reflects.

https://phabricator.services.mozilla.com/D82731

The only problem is still the generated id which is not a uuid as the WebDriver spec defines.

Kris, what would be your suggestion in how to get it integrated but with uuid? You changed it from uuid to Math.random() in bug 1560166. So maybe you have a good suggestion for us? I'm not that happy in having to handle a separate map. Maybe we would have to simply clone it and make use of uuid instead?

Comment 3

[James Graham [:jgraham]]

What's the case against a seperate map? On the face of it having a class that wraps ContentDomReference but translates the id to/from uuids seems very much like the way forward. If we aren't super concerned about the uuids being cryptographically secure it's possible to write a function that converts the random number into a uuid or vice-versa, but I definitely go with the map option first.

Comment 4

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

(In reply to James Graham [:jgraham] from comment #3)

What's the case against a seperate map? On the face of it having a class that wraps ContentDomReference but translates the id to/from uuids seems very much like the way forward. If we aren't super concerned about the uuids being cryptographically secure it's possible to write a function that converts the random number into a uuid or vice-versa, but I definitely go with the map option first.

I will have a look at that. Probably the best is to have it as part of the already existent WebElement class. That way we just re-use that existent code and most of the internals will stay the same. I will play around with the JSWindowActor PoC first to get an idea how this would look like.

Comment 5

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

After some investigation of the Marionette code I can say that we don't have to use that module. At least not for now. Primary intent of its existence is that no CPOW references of elements need to be transferred between processes.

In our case that's not the case at all given that the only information we send between the parent and content processes is the id of the WebElement, which basically is a uuid. There is no CPOW usage at all.

Also similar to ContentDOMReference Marionette keeps a weak reference to the element in its own element store. Instances of this store exist inside the parent process for each individual chrome window, and in each of the framescript instances (listener.js).

We might need some updates when going to use the JSWindowActor class and Fission enabled, but lets that post-pone to when it is really necessary.

For now I'm going to close out this bug.