Skip to content
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

Add a bootstrap scripts feature #65

Closed
jgraham opened this issue Oct 27, 2020 · 4 comments
Closed

Add a bootstrap scripts feature #65

jgraham opened this issue Oct 27, 2020 · 4 comments

Comments

@jgraham
Copy link
Member

jgraham commented Oct 27, 2020

Initial proposal at https://github.com/w3c/webdriver-bidi/blob/master/proposals/bootstrap-scripts.md

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed BiDi Bootstrap scripts.

The full IRC log of that discussion <AutomatedTester> Topic: BiDi Bootstrap scripts
<AutomatedTester> https://github.com//issues/63
<AutomatedTester> jgraham: before we start on that it might be worth noting that I raised an issue from yesterdays discussion issue 63
<jgraham> GitHub Topic: https://github.com//issues/65
<AutomatedTester> brwalder: Bootstrap scripts execute as the first thing that is executed in a realm as it is created. It allows them to introspect the realm or setup the page for the items needed for testing
<AutomatedTester> ... in a bidi world it would be a good have a mechanism for bootstrapping scripts and send it back to local end
<jgraham> q+
<simonstewart> q+
<AutomatedTester> ... there is a proposal that allows us to bootstrap a script assigned to a url pattern and when a pattern is matched it would inject the script on that realm
<AutomatedTester> ... and if we needed to inject it was on a realm ID it would create a race condition that we would like to avoid
<AutomatedTester> ... The script would allow you work on newly created realms
<AutomatedTester> ... [describing how a JS port and JS works in this context]
<AutomatedTester> ... the scenarios that this enables is listening for events on a page and send them from the bootstrap script to the local end
<AutomatedTester> ... and allows us to support items without having to specifically support items
<AutomatedTester> ... and we can have a scenario for handling JS errors and cause a test to end quicker
<AutomatedTester> q?
<AutomatedTester> ack jgraham
<AutomatedTester> jgraham: Overall this is a good idea, and it's a powerful idea. I have had cases where webdriver has not be able to do this.
<AutomatedTester> ... there are a number of open questions and these are partly script execution and this feature
<AutomatedTester> ... is this only for scripts associated with documents only
<AutomatedTester> ... e.g. worklets could be very difficult
<AutomatedTester> ... The other item to wonder about is this executes before the page has loaded. DO we need to have guarantees around the shape of the document/DOM
<AutomatedTester> ... and are scripts sandboxed?
<AutomatedTester> ... A precedence for here are extensions
<AutomatedTester> ... The final thing is around the value communication issue. The message ports is ok but that only works with certain values
<AutomatedTester> ... and we should maybe model it like a DOM API?
<AutomatedTester> brwalder: Should this work in a document or all realms? The proposal doesn't mention this just yet
<AutomatedTester> ... and this touches slightly on the discussion yesterday on how script execution works
<AutomatedTester> ... so yes we need to have more expressive pattern matching
<AutomatedTester> ... re: sandboxing there are cases where we want to be limited to the sandbox and sometimes we don't
<AutomatedTester> ... the proposal already handles this
<AutomatedTester> ... re: message port... if we are using message porting as specified won't work here. I think we need to have a way to do the serialisation better
<AutomatedTester> q?
<AutomatedTester> ack simonstewart
<AutomatedTester> simonstewart: I second the proposal on the serialization
<AutomatedTester> ... is the message port is just a 1-way comms?
<AutomatedTester> brwalder: I missed mentioning this earlier, there would be a specific command for this
<AutomatedTester> simonstewart: with worklets... they are supposed to be high performance... how will this impact this performance here?
<AutomatedTester> jgraham: my thought is that we start by making this on JS Realms that are part of a Window global and then go to realms as the use cases present themselves
<AutomatedTester> simonstewart: I can see people wanting workers and we can add as the need but worklets less likely
<AutomatedTester> ... though we can see about extensions in later versions of the specification
<AutomatedTester> jgraham: yes, we can easily do that.
<AutomatedTester> ... so the question to implementors, are there any concerns in this area?
<AutomatedTester> ... and hopefully it will be like extensions
<AutomatedTester> brwalder: this is certainly possibly... CDP has the bootstrapping but its for ALL realms
<AutomatedTester> ... and is possible
<AutomatedTester> brrian: and in webkit it is possible
<AutomatedTester> Resolution: brwalder to create a formal PR for this area.
<AutomatedTester> jgraham: we want to make sure that this works with general script execution
<AutomatedTester> ... the open issues on general script execution are :
<AutomatedTester> ... a) how will this work on sandboxing
<AutomatedTester> ... b) what the API for comms will look like? Message Ports? DOM API?

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Bootstrap scripts.

