1788861 - script.callFunction clones remote references after deserialization

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P1)

Description

[Julian Descottes [:jdescottes]]

script.callFunction uses Realm.executeInGlobalWithBindings which will force cloning "this" and "arguments" before evaluating the function declaration: https://searchfox.org/mozilla-central/rev/25ec642ed33ca83f25e88ec0f9f27e8ad8a29e24/remote/webdriver-bidi/Realm.jsm#223-240

  executeInGlobalWithBindings(
    functionDeclaration,
    functionArguments,
    thisParameter
  ) {
    const expression = `(${functionDeclaration}).apply(__bidi_this, __bidi_args)`;

    return this.#globalObjectReference.executeInGlobalWithBindings(
      expression,
      {
        __bidi_args: this.#cloneAsDebuggerObject(functionArguments),
        __bidi_this: this.#cloneAsDebuggerObject(thisParameter),
      },
      {
        url: this.#window.document.baseURI,
      }
    );
  }

This is a problem whenever you are trying to use remote references (ie objects coming from this Realm, identified by a handle) because you should be able to use the original objects, and not a cloned version.

The deserialization algorithm should be responsible for cloning objects "when necessary" into the realm, but should preserve original objects if they are remote references.

An example fix can be found at https://phabricator.services.mozilla.com/D156201

A small example that fails because of this is

    remote_value = await bidi_session.script.evaluate(
        expression="window.test = {}; window.test",
        await_promise=False,
        result_ownership="root",
        target=ContextTarget(top_context["context"]),
    )
    result = await bidi_session.script.call_function(
        function_declaration="obj => obj === window.test",
        await_promise=False,
        arguments=[remote_value],
        target=ContextTarget(top_context["context"]),
    )
    assert result == {"type": "boolean", "value": True}

This should pass but since the remote_value gets cloned when we use call_function, the obj === window.test check will return false.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788861 - [bidi] Deserialization of remote references should not create new objects

Depends on D155635

Comment 2

[Julian Descottes [:jdescottes]]

Unassigning until triage.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1] (away 02/13 - 02/19)]

We will have to get this fixed for script.callFunctionOn so that the original object is used and not a clone.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788861 - [wdspec] Apply black formatting to wdspec test call_function.py

Depends on D156201

Simple autoformatting to avoid unrelated changes in the next changeset

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788861 - [wdspec] Add call_function test for deserialization of remote references

Depends on D156530

Add tests for deserialization of both this and arguments parameters, with various data structures (object, array, map, set) plus an additional test case nesting all
together.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1788861 - [bidi] Deserialization of remote references should not create new objects

Alternative to D156201

Comment 7

[Release mgmt bot [:suhaib / :marco/ :calixte]]

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

For more information, please visit auto_nag documentation.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1] (away 02/13 - 02/19)]

This is a M5 bug and should not block the enabling of the script commands by default.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83a729f1fffd
[bidi] Deserialization of remote references should not create new objects r=Sasha,webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/149ee65a5a38
[wdspec] Add call_function test for deserialization of remote references r=webdriver-reviewers,Sasha,whimboo

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1] (away 02/13 - 02/19)]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #10)

Failed to create upstream wpt PR due to merge conflicts. This requires fixup
from a wpt sync admin.

This basically happened because bug 1779231 isn't on mozilla-central yet and it's open PR hasn't been merged. James could you please take a look here? Thanks.

Comment 12

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/83a729f1fffd
https://hg.mozilla.org/mozilla-central/rev/149ee65a5a38

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36033 for changes under testing/web-platform/tests

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham