Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(741)

Issue 1376673002: Revert of ServiceWorkerWriteToCacheJob: refactor (Closed)

Created:
5 years, 2 months ago by horo
Modified:
5 years, 2 months ago
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, tzik, serviceworker-reviews, jam, nhiroki, darin-cc_chromium.org, horo+watch_chromium.org, kinuko+serviceworker, kinuko+watch, blink-worker-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of ServiceWorkerWriteToCacheJob: refactor (patchset #3 id:40001 of https://codereview.chromium.org/1356153002/ ) Reason for revert: This change has broken the Service Worker update logic. See crbug.com/537098 Original issue's description: > ServiceWorkerWriteToCacheJob: refactor > > This CL is a re-land of a previously reviewed and approved CL that was reverted: https://codereview.chromium.org/1315443003 > > ServiceWorkerWriteToCacheJob is a subclass of URLRequestJob that is responsible for filling and updating a write-through cache for ServiceWorker data. Specifically, ServiceWorkerWriteToCacheJob creates an underlying URLRequestJob which it uses to do the actual network request for the script, then returns the script body to the creator of the ServiceWorkerWriteToCacheJob while also storing it in the ServiceWorker cache if it has been updated. > > In order to avoid spurious cache rewrites, ServiceWorkerWriteToCacheJob defers writing an entry for as long as possible: if there is an existing entry, it compares incoming network data to the cache entry's data, and only starts writing once there is a mismatch. Since the mismatch may occur after much data has already been handled, once this happens ServiceWorkerWriteToCacheJob must transparently copy all the existing data up to that point from the cache, then resume streaming network data into the cache and back to its consumer. This is the "compare-and-copy" process. If there is no existing entry, network data is streamed directly to the cache; this is the "passthrough" process. > > This CL removes the code implementing the compare-and-copy and passthrough processes from ServiceWorkerWriteToCacheJob and moves it into a separate class, ServiceWorkerCacheWriter. ServiceWorkerCacheWriter exposes an asynchronous API for overwriting a single cache entry that is now used by ServiceWorkerWriteToCacheJob. > > BUG=474859 > TBR=jochen,falken > > Committed: https://crrev.com/d778a5a82cdccab2b3cbf4940749753c98e59dcc > Cr-Commit-Position: refs/heads/master@{#349669} > > patch from issue 1315443003 at patchset 260001 (http://crrev.com/1315443003#ps260001) > > Committed: https://crrev.com/015f4a69ad39e4625648e160dde9b75f8aaad522 > Cr-Commit-Position: refs/heads/master@{#350824} TBR=mmenke@chromium.org,falken@chromium.org,ellyjones@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=474859 Committed: https://crrev.com/2ec1d6fc3c5bc82ce9c2e2ce2da3a206f353830e Cr-Commit-Position: refs/heads/master@{#351264}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+510 lines, -1584 lines) Patch
M content/browser/appcache/appcache_response.h View 2 chunks +4 lines, -7 lines 0 comments Download
D content/browser/service_worker/service_worker_cache_writer.h View 1 chunk +0 lines, -230 lines 0 comments Download
D content/browser/service_worker/service_worker_cache_writer.cc View 1 chunk +0 lines, -498 lines 0 comments Download
D content/browser/service_worker/service_worker_cache_writer_unittest.cc View 1 chunk +0 lines, -697 lines 0 comments Download
M content/browser/service_worker/service_worker_write_to_cache_job.h View 5 chunks +22 lines, -20 lines 0 comments Download
M content/browser/service_worker/service_worker_write_to_cache_job.cc View 18 chunks +484 lines, -129 lines 0 comments Download
M content/content_browser.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M content/content_tests.gypi View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
horo
Created Revert of ServiceWorkerWriteToCacheJob: refactor
5 years, 2 months ago (2015-09-29 07:30:37 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376673002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376673002/1
5 years, 2 months ago (2015-09-29 07:30:51 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376673002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376673002/1
5 years, 2 months ago (2015-09-29 07:31:29 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-09-29 07:32:53 UTC) #4
commit-bot: I haz the power
5 years, 2 months ago (2015-09-29 07:33:47 UTC) #5
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/2ec1d6fc3c5bc82ce9c2e2ce2da3a206f353830e
Cr-Commit-Position: refs/heads/master@{#351264}

Powered by Google App Engine
This is Rietveld 408576698