Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC 90: [testdriver] Primitives for cross-context messaging #90
RFC 90: [testdriver] Primitives for cross-context messaging #90
Changes from 1 commit
95ec307
7ef74d4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the UUID come from?
You said we can't generate them at will, but there will be instead one per context predefined.
Do we have an UUID for every type of context? (document, sharedworker, dedicatedworker, serviceworker).
How do you convey the uuid from one context to the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess they come from the uuid params of the URL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #88
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "receive"
(×2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't those specify the uuid of the queue your are receiving messages?
It is highly important to be able to listen to different queues. You might want to control multiple context or run multiple subtest in parallel. Having multiple queues is very important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way it's set up at the moment, you can only listen for messages sent to your queue. But each message has a
src
property that tells you where it came from. So you don't supply a UUID here because it's implied that it's the one for the current context.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can this be used to run several subtest in parallel?
Having a single queue per context is a strong limitation. As is, I don't believe this solution could replace code from:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably you could have some code like:
And then a test would do something like
So I don't think it's impossible, but I agree there's some additional overhead from the fact that the message queues are per context. But it feels like the per-context model is closer to the model for other testdriver commands. Do you think that adding some additional built-in API surface for this kind of message dispatch would be enough to make the solution work for you, or do you have another suggested approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it has to belong to testdriver? I am not sure to understand the benefits. Maybe if I knew, they could potentially offset the drawback mentioned above?
I liked a lot hiroshige@ patches. Keep the message queue API very simple. Every context can pop/push to any arbitrary queue. Messages are simple string.
Then, once you have the capability to communicate, you can build second layers object like the Executor/ExecutorTarget, providing remote code execution, and returning result or errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot... Most COEP:credentials are built this way:
So there are multiple message queues to communicate in each pair (driver, helper) contexts.
With the current solution, it is limited to one queue per receiver context. As you said, we can simulate more by dispatching per sender context. So at most we have one queue per (receiver, sender) pair, but this is not enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't have to of course. But the reason for proposing this is to end up with a unified way of working with remote contexts, rather than having the concept of a remote context with a bunch of functionality like invoking user gestures, or running script, and a semi-decoupled model of messaging where the message queue ids are kind of coupled to context ids (because we give contexts an id through the URL), but anyone with the id has full read/write access. The model here seems closer to other things like
postMessage
where anyone (more or less) can write but only the target can read.I suppose another solution to the multi-test problem above would be introducing the ability to create non-context-bound message queues like the
MessagePort
DOM API. Then a test would create a context, and send a message to it containing the channel id to reply on. The test could await a message on that specific channel rather than doing dispatch on the src on the context-global channel.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am thinking, what about:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "message"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: I like specifying the target context/uuid before the messages.:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the other testdriver commands take the context as the final argument, which is why I went with that order, even though I agree that this is more natural.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK. That's unfortunate, but not a big deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promises? Or challenges?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It promises some challenges :p (but yes, good catch).