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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 580063003: Stitch ServiceWorkerCache between renderer and browser. (Closed)

Created:
6 years, 3 months ago by jkarlin
Modified:
6 years, 2 months ago
Reviewers:
michaeln, Tom Sepez, horo
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, tzik, serviceworker-reviews, jam, kinuko+serviceworker, nhiroki, darin-cc_chromium.org, horo+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@gavin_code
Project:
chromium
Visibility:
Public.

Description

Connect the ServiceWorkerCache functionality between the renderer and the browser. BUG=392621 Committed: https://crrev.com/a934835ea3e3091dc6f57681e5af74c257acd01f Cr-Commit-Position: refs/heads/master@{#297848}

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Nits #

Total comments: 2

Patch Set 4 : Address comments from PS3 #

Total comments: 4

Patch Set 5 : Addresses issues from PS4 #

Patch Set 6 : Rebase #

Patch Set 7 : Rebase, now supports response in Put() callback #

Total comments: 2

Patch Set 8 : Address comments from PS7 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -9 lines) Patch
M content/browser/service_worker/service_worker_cache_listener.h View 1 2 3 4 5 6 3 chunks +18 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_cache_listener.cc View 1 2 3 4 5 6 7 6 chunks +180 lines, -9 lines 0 comments Download

Messages

Total messages: 18 (4 generated)
jkarlin
michaeln@chromium.org: Please review changes in everything. horo@chromium.org: Please review changes in everything. tsepez@chromium.org: Please look ...
6 years, 2 months ago (2014-09-26 15:45:08 UTC) #2
Tom Sepez
LGTM https://codereview.chromium.org/580063003/diff/40001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/40001/content/browser/service_worker/service_worker_cache_listener.cc#newcode266 content/browser/service_worker/service_worker_cache_listener.cc:266: } else if (operation.operation_type == nit: else after ...
6 years, 2 months ago (2014-09-26 16:01:48 UTC) #3
jkarlin
Thanks! https://codereview.chromium.org/580063003/diff/40001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/40001/content/browser/service_worker/service_worker_cache_listener.cc#newcode266 content/browser/service_worker/service_worker_cache_listener.cc:266: } else if (operation.operation_type == On 2014/09/26 16:01:48, ...
6 years, 2 months ago (2014-09-26 16:29:11 UTC) #4
horo
https://codereview.chromium.org/580063003/diff/60001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/60001/content/browser/service_worker/service_worker_cache_listener.cc#newcode200 content/browser/service_worker/service_worker_cache_listener.cc:200: Send(ServiceWorkerMsg_CacheMatchError( This should be removed. https://codereview.chromium.org/580063003/diff/60001/content/browser/service_worker/service_worker_cache_listener.cc#newcode426 content/browser/service_worker/service_worker_cache_listener.cc:426: ServiceWorkerMsg_CacheBatchSuccess(request_id, Why ...
6 years, 2 months ago (2014-09-29 04:05:05 UTC) #5
jkarlin
https://codereview.chromium.org/580063003/diff/60001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/60001/content/browser/service_worker/service_worker_cache_listener.cc#newcode200 content/browser/service_worker/service_worker_cache_listener.cc:200: Send(ServiceWorkerMsg_CacheMatchError( On 2014/09/29 04:05:04, horo wrote: > This should ...
6 years, 2 months ago (2014-09-29 13:04:14 UTC) #6
horo
lgtm
6 years, 2 months ago (2014-09-29 13:13:45 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/580063003/80001
6 years, 2 months ago (2014-09-29 13:23:42 UTC) #9
michaeln
lgtm https://codereview.chromium.org/580063003/diff/120001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/120001/content/browser/service_worker/service_worker_cache_listener.cc#newcode247 content/browser/service_worker/service_worker_cache_listener.cc:247: const ServiceWorkerBatchOperation operation = operations[0]; can this be ...
6 years, 2 months ago (2014-10-01 01:58:58 UTC) #11
jkarlin
https://codereview.chromium.org/580063003/diff/120001/content/browser/service_worker/service_worker_cache_listener.cc File content/browser/service_worker/service_worker_cache_listener.cc (right): https://codereview.chromium.org/580063003/diff/120001/content/browser/service_worker/service_worker_cache_listener.cc#newcode247 content/browser/service_worker/service_worker_cache_listener.cc:247: const ServiceWorkerBatchOperation operation = operations[0]; On 2014/10/01 01:58:58, michaeln ...
6 years, 2 months ago (2014-10-02 14:54:19 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/580063003/140001
6 years, 2 months ago (2014-10-02 16:28:52 UTC) #14
commit-bot: I haz the power
Committed patchset #8 (id:140001) as bd33e01d81b6b2075a51f6deed6c8b12cd25fefd
6 years, 2 months ago (2014-10-02 16:32:41 UTC) #15
commit-bot: I haz the power
Patchset 8 (id:??) landed as https://crrev.com/a934835ea3e3091dc6f57681e5af74c257acd01f Cr-Commit-Position: refs/heads/master@{#297848}
6 years, 2 months ago (2014-10-02 16:33:16 UTC) #16
jkarlin
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/623703004/ by jkarlin@chromium.org. ...
6 years, 2 months ago (2014-10-02 16:43:13 UTC) #17
jkarlin
6 years, 2 months ago (2014-10-02 16:49:08 UTC) #18
Message was sent while issue was closed. 
On 2014/10/02 16:43:13, jkarlin wrote:
> A revert of this CL (patchset #8 id:140001) has been created in
> https://codereview.chromium.org/623703004/ by mailto:jkarlin@chromium.org.
> 
> The reason for reverting is: Trybot tests were stale.  Need to rebase and land
> again..

Reland is here: https://codereview.chromium.org/622783003/

Powered by Google App Engine
This is Rietveld 408576698