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

Merge QuicTransport and Http3Transport into a single class? #129

Closed
vasilvv opened this issue Aug 13, 2020 · 6 comments · Fixed by #134
Closed

Merge QuicTransport and Http3Transport into a single class? #129

vasilvv opened this issue Aug 13, 2020 · 6 comments · Fixed by #134

Comments

@vasilvv
Copy link
Contributor

vasilvv commented Aug 13, 2020

Currently we have two separate constructors for QuicTransport and Http3Transport:

let transport1 = new QuicTransport("quic-transport://example.org/endpoint");
let transport2 = new Http3Transport("https://example.org/endpoint");

Notice that the transport information is redundantly encoded twice: once in the class name, and once in the scheme.

I propose that we merge those into a single constructor that dispatches based on the scheme:

let transport1 = new WebTransport("quic-transport://example.org/endpoint");  // QuicTransport 
let transport2 = new WebTransport("https://example.org/endpoint");  // Http{2,3}Transport

This has a nice property of increasing transport agility (since URLs are typically data, transport can be changed with no code changes involved), and will probably simplify client implementation too (by reducing code duplication).

@ghost
Copy link

ghost commented Aug 13, 2020

How might protocol agility and support be managed? Without a 1:1 mapping of class to protocol, should this change also include a static method that can expose which schemes are available for a given implementation?

@yutakahirano
Copy link
Contributor

What will the relationship between WebTransport and RTCQuicTransport be?

@vasilvv
Copy link
Contributor Author

vasilvv commented Aug 14, 2020

What will the relationship between WebTransport and RTCQuicTransport be?

I expect RTCQuicTransport to implement the same set of mixins, but have different class, since any RTC version would be unlikely to be constructible from a URL.

How might protocol agility and support be managed? Without a 1:1 mapping of class to protocol, should this change also include a static method that can expose which schemes are available for a given implementation?

A static method is an option, though since various transports can be made unavailable for various reasons (lack of support in browser, enterprise policy, network firewall, etc), I believe the best approach would be trying to connect to a given URL to see what happens. To quote TAG design principles:

It should generally be indistinguishable why a feature is unavailable, so that feature detection code written for one case of unavailability (e.g., the feature not being implemented yet in some browsers) also works in other cases (e.g., a library being used in a non-secure context when the feature is limited to secure contexts).

@ricea
Copy link
Contributor

ricea commented Aug 14, 2020

What should getStats() do if the transport is not QUIC?

@aboba
Copy link
Collaborator

aboba commented Aug 18, 2020

@yutakahirano I assume that RTCQuicTransport will inherit from WebTransport instead of from QuicTransport: https://w3c.github.io/webrtc-quic/#rtcquic-transport*

However, because there is no URI in the constructor I'm assuming that only the QuicTransport protocol would be supported with RTCQuicTransport.

@Tiedye
Copy link

Tiedye commented Aug 31, 2020

Having the additional agility of this scheme seems nice. Though I think the dynamically dispatching constructor is an unusual pattern that isn't really seen in other API's. Maybe a (static) method like document.createElement would be more appropriate to not violate developers expectations of creating objects with new.

vasilvv added a commit to vasilvv/web-transport that referenced this issue Sep 15, 2020
pull bot pushed a commit to FreddyZeng/chromium that referenced this issue Sep 16, 2020
Expose WebTransport interface so that we can use the interface for
various protocols such as QUIC and HTTP/3. We keep QuicTransport
for some time to make the transition easier.

w3c/webtransport#129

Bug: 1123413
Change-Id: I8a6cc835fa0ae44804b5cb21bf6e5553c371334a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386692
Commit-Queue: Yutaka Hirano <[email protected]>
Reviewed-by: Victor Vasiliev <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807410}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Expose WebTransport interface so that we can use the interface for
various protocols such as QUIC and HTTP/3. We keep QuicTransport
for some time to make the transition easier.

w3c/webtransport#129

Bug: 1123413
Change-Id: I8a6cc835fa0ae44804b5cb21bf6e5553c371334a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386692
Commit-Queue: Yutaka Hirano <[email protected]>
Reviewed-by: Victor Vasiliev <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807410}
GitOrigin-RevId: c81edd9d28ec0116464af497c2b202eff5ea61de
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants