1830839 - "WebDriver:ExecuteScript" and "WebDriver:ExecuteAsyncScript" fail to return properties as defined via "defineProperty" with custom sandbox defined

Status: NEW

(Remote Protocol :: Agent, defect, P3)

Description

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

Originally filed as: https://github.com/mozilla/geckodriver/issues/1701

The following code fails when run via geckodriver because it used a custom sandbox for the script evaluation. It works fine with Marionette when no sandbox or the system one are used. So probably an issue with XRays?

        code = """
          class Wrapper {
            constructor(generator) {
              Object.defineProperty(this, 'foo', {
                enumerable: true,
                get: generator
              });
            }
          }
          const f = new Wrapper(() => "Hello World")
          const done = arguments[0];
          return f;
          """
        self.marionette.execute_script(code, sandbox="system")

FYI I've also tested with our WebDriver BiDi implementation and it works just fine when using a sandbox. So maybe we should consider using the same technique for Marionette as well?

Comment 1

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

It would actually be nice as well if we could share realms (sandboxes) between Marionette and WebDriver BiDi. It would unify the code that we have for both protocols and make it easier to update its behavior in the future as well (see CSP support or wantXrays()).

In terms of WebDriver classic it doesn't allow to specify a sandbox name and as such our code in Marionette runs through lazy.sandbox.createMutable().

Comment 2

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

This most likely is dependent on killing the usage of XRays via bug 1491490.

Eventually we might want to re-use the script evaluation code that we have for WebDriver BiDi, which basically passes this scenario. But it's not a priority right now.

Comment 3

[Kris Maglione [:kmag]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #2)

This most likely is dependent on killing the usage of XRays via bug 1491490.

Removing the wantXrays flag shouldn't block anything. It will just remove the flag and have it default to false. Anywhere it's currently set to true should be updated before that bug lands. As soon as possible, really.

Comment 4

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

Thank you Kris! We will discuss the priority again in our next triage session.

Comment 5

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

We actually also use this flag in our WebDriver BiDi implementation. So it does not only affect Marionette. Here a query set lists all the instances where it is used:

https://searchfox.org/mozilla-central/source/remote/shared/Realm.sys.mjs#217

Unless this bug would block platform we agreed on keeping it as P3 for now and get to it eventually.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit BugBot documentation.