1836514 - Unwaive Xrays during Node serialization

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Alexandra Borovova [:Sasha]]

At the moment, we unwaive Xrays only when we try to access shadow root of the node: https://searchfox.org/mozilla-central/source/remote/webdriver-bidi/RemoteValue.sys.mjs#476. But in case we try to serialize shadow root directly like in this wdspec test: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/webdriver/tests/bidi/script/evaluate/result_node.py#448 it's not happening, and it causes the check for shadow root wrongly identify this node as not shadow root and serialize children even when includeShadowTree is none. So we should do unwaiving earlier.
The test is fixed in Github repo: https://github.com/web-platform-tests/wpt/pull/40361. So we should probably wait until it's synced to m-c.

Comment 1

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

I wonder if we could unwaive XRays immediately at the beginning of serializeNode() or if that would have some side-effects. If we can do that we won't need to do it anymore in getSharedIdForNode().

Comment 2

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

Attachment: Bug 1836514 - [webdriver-bidi] Unwaive Xrays once during Node serialization.

Comment 3

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

Attachment: Bug 1836514 - [wdspec] Improve ShadowRoot tests for children and mode properties.

Depends on D181906

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05b7fbb7bac3
[webdriver-bidi] Unwaive Xrays once during Node serialization. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/55b5600b5957
[wdspec] Improve ShadowRoot tests for children and mode properties. r=webdriver-reviewers,jdescottes

Comment 5

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

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

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/05b7fbb7bac3
https://hg.mozilla.org/mozilla-central/rev/55b5600b5957

Comment 7

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

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

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

Hi James, can you please help and check what caused this failure in creating the upstream PR? Thanks!

Comment 8

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

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

Comment 9

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

Upstream PR merged by whimboo