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

Fetch: Delay buffering Response data. #17745

Merged
merged 1 commit into from Jul 10, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jul 9, 2019

When a service worker executes code like evt.respondWith(fetch(r)) we
should be able to directly pass the fetch's resulting mojo::DataPipe
through without any data copying. This previously did not work,
however, since the BufferingBytesConsumer would immediately start
buffering the Response body data.

This CL fixes this issue by delaying the start of the buffering by a
short amount of time. This gives the service worker time to drain the
pipe.

The delay is currently disabled by default behind the
"BufferingBytesConsumerDelay" feature.

Based on yhirano's draft CL at crrev.com/c/1383755.

Bug: 911036
Change-Id: I65675ce62a7ce593c8994b3e1258634840ba6c2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1679669
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676053}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

Already reviewed downstream.

When a service worker executes code like `evt.respondWith(fetch(r))` we
should be able to directly pass the fetch's resulting mojo::DataPipe
through without any data copying.  This previously did not work,
however, since the BufferingBytesConsumer would immediately start
buffering the Response body data.

This CL fixes this issue by delaying the start of the buffering by a
short amount of time.  This gives the service worker time to drain the
pipe.

The delay is currently disabled by default behind the
"BufferingBytesConsumerDelay" feature.

Based on yhirano's draft CL at crrev.com/c/1383755.

Bug: 911036
Change-Id: I65675ce62a7ce593c8994b3e1258634840ba6c2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1679669
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676053}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants