Add timeout for `browsingContext.locateNodes`

[jimevans]

In the WebDriver Classic spec, there is an "implicit wait" timeout for finding elements. When locating nodes in WebDriver BiDi, there is no mechanism to make a single call, and have it retry until success or a timeout. For better or for worse, users prefer the command behavior of "call once from the local end, don't return from the remote end until you get a result or reach a timeout." To enable this, we should update the command to continue searching for nodes until at least one matching node is found or the timeout is reached.

[jgraham]

I'm a bit skeptical about putting this in the protocol. We've generally eschewed in-protocol timeouts in BiDi, preferring to allow the client to implement their own retry strategies. I don't think there's anything we can realistically do on the browser side that can't be done on the client side here; in particular I don't think that there's any way to interrupt an in-progress locator; we fully depend on evaluating the locators to be fast. That means we can't implement a more accurate timer on the browser side than in the client.

Is there something in particular here that you can't simply implement as a retry loop in the client?

[jimevans]

I understand that we've mostly declined to do this in the protocol, and I understand the reasons why. As with most things like this "feature" (scare quotes intentional), this mostly comes down to ergonomics of the API and meeting users where they are. Adding a timeout for locating nodes removes the need for multiple (potentially many) round trips from the local end to the remote end.

I desperately wish that users didn't want this behavior, but they do, and they expect it. To give users what they want, every client will need to implement this as a polling wait. It seems more efficient to lift that retry to the remote end.

[sadym-chromium]

@jimevans I agree that waiting for the given locator to be non-empty is extremely frequent scenario and essential for robust testing. I see the advantage on having this logic on the user agent side in reducing amount of roundtrips. However implementation and specification-wise, we will have to implement it in the very same way - by polling the selector over and over again. Even if we decide to extend the browsingContext.locateNodes with timeout, we should also add "interval" parameter, so that the users are aware of possible race conditions.

FWIW, in Chromium-BiDi we implement these selectors by injecting client scripts on the page, which clients can implement as well:

• "css"
• "xpath"
• "innerText"

For these selectors we use CDP:

• "context" (iframe parent)
• "accessibility"

[OrKoN]

DOM mutation observer and RAF-based polling are two other alternatives to polling by time. A11y tree updates could be another source to re-poll for elements. I would suggest not calling the param timeout to not confuse with a timeout for the non-waiting call but instead have waitOptions to configure waiting behavior.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Timeout for browsingContext.locateNodes.

The full IRC log of that discussion

<AutomatedTester> topic: Timeout for browsingContext.locateNodes
<AutomatedTester> github: https://github.com//issues/1055
<AutomatedTester> jimevans: it has become clear to me as a client author is one thing users want is they want to call a command and not have it it return until thing they want is done
<AutomatedTester> ... for things like locating an element/node or waiting for an element to interactable before returning the command
<AutomatedTester> ... to that end that locating node on a page that we can create a way in the protocol to optionally wait for something to be true
<AutomatedTester> ... I can see why the protocol might not want to do it and they have to put it into the client which adds complexity to how things go back and forth
<AutomatedTester> ... and I have said earlier that fewer trips acorss the wire is better
<AutomatedTester> ... and if we move it to the protocol that would make things better for users
<jgraham> q+
<AutomatedTester> ... I am not concerned with how we should structure the data sent across the wire
<sadym> q+
<jdescottes> q+
<AutomatedTester> ... my concern is that, as a client author, is that I want to mkae 1 call across the protocol rather than many
<AutomatedTester> .... I understand that it complicates the remote end of the protocol
<AutomatedTester> ack next
<AutomatedTester> jgraham: I agree there is clearly a pattern in libraries that want to to wait for a page to reach a certain point
<jimevans> q+
<AutomatedTester> ... I believe that in puppeteer/playwright do this via polling which isnt problematic
<AutomatedTester> ... which makes it a low priority for now and we have avoided timeouts in the current spec
<jdescottes> q-
<AutomatedTester> ... I do take your point that if the only tool the client has about polling
<orkon> q+
<AutomatedTester> ... <dicsusses items in the github issues>
<AutomatedTester> ... as we things that can be done via javascript commands
<orkon> q-
<AutomatedTester> ... if we aren't having timeouts elsewhere in the code, is locatenodes a special case?
<AutomatedTester> ack next
<AutomatedTester> sadym: from the user perspective that 95% of failures are down to not waiting properly
<AutomatedTester> ... and there are times where locate nodes is the way to do it in a SPA. As for client or browser I lean towards client
<orkon> q+
<AutomatedTester> ... all the solutions have tradeoffs but I think it should live in the client side
<AutomatedTester> ack next
<AutomatedTester> jimevans: we have touched on some of my concerns. We do allow searching for nodes via hard ways that cant be done via JS. As a client library if you wanted to search shadow roots for a given child instead of the light dom, it becomes hard to deal with in JS if the shadow roots are closed
<AutomatedTester> ... however we allow piercing the shadow roots
<AutomatedTester> ... it is something that many users are confused on how to handle
<AutomatedTester> ... and they see it as a feature of non-selenium client about piercing closed shadow root. Playwright does element location is done via JS injection and can only do open shadow roots
<AutomatedTester> ... and I do believe locating nodes is unique since not all our strategies can be done with JS
<AutomatedTester> ... and in the interest of trying to reduce the traffic
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> orkon: We do polling via different mechanism. We also have mutation observers but they don't support shadow roots
<AutomatedTester> ... my opinion is that we should have it but I dont think that its a magical way to solve. We need to have a way to handle this properly
<jgraham> q+
<AutomatedTester> ack next
<jimevans> q+
<AutomatedTester> jgraham: if I am paging things in better, we dont have a way to handle shadow roots that are closed
<AutomatedTester> ... i think if we make locatenodes more powerful that we should have a timeout
<AutomatedTester> ... I can't rememober the discussions that got use to the tradeoffs that we currently have
<AutomatedTester> ... i think we will either do something basic which wasn't as perfomant as puppeteer
<AutomatedTester> ... so one of the questions is do we have someone that is happy to descrives the work around doing things like that
<jdescottes> q+
<AutomatedTester> ... I think the biggest value areas or locators that don't map to JS APIs
<AutomatedTester> ack next
<AutomatedTester> jimevans: I do remember of how we got to current implemention. The original proposal was more complicated and we simplified around not batching
<AutomatedTester> ... if we decided to apply this to things that can't easily replicated with cases that can't be handled by JS
<AutomatedTester> ... my worry is that I can't spec it but I know others in this group can
<AutomatedTester> ... I am hoping that others see the merit in what I am proposing and help spec it
<sadym> q+
<AutomatedTester> ... if the group wants to ignore it that's fine too
<AutomatedTester> ack next
<AutomatedTester> jdescottes: potentially a bad idea is that specing polling could be hard but if we had a generic polling should be simple to do
<jgraham> q+
<AutomatedTester> ack next
<jdescottes> s/specing polling/specing polling strategies/
<AutomatedTester> sadym: jimevans do you have an estimate of time to move to from polling to server, what is the percentage of time that time
<AutomatedTester> jimevans: I don't have metrics but want to point out that localhost isn't the only use case here. Remote end could be somewhere in the cloud
<AutomatedTester> ... that could mean a significant relative performance gains if in the cloud case
<AutomatedTester> sadym: my gut is saying the delay shouldn't be no more than latency of the connection
<AutomatedTester> jimevans: that's what I said in absolute perfomance I am not sure of the gains but in relative performance there would be gains by my gut feel
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> jgraham: regarding jdescottes about a generic retry command. There is an appeal to having this
<AutomatedTester> ... it's harder to handle the generic case. I think we need to look through the spec to see how we can benefit things here
<orkon> q+
<AutomatedTester> ... we can see a complicated client, like puppeteer could have their own retries while a simple client could rely on these retries
<AutomatedTester> ack next
<AutomatedTester> orkon: I wanted to mention that if a person has a latency issue
<AutomatedTester> ... if people are awaiting 100ms. If we don't do the complicated polling, then the simple polling can be done on the client
<AutomatedTester> ... it might solve chattiness of the protocol but not improve the performance
<sadym> * my gut says if the client polls and there is a need in retry, it means there is a delay of the node to appear. And if so, it should not be that different from if the client was very close to the server.
<jdescottes> q+
<jimevans> q+
<AutomatedTester> jdescottes: I will file an issue around creating a generic polling. Is this for performance or easier to use?
<AutomatedTester> ack next
<AutomatedTester> ack next
<AutomatedTester> jimevans: this is around using a different client to selenium and it made my client code harder to create as a generic new client that finds it hard to create
<jgraham> q+
<AutomatedTester> ack next
<AutomatedTester> jgraham: there is some appetite to look at this in the future and do a little more review for the future. Let's keep the issue open for now
<tidoust> scribe+
<tidoust> scribe+

[whimboo]

Another use case (one that doesn't seem to have been discussed yet) for waiting until a node is present arises in the context of a future command batching mechanism. In such a scenario, the client has no control over how many times the locateNodes command would need to be executed to poll for the node’s availability. If the node is not available at the time the batch is executed, the remaining commands in the chain would fail. As a result, locateNodes would effectively always need to be executed outside the batch to ensure stability.