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

Try rewriting the explainer #98

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

gsnedders
Copy link
Member

@gsnedders gsnedders commented Apr 11, 2021

This attempts to follow the TAG-given advice at https://w3ctag.github.io/explainers

It also tries to bring the explainer in-line with today's spec.

Rendered

This attempts to follow the TAG-given advice at
https://w3ctag.github.io/explainers

It also tries to bring the explainer in-line with today's spec.
@gsnedders gsnedders requested review from jgraham and foolip April 11, 2021 18:03

### Considered alternatives

\[FIXME: Adopting CDP wholesale\]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention: reluctant to standardise, exposes implementation details, chatty protocol only designed for local usage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, some of the FIXME was definitely "it's Sunday and I'm meant to be tidying my flat, not trying to improve the explainer due to WebDriver BiDi getting mentioned on orange site" (there are currently no interesting comments).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is where we mention WebTransport and CBOR, two newer things we're not going to use right now.


### Choice of Transport Layer

\[FIXME\]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno what you want here, but JSON-over-WebSockets is close the the prior art and the most standard thing for a bidi communication protocol on the web stack.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignoring the prior art for a second, it's not immediately obvious why JSON-over-WebSockets is a better solution than CBOR-over-TCP, especially when it comes to non-local usage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "this is not where we want to spend our complexity budget; tooling in this space uniformly has access to JSON parsing, and WebSockets is the common denominator in existing implementations", but also "this is something we could change later if we end up in a place where the transport is having a noticable effect on performance" (but I'd be surprised if we end up in that place; if the deserialization cost is that high the protocol got too chatty anyway).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unusually for a W3C spec, we expect implementations to be provided in many different client languages. WebSockets and JSON give us a widely-supported base for local end implementations to be built from, and opens the door to quick shell scripts to do useful things.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could just note that existing debugging protocols use WebSockets, so we will too.


The protocol is designed with the following goals in mind:
WebDriver BiDi should help ensure a better end-user experience across all browsers by allowing
developers to use the same tooling across all browsers.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an additional motivation, which is to decouple automation tools from an ever-changing debugging protocol. Primarily, this reduces the maintenance burden of tool vendors whilst allowing them to (as you point out) target multiple different browsers at the same time. Because the CDP protocol changes regularly, and webdriver bidi would be stable, this would also allow cross-version testing, which is also important.

Secondarily, this allows the Chromium team to make any changes they need to the protocol without fear of breaking those tools.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shs96c agreed that allowing the debugging protocols to evolve to keep up with devtools product needs is an important reason why "just standardize CDP" didn't fly. Do you have a wording suggestion for this?

Note that since BiDi will be implemented on top of CDP, changes to CDP will actually very much come with the fear of breaking tools using BiDi, and only rigorous internal testing will mitigate that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As such we should really do better in creating detailed WebDriver-bidi tests that will cover nearly all (important) cases for individual commands and events. For WebDriver HTTP I still see a lot of untested areas, especially around navigation. :(

@@ -31,19 +43,50 @@ The protocol is designed with the following goals in mind:
- Simple for browser vendors to implement and maintain.
- Possible for clients to enhance their WebDriver automation with browser-specific devtools protocol features.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although not discussed at the time, being able to support something like lighthouse would be a useful thing for this spec to be able to do.

\[FIXME. Why JSON?\]

CDDL is used to describe the protocol layer because it provides formal semantics, accomplishing the
"machine-readable API specification" goal while being similar to JSON-RPC used in the prior art.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original WebDriver spec also uses JSON, so we're following in an already established precedent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could do with some tweaking I think. CDDL describes the shapes of messages that can be encoded as JSON, and we're using CDDL to describe something very similar to JSON-RPC, but CDDL itself is not similar to JSON-RPC, they're on different layers.

Perhaps "while allowing us to define a protocol similar to JSON-RPC used in the prior art"?


### Choice of Transport Layer

\[FIXME\]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unusually for a W3C spec, we expect implementations to be provided in many different client languages. WebSockets and JSON give us a widely-supported base for local end implementations to be built from, and opens the door to quick shell scripts to do useful things.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement, there are some FIXMEs, but "try" is too humble a description. With just a few small fixes I'd be happy for us to land this.


## Overview
WebDriver BiDi (Bi-Directional) is being developed to allow web developers to migrate from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! 👍


## Goals
Many developer tools are exclusively targetting Chrome, relying on CDP, resulting in websites being
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Among test tooling not using WebDriver, I think Cypress is the most popular, and it's Chrome+Firefox. Puppeteer isn't hugely popular for testing (mostly automation) but it too is Chrome+Firefox.

I'd describe the problem as "Many developer tools are targeting mainly Chrome, and sometimes Firefox, relying on different and non-standard protocols for each supported browser. This results in websites being better tested in those browsers, leading to site compatibility bits for other browsers."

WDYT?

There are also lessons in Web Testing Report (should we link that?) about support for a browser in a test framework not leading directly to that browser being tested. It also needs to be really simple (multi-browser by default I think) and above all the developer needs to want to test the browser to begin with, probably already doing some manual testing in it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whimboo you might be interested in this wording as well, given your comment at #98 (comment)


The protocol is designed with the following goals in mind:
WebDriver BiDi should help ensure a better end-user experience across all browsers by allowing
developers to use the same tooling across all browsers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shs96c agreed that allowing the debugging protocols to evolve to keep up with devtools product needs is an important reason why "just standardize CDP" didn't fly. Do you have a wording suggestion for this?

Note that since BiDi will be implemented on top of CDP, changes to CDP will actually very much come with the fear of breaking tools using BiDi, and only rigorous internal testing will mitigate that.

## Non-goals

Feature parity with CDP is a non-goal at this time; many features of CDP are rarely used by
existing developer tooling, and being able to entirely supplant CDP is not a goal at this time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed!


## Prior Art

\[FIXME: [CDP](https://chromedevtools.github.io/devtools-protocol/),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just turn this into a list, or is there additional fixing needed?


### Choice of Transport Layer

\[FIXME\]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could just note that existing debugging protocols use WebSockets, so we will too.


### Choice of Protocol Layer

\[FIXME. Why JSON?\]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer is it matches the prior art. We should also say why we can't use JSON-RPC verbatim, and perhaps say that using CBOR is a future possibility, and the design allows for it by avoiding sending binary WebSockets messages entirely, and the first line of https://w3c.github.io/webdriver-bidi/#handle-an-incoming-message

(Although maybe we should change that first line to silently drop binary messages or respond with an error instead of closing the connectin...)

\[FIXME. Why JSON?\]

CDDL is used to describe the protocol layer because it provides formal semantics, accomplishing the
"machine-readable API specification" goal while being similar to JSON-RPC used in the prior art.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could do with some tweaking I think. CDDL describes the shapes of messages that can be encoded as JSON, and we're using CDDL to describe something very similar to JSON-RPC, but CDDL itself is not similar to JSON-RPC, they're on different layers.

Perhaps "while allowing us to define a protocol similar to JSON-RPC used in the prior art"?


### Considered alternatives

\[FIXME: Adopting CDP wholesale\]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is where we mention WebTransport and CBOR, two newer things we're not going to use right now.

- [Bootstrap Scripts](./proposals/bootstrap-scripts.md)
Any protocol that can be used for web testing automation can also open browsers up to malicious
actors. It is vital that any functionality cannot be accessed from web platform content; browsers
with multi-process architectures may want to minimise the amount of functionality within the web
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very good point!

Aside: This is actually relevant to the question of whether the navigate command should support relative URLs, because to do so would require going to the content process to resolve the URL, or using a cached-and-maybe-stale URL in another process.


## Overview
WebDriver BiDi (Bi-Directional) is being developed to allow web developers to migrate from
Chromium-only [CDP](https://chromedevtools.github.io/devtools-protocol/)-based (Chrome DevTools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we remove the Chromium-only bit here given that we also have (partial) support for it in Firefox?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably makes sense to say something about it being non-standard and de-facto defined by Chromium?

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

Successfully merging this pull request may close these issues.

5 participants