Commit 56dd83c9 authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

service worker: s13n: Support partial responses from blobs better.

This CL allows media to play and be seekable when service worker
provides the response from cache storage.
Live demo from Jake Archibald: https://static-misc.glitch.me/sw-audio-bug/

This CL fixes two things:
- Supports blob responses from service worker for partial requests with
  a range of [x, -1] where x is not 0 (the zero case was already
  supported). This requires changes to
  ServiceWorkerLoaderHelpers::ReadBlobResponseBody and
  mojom.Blob.ReadRange().
- Sets ResourceResponseInfo.content_length based on the HTTP header
  Content-Length for service worker loaders. This allows Chromium's
  media code to allow seeking over the media source.

Bug: 892227
Change-Id: I9e6f6faaae7b83ad3aee9c0f217834d62c214b85
Reviewed-on: https://chromium-review.googlesource.com/c/1263739Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597057}
parent 623c4e77
......@@ -4,7 +4,10 @@
#include "content/browser/service_worker/service_worker_navigation_loader.h"
#include <string>
#include <utility>
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "content/browser/loader/navigation_loader_interceptor.h"
......@@ -51,6 +54,10 @@ blink::mojom::FetchAPIResponsePtr OkResponse(
response->status_text = "OK";
response->response_type = network::mojom::FetchResponseType::kDefault;
response->blob = std::move(blob_body);
if (response->blob) {
response->headers.emplace("Content-Length",
base::NumberToString(response->blob->size));
}
return response;
}
......@@ -678,6 +685,7 @@ TEST_F(ServiceWorkerNavigationLoaderTest, BlobResponse) {
const network::ResourceResponseHead& info = client_.response_head();
EXPECT_EQ(200, info.headers->response_code());
ExpectResponseInfo(info, *CreateResponseInfoFromServiceWorker());
EXPECT_EQ(33, info.content_length);
// Test the body.
std::string body;
......
......@@ -4,6 +4,7 @@
#include "content/common/service_worker/service_worker_loader_helpers.h"
#include <limits>
#include <memory>
#include <string>
#include <utility>
......@@ -44,7 +45,8 @@ class BlobCompleteCaller : public blink::mojom::BlobReaderClient {
};
// Sets |has_range_out| to true if |headers| specify a single range request, and
// |offset_out| and |size_out| to the range. Returns true on valid input
// |offset_out| and |length_out| to the range. If the range has an unbounded
// end, |length_out| is set to uint64_t's max value. Returns true on valid input
// (regardless of |has_range_out|), and false if there is more than one range or
// if the bounds overflow.
bool ExtractSinglePartHttpRange(const net::HttpRequestHeaders& headers,
......@@ -67,14 +69,26 @@ bool ExtractSinglePartHttpRange(const net::HttpRequestHeaders& headers,
// Safely parse the single range to our more-sane output format.
*has_range_out = true;
const net::HttpByteRange& byte_range = ranges[0];
// The first byte must be non-negative.
if (byte_range.first_byte_position() < 0)
return false;
// Allow the range [0, -1] to be valid and specify the entire range.
if (byte_range.first_byte_position() == 0 &&
byte_range.last_byte_position() == -1) {
*has_range_out = false;
// The last byte can be -1 to specify unbounded end.
if (byte_range.last_byte_position() == -1) {
// The range [0, -1] is the same as the entire range (no range specified).
if (byte_range.first_byte_position() == 0 &&
byte_range.last_byte_position() == -1) {
*has_range_out = false;
return true;
}
// Otherwise, return the range with unbounded end.
*length_out = std::numeric_limits<uint64_t>::max();
return true;
}
// The last byte must be non-negative.
if (byte_range.last_byte_position() < 0)
return false;
......@@ -133,6 +147,11 @@ void ServiceWorkerLoaderHelpers::SaveResponseHeaders(
if (out_head->headers->GetCharset(&charset))
out_head->charset = charset;
}
// Populate |out_head|'s content length with the value from the HTTP response
// headers.
if (out_head->content_length == -1)
out_head->content_length = out_head->headers->GetContentLength();
}
// static
......@@ -189,9 +208,8 @@ int ServiceWorkerLoaderHelpers::ReadBlobResponseBody(
// We don't support multiple range requests in one single URL request,
// because we need to do multipart encoding here.
// TODO(falken): Support multipart byte range requests.
if (!ExtractSinglePartHttpRange(headers, &byte_range_set, &offset, &length)) {
if (!ExtractSinglePartHttpRange(headers, &byte_range_set, &offset, &length))
return net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
}
mojo::DataPipe data_pipe(blink::BlobUtils::GetDataPipeCapacity(blob_size));
blink::mojom::BlobReaderClientPtr blob_reader_client;
......
......@@ -133,6 +133,10 @@ class FakeControllerServiceWorker : public mojom::ControllerServiceWorker {
response->status_text = "OK";
response->response_type = network::mojom::FetchResponseType::kDefault;
response->blob = std::move(blob_body);
if (response->blob) {
response->headers.emplace("Content-Length",
base::NumberToString(response->blob->size));
}
return response;
}
......@@ -191,6 +195,7 @@ class FakeControllerServiceWorker : public mojom::ControllerServiceWorker {
response_mode_ = ResponseMode::kBlob;
blob_body_ = blink::mojom::SerializedBlob::New();
blob_body_->uuid = "dummy-blob-uuid";
blob_body_->size = body.size();
mojo::MakeStrongBinding(
std::make_unique<FakeBlob>(std::move(metadata), std::move(body)),
mojo::MakeRequest(&blob_body_->blob));
......@@ -885,6 +890,7 @@ TEST_F(ServiceWorkerSubresourceLoaderTest, BlobResponse) {
const network::ResourceResponseHead& info = client->response_head();
ExpectResponseInfo(info, *CreateResponseInfoFromServiceWorker());
EXPECT_EQ(33, info.content_length);
// Test the cached metadata.
client->RunUntilCachedMetadataReceived();
......
......@@ -4,8 +4,10 @@
#include "storage/browser/blob/blob_impl.h"
#include <limits>
#include <memory>
#include <utility>
#include <vector>
#include "net/base/io_buffer.h"
#include "net/disk_cache/disk_cache.h"
......@@ -108,7 +110,10 @@ void BlobImpl::ReadRange(uint64_t offset,
mojo::ScopedDataPipeProducerHandle handle,
blink::mojom::BlobReaderClientPtr client) {
MojoBlobReader::Create(
handle_.get(), net::HttpByteRange::Bounded(offset, offset + length - 1),
handle_.get(),
(length == std::numeric_limits<uint64_t>::max())
? net::HttpByteRange::RightUnbounded(offset)
: net::HttpByteRange::Bounded(offset, offset + length - 1),
std::make_unique<ReaderDelegate>(std::move(handle), std::move(client)));
}
......
......@@ -4,7 +4,10 @@
#include "storage/browser/blob/blob_impl.h"
#include <limits>
#include <memory>
#include <string>
#include <utility>
#include "base/run_loop.h"
#include "base/task/post_task.h"
......@@ -281,6 +284,36 @@ TEST_F(BlobImplTest, ReadRange_TooLargeLength) {
EXPECT_EQ(kContents.size() - 2, client.data_length_);
}
TEST_F(BlobImplTest, ReadRange_UnboundedLength) {
const std::string kId = "id";
const std::string kContents = "hello world";
auto handle = CreateBlobFromString(kId, kContents);
blink::mojom::BlobPtr ptr;
BlobImpl::Create(std::move(handle), MakeRequest(&ptr));
MockBlobReaderClient client;
blink::mojom::BlobReaderClientPtr client_ptr;
mojo::Binding<blink::mojom::BlobReaderClient> client_binding(
&client, MakeRequest(&client_ptr));
mojo::DataPipe pipe;
ptr->ReadRange(2, std::numeric_limits<uint64_t>::max(),
std::move(pipe.producer_handle), std::move(client_ptr));
std::string received = ReadDataPipe(std::move(pipe.consumer_handle));
EXPECT_EQ(kContents.substr(2, kContents.size()), received);
client_binding.FlushForTesting();
EXPECT_TRUE(client.calculated_size_);
EXPECT_EQ(kContents.size(), client.total_size_);
EXPECT_EQ(kContents.size() - 2, client.expected_content_size_);
EXPECT_TRUE(client.completed_);
EXPECT_EQ(net::OK, client.status_);
EXPECT_EQ(kContents.size() - 2, client.data_length_);
}
TEST_F(BlobImplTest, ReadRange_BrokenBlob) {
const std::string kId = "id";
auto handle = context_->AddBrokenBlob(
......
......@@ -39,9 +39,11 @@ interface Blob {
// read operation.
ReadAll(handle<data_pipe_producer> pipe, BlobReaderClient? client);
// Causes a subrange of the contents of this blob to be written into the given
// data pipe. An optional BlobReaderClient will be informed of the result of
// the read operation.
// Causes a subrange of the contents of this blob to be written into the
// given data pipe. If |length| is -1 (uint64_t max), the range's end is
// unbounded so the entire contents are read starting at |offset|. An
// optional BlobReaderClient will be informed of the result of the read
// operation.
ReadRange(uint64 offset, uint64 length, handle<data_pipe_producer> pipe,
BlobReaderClient? client);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment