SharedReference serialization + deserialization

[sadym-chromium]

• Defined SharedReference;
• Described RemoteReference deserialization based on handle first and sharedId afterwards.
• Define Node serialization.

(edit @whimboo) No longer blocked by w3c/webdriver#1707

---

Preview | Diff

[sadym-chromium]

WDYT about the deserialization approach?

[jgraham]

A couple of quick high-level comments; I still need to do a full review:

• I think we should try to make SandboxProxy transparent. It's basically the representation of a wrapper object, but in implementations you can't check if something "is" a wrapper object or not; it's just that all objects are always accessed through a wrapper. So I think we should say things like "If the object wrapped by a SandboxProxy implements interface X then the SandboxProxy also implements X".
• I don't love crossSandboxId as a name. I guess the idea was to avoid tying it to DOM objects in particular in case we later need it for Shared Array Buffer or similar. If we want to avoid domId I think I'd even prefer something like sharedId or uniqueId.

[sadym-chromium]

@jgraham

• I think we should try to make SandboxProxy transparent. It's basically the representation of a wrapper object, but in implementations you can't check if something "is" a wrapper object or not; it's just that all objects are always accessed through a wrapper. So I think we should say things like "If the object wrapped by a SandboxProxy implements interface X then the SandboxProxy also implements X".

I'm not sure if I understand this. Do you mean we should not reference the wrapped object?

• I don't love crossSandboxId as a name. I guess the idea was to avoid tying it to DOM objects in particular in case we later need it for Shared Array Buffer or similar. If we want to avoid domId I think I'd even prefer something like sharedId or uniqueId.

I changed name to shared: sharedId, sharedReference etc. We can use domId, but not sure if it makes sense.

• domId: expresses scope and usage scenarios, which can make developers' lifes easier.
• sharedId: keeps flexibility in case of any other sharing scenario came up.

WDYT?

[jgraham]

I'm not sure if I understand this. Do you mean we should not reference the wrapped object?

I mean if we use the model that any DOM object is really a JSWrapper wrapping a platform IDLObject (that's implemented in C++ or whatever), then given e.g. a JS object that's a JSWrapper wrapping a Node IDLObject we're happy to say that the object itself "implements Node", even though at the lowest levels the implementation does have to make the distinction between the wrapper and the underlying DOM object.

A SandboxProxy object is just intended to be a spec representation of the JSWrapper, but scoped just to sandboxes to avoid rewriting the whole world. So I think we want to avoid asking "is this object a SandboxProxy" whenever possible; we largely want to handle the SandboxProxy objects as if they are indistinguishable from the underlying object (or, more precisely, indistinguishable from a non-sandbox JSWrapper, because AIUI in e.g. Blink's implementation they actually are the same type, and in Gecko's implementation they're actually all different views of the same object).

I'm not sure if that clarifies things or not?

[sadym-chromium]

To clarify: To ensure a SandboxProxy is a singleton for the given Platform object, we need to be able to map the given Platform object to something. In current approach, I use the term "object wrapped by the SandboxProxy".

UPD 2022-12-20: outdated

[jgraham]

So this means if you send {objectId: "foo", sharedId: "bar"} we won't reject the request, but we will prefer the objectId and use sharedId if the objectId doesn't exist. I think we should make that an InvalidArgument error.

[sadym-chromium]

I wanted to avoid having InvalidArgument in case of sanding back the object received from the backend, as we do with remoteReference.

[whimboo]

Hi @sadym-chromium. As it looks like this pull request hasn't been updated for quite a while and it might be great to actually have the proposed changes landed. Would you be still interested to continue? Thanks!

[sadym-chromium]

Hi Henrik! Thanks for reminding, I'm going to continue working on it soon.

[whimboo]

Hi Henrik! Thanks for reminding, I'm going to continue working on it soon.

@sadym-chromium, just a friendly reminder... Did you had a chance to continue on this PR? Another three months passed by and it would be great to have a shared id for elements. Thanks.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Update on `SharedReference` .

The full IRC log of that discussion

<jgraham> Topic: Update on `SharedReference`
<jgraham> github: https://github.com//pull/180
<jgraham> sadym: I expect to start working on this again in the next week or two.

[jgraham]

I've rebased this and fixed some problems. I think it's good to go now, but it would be useful for e.g. @whimboo and @foolip to check for errors and omissions.

[whimboo]

This is not visible to viewers of the spec. Shall we make that a note instead?

[jgraham]

Well the intent was for spec authors to know not to change things. The problem we have here is that if you actually use CDDL directly you can't tell the difference between these constructs. But also CDDL doesn't tell you which production actually matched. So I think the conclusion would be that we should have a general note that if you're trying to derive types from the CDDL productions then the order matters. But also we should document the other conventions we're using.

[whimboo]

I may have missed a discussion (due the lengthy thread) but is that actually necessary to still check for the correct type if we are going to ignore the handle anyway?

[jgraham]

It means that you can't write:

{
   sharedId: "foo",
   handle: ["some other type"]
}

which otherwise would match due to Extensible. Given that the proposed use case is "we want to allow people to directly send JSON data from responses" it makes sense to me to ensure that we can't extend this in a way that doesn't make sense (but honestly I still think we should care less about this use case and prefer nomnative typing over structural typing. However I don't seem to have much support for that position).

[whimboo]

I think that this looks fine to me now. For the remaining open questions I would like to see what others think as well. @foolip might you be able to have a look at this PR while @sadym-chromium is away?

[sadym-chromium]

@jgraham thanks for rebasing and fixing issues.
@whimboo thanks for reviewing.

I reviewed the new changes, and it LGTM, but I cannot self-approve the PR. @jgraham feel free to merge.