Serve cached metadata through ServiceWorkerSubresourceLoader |
||||||
Issue descriptionCurrently we don't send the metadata when the response is from cache storage. We need to get it and pass it to clients for improving performance. Before NetS13nServiceWorker, ServiceWorkerBlobReader internally creates an URLRequest by storage::BlobProtocolHandler::CreateBlobRequest(), and the response_info() of the request has a metadata. We send it. https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_blob_reader.h?type=cs&sq=package:chromium&g=0&l=28 Probably we need to get it from a blob ptr or something, but I'm not sure where is the best since I don't understand the whole code path.
,
Jun 26 2018
,
Jun 26 2018
I think we can probably get side data from BlobReader::ReadSideData() also in the renderer process (that's what BlobURLRequestJob (called from CreateBlobRequest) is doing)
,
Jun 26 2018
Oops, sorry the method wasn't exposed on mojom, so we need something there.
,
Jun 26 2018
Oh yes we have Blob::ReadSideData. (Sorry for noisy updates)
,
Jun 26 2018
Yep, the mojom Blob interface has a ReadSideData method that should return the cached metadata if the blob was created from a cache entry. Semi-related question: how big does this cached metadata tend to be? Currently the ReadSideData method just returns the data directly as a array<uint8>, since that's how it was done in URLLoaderClient.OnReceivedCachedMetadata as well. But that always felt wrong to me, so not sure if we should be changing that to its own datapipe or (simpler) just a mojo_base.mojom.BigBuffer... (Oh, and other question, does this mean that ServiceWorkerBlobReader is pretty much dead (or will soon be dead), and so I don't have to worry about migrating that away from using BlobProtocolHandler?)
,
Jun 26 2018
,
Jun 27 2018
#6- yes as far as I know cached metadata could be big, and that's been a bit bothering me too. I think that could probably be a nice-to-do follow-up work. And to the other question, yes, we're starting to run the new S13N code path on Canary and once that looks good we should be able to remove ServiceWorkerBlobReader code.
,
Jun 27 2018
Thanks. I'll make a CL soon.
,
Jun 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b34136fd99672df4192100d8181384bffcdc1b8 commit 5b34136fd99672df4192100d8181384bffcdc1b8 Author: Makoto Shimazu <shimazu@chromium.org> Date: Thu Jun 28 09:22:31 2018 Serve cached metadata through ServiceWorkerSubresourceLoader Currently we don't read the cached metadata (= blob's side data) at the subresource loader. This patch is to read it and send it back to the client. Bug: 856521 Change-Id: I8ba4010e6551ba32ff227e47390d974c64bfbfdc Reviewed-on: https://chromium-review.googlesource.com/1116630 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#571056} [modify] https://crrev.com/5b34136fd99672df4192100d8181384bffcdc1b8/content/renderer/service_worker/service_worker_subresource_loader.cc [modify] https://crrev.com/5b34136fd99672df4192100d8181384bffcdc1b8/content/renderer/service_worker/service_worker_subresource_loader.h [modify] https://crrev.com/5b34136fd99672df4192100d8181384bffcdc1b8/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc
,
Jun 29 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by shimazu@chromium.org
, Jun 26 2018