The full IRC log of that discussion <AutomatedTester> topic: Bootstrap scripts
<AutomatedTester> Github: https://github.com//issues/65
<AutomatedTester> jgraham (IRC): The concept here is we have a method that allows a script to be injected and run before a page is loaded
<AutomatedTester> ... the use case is we can define functionality that will be guaranteed when we access the page
<AutomatedTester> ... and we don't need to worry about racing with loading
<AutomatedTester> ... my reason for adding this to the agenda
<AutomatedTester> ... in berlin there was a talk about priorities
<DevinRousso> q+
<AutomatedTester> ... and seeing that it is in a number of clients
<AutomatedTester> ... so my questions are
<AutomatedTester> ... what is the priority of this work?
<AutomatedTester> ... and what is the MVP to make this feature useful
<AutomatedTester> ack next
<AutomatedTester> Devin Rousso: one of the reasons I suggested this in 2019 is that itenables us to do things without having to creating new events
<AutomatedTester> ... it gives us access to the events and apis that are there
<AutomatedTester> ... I think that this important but it's behind the networking interception
<sadym> q+
<AutomatedTester> ... it's important and we should do it
<AutomatedTester> q?
<AutomatedTester> ack next
<DevinRousso> AutomatedTester: i think it'd be more accurate to say they're both kinda equal in my mind?
<AutomatedTester> sadym (IRC): it's high priority as it's required in puppeteer
<AutomatedTester> ... it was raised in berlin that it was going to be used by selenium to do things with the DOM
<AutomatedTester> ... so it's a high priority
<AutomatedTester> q?
<sadym> is was mutationObserver
<DevinRousso> ^ yeah that's one use case I see for it
<DevinRousso> a big one
<AutomatedTester> jgraham (IRC): there is agreement that clients want it and it has a reasonable priority
<AutomatedTester> ... so the next thing is how do we get there?
<AutomatedTester> ... so the proposal is fairly complicated because it has a lot of filtering
<jgraham> https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-addScriptToEvaluateOnNewDocument
<AutomatedTester> ... but cDP just wants us to run things
<DevinRousso> +1 to having it be just "here's a script to run, i'll figure out what to do in JS myself"
<jgraham> https://github.com//issues/157
<AutomatedTester> ... one of the things here is that the JS might want to communicate back the client
<AutomatedTester> ... and we have punted on that so we might need to solve that first before coming back to this
<AutomatedTester> ... looking at this issue the main issue is how do you configure this since its before the page is loaded
<AutomatedTester> ... [describes how this runs in a function]
<jgraham> q?
<DevinRousso> q+
<jgraham> q+
<AutomatedTester> ack next
<sadym> q+ > it should still be a platform object
<sadym> q+
<AutomatedTester> Devin Rousso: I think the webdriver side to comms this looks fine
<AutomatedTester> ... there is a extension scopes in webkit that allows us to be able do some extensions when running
<AutomatedTester> ... and I am not against having this have be a function
<AutomatedTester> ... mesage port seems fine but we don't need everything on it
<AutomatedTester> ... and it would be good to describe the data that is sent back
<AutomatedTester> ... [describes a use case]
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> jgraham (IRC): treating this a stack, when serialsing we wont be doing anything special in this context
<DevinRousso> AutomatedTester: e.g. sending a DOM node back to the driver, which would do some sort of conversion to some format that the driver can handle (ideally the same primitives used elsewhere in WebDriver)
<AutomatedTester> ... so javascript objects are serialised back
<AutomatedTester> ... to discuss the previous design spaces
<AutomatedTester> ... in cDp there is an add binding method that allows it to be available globally
<DevinRousso> q+
<AutomatedTester> ... there are 2 other options
<AutomatedTester> ... we might want to message from a non-bootstrapped script to others
<AutomatedTester> ... the other option is we have special value the client sends to handle this like a message port but not quite a message port
<AutomatedTester> ... it puts the client in charge here as they can create the port and tells the browser
<AutomatedTester> ...an advantage of separate ports is that it becomes implicit in the protocol
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> sadym (IRC): it will be a little complicated to implement it in chromium
<AutomatedTester> ... it exposes things globally but not to all realms
<AutomatedTester> ... there is not concept of a binding argument
<AutomatedTester> q?
<AutomatedTester> jgraham (IRC): my understand of CDP is that you can do add binding because you don't know what the realm id will be for the boostrap script
<AutomatedTester> ... so you can sandbox the bootstrap script and that limits to where it canrun
<AutomatedTester> sadym (IRC): that is true
<AutomatedTester> ... when you bootstrap you can describe how the script will be called in the sandbox
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> Devin Rousso: in webkit I think we do it when the realm is created
<AutomatedTester> ... and I am not sure we can filter as there are things we don't want to expose everything to the client
<AutomatedTester> ... I think it would be better to basic and primative and let the driver do it but if we want to do something more complex that is fine too. It's managing state there...
<AutomatedTester> ... isolatedvs in the page... we can do that later
<AutomatedTester> ... in either scenario we can do it as scope extension
<jgraham> q+
<AutomatedTester> ... and we can give them the building blocks to do the right thing for them
<AutomatedTester> ... in the scenario where the driver provides the script to say run this everywhere...
<AutomatedTester> ... I can see there be an event when the bootstrap script is created
<AutomatedTester> ... and get people to listen
<AutomatedTester> ... i can see both ways working
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> jgraham (IRC):I am hearing we want this feature and we want the scripts to run in a sandbox... or not
<AutomatedTester> ... and there is a need for some form of messaging coming out of those scripts
<AutomatedTester> ... that seems reasonable to me
<AutomatedTester> ... I don't know how easy it will be to implement it
<AutomatedTester> ... in the sandbox case it will be the most useful
<AutomatedTester> Devin Rousso: I think the non-sandboxed version being the most useful
<AutomatedTester> ... I think we will need both
<AutomatedTester> q?
<sadym> could you please repeat the proposed dates?
<DevinRousso> i believe it was 11/28-12/02
<AutomatedTester> rrsagent: Make minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2022/09/16-webdriver-minutes.html AutomatedTester
<AutomatedTester> zakim: bye
<AutomatedTester> rrsagent: bye
<RRSAgent> I see no action items

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Bootstrap scripts.

The full IRC log of that discussion <jgraham> Topic: Bootstrap scripts
<jgraham> github: https://github.com//issues/65
<jgraham> q?
<jgraham> sadym: We want start implementing bindings to send custom events, because this is used by puppeteer. This is straightforward if it was an argument of callFunction. Bootstrap scripts could be a function that accepts an argument that would be called to emit custom events.
<jgraham> q+
<jgraham> ack jgraham
<AlexRudenko1> q+
<jgraham> jgraham: We discussed this at TPAC. We considered both the possibility of passing in a specal kind of remote value as a function argument and putting a WebDriver object with an emitEvent method in the scope of WebDriver scripts. The first option makes routing more flexible, but the general consensus at that meeting seemed to be that the second is simpler.
<jgraham> ack AlexRudenko
<sadym> q+
<jgraham> AlexRudenko: Do you intend to expose emitevent only for bootstrap scripts or for any kind of script evalutation? It seems useful in both cases.
<jgraham> jgraham: For any WebDriver-BiDi script including those run via callFunction and evaluate
<jgraham> ack sadym
<jgraham> sadym: A concern with this is that implicitly adding the script into every scope is some overhead for us. Do we want to add it always or just when the user wants the capability?
<jgraham> jgraham: From a user point of view this would just look like a normal WebIDL interface if it's on the scope, whereas a special kind of value you get as a function argument is more unusual.
<jgraham> sadym: I think the opposite; something that's just in scope for WebDriver seems confusing compared to something that's explicitly passed in.
<jgraham> q?
<sadym> q+
<jgraham> sadym: From the implementation perspective, if we provide this in the scope we'd have to wrap the script to provide that scope, which makes it harder to provide the stack trace.
<jgraham> jgraham: For our implementation it doesn't matter either way. I don't know what's easier to spec
<jgraham> foolip: For event handlers we had something like this, but it changed. We couldn't do this with WebIDL, it would have to be done in terms of ECMAScript.
<jgraham> foolip: I prefer the arguments version, since it means we don't have to pick a name.
<jgraham> jgraham: In the previous discussion, Apple were in favour of the implicit global, is that still the case?
<sadym> q+
<jgraham> patrickangle: The object that's just implictly in scope makes it easier to work with cases where we don't have an implied function wrapper e.g. script.evaluate. But if we don't care about that, just being able to use an argument does have advantages in that we don't have to pick a name, etc.
<jgraham> ack sadym
<jgraham> sadym: For now we have two ways to evaluate script. Bootstrap will be a third method. We don't expect to introduce new script execution mechanisms. So we might be able to limit this to just working with bootstap script and call function.
<jgraham> jgraham: So how would bootstrap scripts work in this model
<jgraham> sadym: It would take a functionDeclaration like in callFunction.
<jgraham> sadym: We could provide information about the page as arguments to bootstrap script
<jgraham> jgraham: I think you'd already have access to window.location &c. at this point so it's not clear what the extra information would be.
<AlexRudenko1> q+
<jgraham> jgraham: Any input from client authors?
<jgraham> ack AlexRudenko
<sadym> q+
<jgraham> AlexRudenko: In puppeteer bootstrap scripts are more like evaluate(). From a Puppeteer perspective there's not a big difference, as long as you can send the message back and choose when the binding is available e.g. should be able to use setTimeout and still use the callback. For puppeteer we call the same bindings from multiple parts of the code, but that might be quite design specific.
<jgraham> ack sadym
<jgraham> sadym: I'm not sure if Puppeteer is using CDP bindings?
<jgraham> AlexRudenko: Yes. We don't inject bindings into the bootstrap script, but the bootstrap script removes the bindings from the actual context. But BiDi could be an improvement here.
<jgraham> q?
<jgraham> jgraham: Are we assuming that there's a different channel per RemoteValue in this model or something global?
<jgraham> sadym: One per instance, not global
<AlexRudenko1> q+
<jgraham> simons: No strong opinion from Selenium side
<jgraham> JimEvans: Agreed,
<jgraham> ack AlexRudenko
<jgraham> AlexRudenko: Bootstrap scripts are usually upfront for a specific browsing context. How are we going to provide arguments that survive across multiple contexts?
<sadym> q+
<jgraham> AlexRudenko: If you install bindings in bootstrap script and then navigate what happens? In CDP the bootstrap script is run in the new page, and the binding also survives.
<jgraham> AlexRudenko: RemoteValue would be bound to a context?
<jgraham> ack sadym
<jgraham> sadym: The binding wouldn't stick to a realm. For each browsing context you call with the same bindings.
<jdescottes> q+
<jgraham> jgraham: I think the event channels would be owned by the session. Every evaluation would get the channelfrom the session, so you'd alays get the same one.
<jgraham> jgraham: this could also work with other argument types, although if you passed e.g. a node that was gc'd then you'd get an error trying to run the script.
<jgraham> ack jdescottes
<jgraham> jdescottes: What do you expect to do with the bindings that surive? Are you keeping state in it? How much do you expect to be persisted?
<jgraham> AlexRudenko: We don't expect anything to be persisted. I had a different model in mind, but that doesn't seem to be what's proposed. We map each binding name to a specific function on the client side and use that to handle state, nothing is shared in the context.
<jgraham> jgraham: Seems like we want to adopt the argument-based approach, which is a change from TPAC. The next step is to make a PR so that we can evaluate the technical details.

@sadym-chromium
Copy link
Contributor

I believe this issue can be closed as implemented: https://w3c.github.io/webdriver-bidi/#command-script-addPreloadScript

@jgraham jgraham closed this as completed May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants