On Wed, Jun 7, 2023 at 10:01 AM Adam Rice <ri...@chromium.org> wrote:
> I agree that these patterns are likely to be rare, making this a low risk >> endeavor. Please still make sure the flags are working as expected, and >> slowly roll this out, just to be on the safe side. > > > Is it sufficient to roll-out the new cloning behaviour and the BYOB > support separately, or do you want a Finch rollout? I don't find rolling > out small percentages for potentially breaking changes that can only be > detected by bug reports useful, as it leads to "works on my machine, > doesn't work on his" situations which are confusing to debug. > I agree that Finch is not the right choice here. "Careful roll out" would've been a better choice of words. In practice, I think it's sufficient to carefully watch as this lands in Dev and Beta channels, and watch out for any bug reports there. If there are none, it's fine to continue to Stable, while continuing to pay close attention to any breakage reports. > > On Wed, 7 Jun 2023 at 16:47, Yoav Weiss <yoavwe...@chromium.org> wrote: > >> LGTM2 >> >> I agree that these patterns are likely to be rare, making this a low risk >> endeavor. Please still make sure the flags are working as expected, and >> slowly roll this out, just to be on the safe side. >> >> On Tue, Jun 6, 2023 at 8:27 AM Nidhi Jaju <nidhij...@chromium.org> wrote: >> >>> >>> >>> On Tue, Jun 6, 2023 at 1:01 PM Yoav Weiss <yoavwe...@chromium.org> >>> wrote: >>> >>>> >>>> >>>> On Monday, June 5, 2023 at 1:13:11 PM UTC+2 Nidhi Jaju wrote: >>>> >>>> Thank you, Mike and Yoav! >>>> >>>> I filed a standards position request with WebKit, and they are >>>> supportive as well: https://github.com/WebKit/standards-positions/ >>>> issues/194. >>>> >>>> >>>> That's great!! >>>> >>>> >>>> >>>> On Wed, May 31, 2023 at 9:33 PM Yoav Weiss <yoavwe...@chromium.org> >>>> wrote: >>>> >>>> Thanks for aligning with Firefox here! >>>> >>>> On Fri, May 26, 2023 at 5:32 AM Nidhi Jaju <nidhij...@chromium.org> >>>> wrote: >>>> >>>> Contact emailsnidhij...@chromium.org >>>> >>>> ExplainerNone >>>> >>>> Specificationhttps://fetch.spec.whatwg.org/#http-network-fetch >>>> >>>> Summary >>>> >>>> Makes Response.body be a readable byte stream instead of a "default" >>>> readable stream. This enables it to be used with bring-your-own-buffer >>>> (BYOB) readers, reducing garbage collection overhead and copies. >>>> >>>> As mentioned in the Activation section below, we plan to ship the >>>> ReadableStreamTeeCloneForBranch2 feature first, and then the FetchBYOB >>>> feature later. >>>> Blink componentBlink>Network>FetchAPI >>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3ENetwork%3EFetchAPI> >>>> >>>> TAG reviewNone >>>> >>>> TAG review statusNot applicable >>>> >>>> Risks >>>> >>>> >>>> Interoperability and Compatibility >>>> >>>> Low risk because streams and fetch have already been standardized for a >>>> long time. Other browsers have implemented other parts of the standard, and >>>> Firefox has already shipped this behavior for many months and others will >>>> most likely also adapt this feature as well soon. >>>> >>>> >>>> *Gecko*: Shipped/Shipping (https://github.com/whatwg/ >>>> fetch/issues/267#issuecomment-1350303670) Already shipped in Firefox >>>> in 2022. >>>> >>>> *WebKit*: Positive (https://github.com/whatwg/fetch/pull/1593) @annevk >>>> from Apple approved the PR to update the spec with relevant changes and >>>> expressed interest as an implementer on behalf of Apple. >>>> >>>> *Web developers*: No signals >>>> >>>> *Other signals*: Deno is also interested in, and somewhat shipped, >>>> this behavior (https://github.com/denoland/deno/issues/17386). >>>> >>>> Activation >>>> >>>> Currently, to clone a body, we tee the body's stream, but teeing always >>>> returns two "default" streams, where the chunks are not cloned for both >>>> streams. Making Response.body into a byte stream, will mean that cloning it >>>> will result in cloning the chunks for the second stream, which is different >>>> behavior. In order to mitigate activation risks, we are splitting this >>>> change into two releases. One where the default teeing behavior will also >>>> start to clone for the second stream behind the " >>>> ReadableStreamTeeCloneForBranch2" feature flag, and then make >>>> Response.body a readable byte stream behind the "FetchBYOB" feature flag. >>>> >>>> >>>> Can you help me understand what potential breakage (if any) may look >>>> like? >>>> How would these 2 behavior changes break developer expectations? What >>>> would code that breaks as a result of this look like? >>>> >>>> The 2 changes are incremental, and will only break developer >>>> expectations if there are websites that rely on the output of cloning a >>>> request/response to have the same chunks as before or rely on modifying one >>>> of the chunks changing the other one. >>>> >>>> A code sample of where the behavior difference would be observable is: >>>> const request = new Request("image.jpg"); >>>> fetch(request).then((response) => { >>>> const cloned_response = response.clone(); >>>> >>>> const { value: value1 } = await response.getReader().read(); >>>> const { value: value2 } = await cloned_response.getReader().read(); >>>> >>>> if (value1 === value2) { >>>> console.log('old behavior'); >>>> } else { >>>> console.log('new behavior'); >>>> } >>>> }); >>>> >>>> In both the old and the new behaviour, the contents of the Uint8Array >>>> are the same, so it only makes a practical difference if a script modifies >>>> the output. >>>> >>>> >>>> Do we have a sense of an upper bound on potential breakage? e.g. a use >>>> counter for fetch response cloning? >>>> >>> >>> There's no use counter for fetch response cloning, but we believe the >>> breakage will be extremely low, and may not even be a concern. There's not >>> too many realistic scenarios where we might observe a breakage i.e. if the >>> stream is input to some other API, for example the Cache API, then the site >>> can't control the timing when the chunk will be consumed. If a service >>> worker tries modifying a chunk after it has been queued, it is not >>> guaranteed whether it will have been read by the Cache API yet or not, and >>> so whether the modification makes it into the cache will not be >>> deterministic. Hence, it's unlikely a site could be relying on that >>> behaviour. >>> >>> >>>> >>>> >>>> (An explainer that outlined that would've been helpful, but inline >>>> explanations are fine as well) >>>> >>>> >>>> >>>> >>>> >>>> WebView application risks >>>> >>>> Does this intent deprecate or change behavior of existing APIs, such >>>> that it has potentially high risk for Android WebView-based applications? >>>> >>>> >>>> >>>> Debuggability >>>> >>>> No special support needed. >>>> >>>> >>>> Will this feature be supported on all six Blink platforms (Windows, >>>> Mac, Linux, Chrome OS, Android, and Android WebView)?Yes >>>> >>>> This feature will be purely implemented in Blink and so cross-platform >>>> support is automatic. >>>> >>>> >>>> Is this feature fully tested by web-platform-tests >>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md> >>>> ?Yes >>>> >>>> Flag nameReadableStreamTeeCloneForBranch2 and FetchBYOB >>>> >>>> >>>> I'm failing to find the FetchBYOB flag. Has it landed? >>>> >>> >>> Actually, I received feedback that I should rename the flag to >>> "ByobFetch", so the flag is defined here >>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/runtime_enabled_features.json5;l=507;drc=ff1ef24e81e559c4412f9a98911a46840160345b>. >>> Sorry for the confusion! >>> >>> >>>> >>>> >>>> >>>> Requires code in //chrome?False >>>> >>>> Tracking bughttps://bugs.chromium.org/p/chromium/issues/detail?id= >>>> 1243329 >>>> >>>> Estimated milestonesShipping on desktop116Shipping on Android116Shipping >>>> on WebView116 >>>> >>>> Anticipated spec changes >>>> >>>> Open questions about a feature may be a source of future web compat or >>>> interop issues. Please list open issues (e.g. links to known github issues >>>> in the project for the feature specification) whose resolution may >>>> introduce web compat/interop risk (e.g., changing to naming or structure of >>>> the API in a non-backward-compatible way). >>>> None. The spec change for BYOB support for Fetch was already landed at >>>> https://github.com/whatwg/fetch/pull/1593. >>>> >>>> Link to entry on the Chrome Platform Statushttps://chromestatus.com/ >>>> feature/5192003450568704 >>>> >>>> Links to previous Intent discussionsIntent to prototype: >>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/Iyt6Ca9PiJQ/m/s_ >>>> D7A0YwCgAJ >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "blink-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to blink-dev+unsubscr...@chromium.org. >>>> To view this discussion on the web visit https://groups.google.com/a/ >>>> chromium.org/d/msgid/blink-dev/CAMZNYANt%2BwCoAYOfh1Bj7EJrdTz3jkd%3D% >>>> 3DAyeBv9f8P2Ccu1wkg%40mail.gmail.com >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMZNYANt%2BwCoAYOfh1Bj7EJrdTz3jkd%3D%3DAyeBv9f8P2Ccu1wkg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>>> -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfXbZaQ8rvBizNzMFfrE_nkE3udBdKUNrzaEjN3gEQRiGg%40mail.gmail.com.
