web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

RFC 90: [testdriver] Primitives for cross-context messaging #90

Closed jgraham closed 2 years ago

jgraham commented 2 years ago

Add send()/poll()/recv() functions that can be used to pass arbitary messages to other testdriver contexts, to help with test scenarios requiring multiple origins

jgraham commented 2 years ago

So, to be clear I'm not wedded to the proposed setup. But I'm also skeptical of making the API surface a multi-producer, multi-consumer queue in which we expose a way for anyone with the token to both read and write. The problem with that design is that I fear it will lead to hard-to-debug flakiness where for timing reasons one context reads something that was supposed to be read by a different context, and the whole implicit protocol breaks down. Constraining who can read from a queue, even if multiple people can write to it, seems to encourage more robust designs.

That said, I agree that the proposal doesn't address the use case of multiple tests running concurrently, and all wanting to communicate back to the test context without needing complex dispatch. One way to extend the proposal to support that might be to allow something like [receiver, sender] = test_driver.channel(); remote_context.send(sender); await reciever.recv() i.e. you'd get seperate objects implementing the send() and poll()/recv() parts of the protocol, and it would be possible to send() the sender (but not the receiver) to a remote context so you'd get a unique channel per test to communicate back results. For example, in the test window:

let uuid = "{{uuid}}"';
window.open(`child.html?uuid=${uuid}`, "_blank", "noopener");
let remote = new test_driver.RemoteContext(uuid); // This only implements send()
let [receiver, sender] = new test_driver.Channel()
await remote.send(sender);
// Listen for a message from the remote end
await reciever.recv()

and in child.html:

let test_channel = test_driver.recv();
try {
  let result = await do_test();
  test_channel.send({status: OK, result: result});
} catch(e) {
  test_channel.send({status: "ERROR", message: e.toString()});
}

So test_driver would automatically implement the receiver API for the current context (and we could also make that available in workers / remote contexts), and given a uuid, it would be possible to construct a sender, but not a receiver.

One thing that might be useful here is to look at use cases. For example:

And maybe some higher level ones:

Does that sound like a reasonable start? What am I missing?

ArthurSonzogni commented 2 years ago

Does that sound like a reasonable start? What am I missing?

I think we both understand each other.

The additional test_driver.Channel() can help, but this adds more work for you and for developers to use. Allowing a uuid params (even optional) would be simpler IMO. I think you should implement all of your proposals and try them it as a replacement for the two send() receive() functions and executor.html file from: https://github.com/web-platform-tests/wpt/blob/master/html/cross-origin-embedder-policy/credentialless/resources/dispatcher.py This might help you figure out what is best, and helps me to realize this is a good fit.

Alternatively, you can also make COOP/COEP tests not to use this proposal, that's also a possibility. This proposal is good on its own, even if we don't manage to use it for the existing tests.

one context reads something that was supposed to be read by a different context.

I would argue that with multiple queues, this precisely avoid having multiple readers reading the same queue.

jgraham commented 2 years ago

OK, so I'm approximately ready to replace this PR with one that proposes a channel-based model.

I've updated the branch at https://github.com/web-platform-tests/wpt/pull/29803 with a WebSockets-backed channel-based proposal and converted a bfcache test and a COOP test to the new model.

The COOP test in particular is very similar in the new model, but I think there are a few quality of life improvements:

The bfcache test required some additional work, mostly because haing ongoing requests / open sockets prevents pages being added to the bfcache. This required a mechanism to stop new messages being read on the channels before navigation (so that the client websocket doesn't buffer a message, only for it to be lost after navigation). Obviously this is easier with a polling approach since the user code is in control of when we poll for new messages. If it turns out that this finegrained control is necessary, we could use the same server-side message storage approach but add a client API in which the client has to explictly poll() for a new message, causing a HTTP request which reads a message off the queue, if any is present. That could reuse most of the same frontend code.

If people think this is a good option to pursue I'll write up a full RFC for the new proposal, but some more technical details to help evaluate what's going on:

Anyway the general pattern is that in the main test file you have something like

<title>Demo of executeScript</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/channel.sub.js"></script>
<script>
promise_test(async t => {
    let page = new TestDriverRemoteContext();
    window.open(`child.html?uuid=${page.uuid}`, "_blank", "noopener");
    assert_equals(await page.executeScript(() => document.body.textContent), "PASS");
})
</script>

And in child.html you have something like

<!-- This is currently needed for reading the uuid, but could maybe be removed -->
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/channel.sub.js"></script>
<p>PASS</p>
<script>
// Listen for messages
ctx_channel();
</script>
ArthurSonzogni commented 2 years ago

I took a quick look. I like the API a lot and how it looks on the existing COOP test above. Thanks for working on this! This will help writing many tests about navigations / iframes / popups.

I would be curious to see how SendChannel would look like if implemented.

jgraham commented 2 years ago

I'm withdrawing this in favour of https://github.com/web-platform-tests/rfcs/pull/98 which has an alternate approach to the same problem, not based on polling.