content::WebServiceWorkerCache implementation.

content::WebServiceWorkerCache implementation.

The WebCache object itself is created by the
ServiceWorkerCacheStorageDispatcher, which also handles much of its
messaging for lifetime reasons, since a cache operation can outlive
its WebServiceWorkerCache object.

R=falken@chromium.org,jkarlin@chromium.org
BUG=None

Committed: https://crrev.com/203f6295dc41e5172931661cfa106ce9a66da92e
Cr-Commit-Position: refs/heads/master@{#296726}

[gavinp, 2014-08-14 01:13:47]

falken, jkarlin: PTAL

[michaeln, 2014-08-15 02:50:24]

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:330:
std::vector<content::ServiceWorkerResponse>)
For the Match/MatchSuccess and MatchAll/MatchAllSuccess case, you also define
case, can you also define a 3rd message type to for the renderer to send to the
browser to Ack receipt of the Success message. I think we need an akc to
transfer ownership of the blobhandles embedded in these messages (so the browser
knows it can drop the handles its got open for them).

[dominicc (has gone to gerrit), 2014-08-15 06:48:36]

DBCs inline.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_listener.cc (right):

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.cc:64: void
ServiceWorkerCacheListener::OnCacheStorageCreate(
Can you privatize some of these? Or is this a Chrome style thing... Seems
obvious this should not be used on multiple threads; detering casual callers
would reinforce that.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.cc:69:
Send(ServiceWorkerMsg_CacheStorageCreateSuccess(request_id, next_cache_id++));
Since zero is a default value, might be nice to not use it as an ID.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_version.h:288:
scoped_ptr<ServiceWorkerCacheListener> stores_listener_;
Does it make sense to rename this member as well?

Does it make sense to do a renaming pass, then add new things in a separate CL?

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
File content/common/service_worker/service_worker_types.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_types.h:79: // In the Cache API
this struct controls the operation of matching in operations
This is repetitively repetitious ... the operation of operations; the matching
that matches; "this struct"... what else would it refer to?

How about:

Controls how requests are matched in the Cache API.

Maybe comments below can be tightened up too.

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_types.h:100: // In the Cache API,
this struct describes a single operation of PUT or DELETE.
Why all caps for put and delete? These look like HTTP methods, which are
cursorily related (cache, requests, methods) so its confusing to have them it
caps.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:28:
class StringMapHTTPHeaderVisitor : public blink::WebHTTPHeaderVisitor {
I wonder if this is the best name? A BirthdayCake is more appetizing than a
CandlesFlourEggsSugarCelebratoryItem.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:130: 
I'm sad at this point that browser can not dabble in Blink types. (Can it?) Not
your fault.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:157:
// The WebCache object is the chrome side implementation of the blink
chrome -> Chrome or Chromium
blink -> Blink

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:193:
// Not owned. Pointer to a dispatcher to use for sending events.
What's the disengagement protocol? Because I thought (but I may be confused)
that WebServiceWorkerCache (ie this) is owned by Blink.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:380:
for (size_t i = 0; i < responses.size(); ++i)
Seems you could have a helper for this; it happens a bunch. Same for requests.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:398: 
There's some inconsistency here wrt vertical whitespace; the previous methods
don't have this blank...

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:463:
int request_id = has_callbacks_.Add(callbacks);
Would be nice if this kind of error was unpossible... is there a neat way to do
that?

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.h
(right):

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:1: 
Delete this blank line.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:27: //
to the browser process and runs callbacks at operation completion. As well,
Can you rephrase to integrate with the existing comment and avoid "As well"?
Organizing this list of duties in a rough chronology probably makes sense.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:37: //
Although this class does not register for messages in the normal way, the
Be direct; the headline here is that ServiceWorkerScriptContext demuxes IPC
messages and forwards them here. Lead with that.

[dominicc (has gone to gerrit), 2014-08-21 06:25:24]

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:157:
// The WebCache object is the chrome side implementation of the blink
Doesn't this need a destructor to clean up the entry in the dispatcher's
web_cache_ map? Where does that happen?

[jsbell, 2014-08-27 21:11:34]

jsbell@chromium.org changed reviewers:
+ jsbell@chromium.org

[jsbell, 2014-08-27 21:11:34]

Maybe I'm just seeing this in an intermediate state...

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:270:
WebCache* web_cache = new WebCache(this, cache_id);
Presumably you don't want a new web_cache local here, but to assign to the one
outside the if block?

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:292:
WebCache* web_cache = new WebCache(this, cache_id);
Ditto?

[gavinp, 2014-09-03 18:35:25]

I've remediated to reviews, and matched this up more closely to upstream CLs.
Please take another look, I'll be going over it myself as well.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_listener.cc (right):

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.cc:64: void
ServiceWorkerCacheListener::OnCacheStorageCreate(
On 2014/08/15 06:48:34, dominicc--slow until 9.9 wrote:
> Can you privatize some of these? Or is this a Chrome style thing... Seems
> obvious this should not be used on multiple threads; detering casual callers
> would reinforce that.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.cc:69:
Send(ServiceWorkerMsg_CacheStorageCreateSuccess(request_id, next_cache_id++));
On 2014/08/15 06:48:34, dominicc--slow until 9.9 wrote:
> Since zero is a default value, might be nice to not use it as an ID.

Moot, that was test code that shouldn't have been uploaded; the actual cache id
allocation code is now in place.

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_version.h:288:
scoped_ptr<ServiceWorkerCacheListener> stores_listener_;
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Does it make sense to rename this member as well?
> 
> Does it make sense to do a renaming pass, then add new things in a separate
CL?

This is now out of this CL.

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:330:
std::vector<content::ServiceWorkerResponse>)
On 2014/08/15 02:50:24, michaeln wrote:
> For the Match/MatchSuccess and MatchAll/MatchAllSuccess case, you also define
> case, can you also define a 3rd message type to for the renderer to send to
the
> browser to Ack receipt of the Success message. I think we need an akc to
> transfer ownership of the blobhandles embedded in these messages (so the
browser
> knows it can drop the handles its got open for them).

That's reasonable, but right now we intend to add blobs to Responses on a
downstream CL rather than this one, there's a TODO in the content side of this
to that effect. Should we add a TODO somewhere in here?

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
File content/common/service_worker/service_worker_types.h (right):

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_types.h:79: // In the Cache API
this struct controls the operation of matching in operations
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> This is repetitively repetitious ... the operation of operations; the matching
> that matches; "this struct"... what else would it refer to?
> 
> How about:
> 
> Controls how requests are matched in the Cache API.
> 
> Maybe comments below can be tightened up too.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/common/service_w...
content/common/service_worker/service_worker_types.h:100: // In the Cache API,
this struct describes a single operation of PUT or DELETE.
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Why all caps for put and delete? These look like HTTP methods, which are
> cursorily related (cache, requests, methods) so its confusing to have them it
> caps.

"Though the Google C++ Style Guide now says to use kConstantNaming for enums,
Chromium was written using MACRO_STYLE naming.  Continue to use this style for
consistency."

http://www.chromium.org/developers/coding-style

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:28:
class StringMapHTTPHeaderVisitor : public blink::WebHTTPHeaderVisitor {
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> I wonder if this is the best name? A BirthdayCake is more appetizing than a
> CandlesFlourEggsSugarCelebratoryItem.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:130: 
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> I'm sad at this point that browser can not dabble in Blink types. (Can it?)
Not
> your fault.

You can, provided that the enumeration is defined in a header that defines ONLY
"pod types" (we say only "pod types" but we permit trivial constructors). This
enumeration is defined in WebServiceWorkerCache.h, so that can't be included
from any non-renderer context.

I'd like to see us use more blink types; we have far too many of these mappings.
The way to do that is to move as many definitions of this type as possible into
simple util headers in the blink API, then we can include them like crazy on the
browser side.

I avoided doing that because it's not the existing pattern. If anything I'm
already doing more of that than most hackers do.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:157:
// The WebCache object is the chrome side implementation of the blink
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> chrome -> Chrome or Chromium
> blink -> Blink

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:193:
// Not owned. Pointer to a dispatcher to use for sending events.
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> What's the disengagement protocol? Because I thought (but I may be confused)
> that WebServiceWorkerCache (ie this) is owned by Blink.

It is. Its destructor is called by Blink when the blink::Cache object is
destroyed. The latest version of the CL adds notification up to the browser side
of this, so the object in the browser's memory for the open cache can be
cleaned.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:380:
for (size_t i = 0; i < responses.size(); ++i)
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Seems you could have a helper for this; it happens a bunch. Same for requests.

I think it's just twice for responses and once for requests. Adding the utility
functions makes the code longer.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:398: 
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> There's some inconsistency here wrt vertical whitespace; the previous methods
> don't have this blank...

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:463:
int request_id = has_callbacks_.Add(callbacks);
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Would be nice if this kind of error was unpossible... is there a neat way to
do
> that?

Static type incompatibility seems the way to go, something like:

template<int N> class BrandedBase { };

template <typename T, int N>
class Branded : public T, public BrandedBase<N> {};

then you can instantiate a distinct Branded<> for each callbacks type for
instance, and they won't be type compatible.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.h
(right):

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:1: 
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Delete this blank line.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:27: //
to the browser process and runs callbacks at operation completion. As well,
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Can you rephrase to integrate with the existing comment and avoid "As well"?
> Organizing this list of duties in a rough chronology probably makes sense.

Done.

https://codereview.chromium.org/474593002/diff/20001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:37: //
Although this class does not register for messages in the normal way, the
On 2014/08/15 06:48:35, dominicc--slow until 9.9 wrote:
> Be direct; the headline here is that ServiceWorkerScriptContext demuxes IPC
> messages and forwards them here. Lead with that.

Done.

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:157:
// The WebCache object is the chrome side implementation of the blink
On 2014/08/21 06:25:24, dominicc--slow until 9.9 wrote:
> Doesn't this need a destructor to clean up the entry in the dispatcher's
> web_cache_ map? Where does that happen?

That's now done in
ServiceWorkerCacheStorageDispatcher::onWebCacheDestruction(int).

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:270:
WebCache* web_cache = new WebCache(this, cache_id);
On 2014/08/27 21:11:34, jsbell wrote:
> Presumably you don't want a new web_cache local here, but to assign to the one
> outside the if block?

Done.

https://codereview.chromium.org/474593002/diff/60001/content/renderer/service...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:292:
WebCache* web_cache = new WebCache(this, cache_id);
On 2014/08/27 21:11:34, jsbell wrote:
> Ditto?

Done.

[jsbell, 2014-09-05 23:35:38]

See commentary over in https://codereview.chromium.org/433793002/ but:

https://codereview.chromium.org/544413002 is a Chromium-side patch that needs be
applied to https://codereview.chromium.org/474593002 (this patch)

https://codereview.chromium.org/546153002 is a Blink-side patch that needs to be
applied to https://codereview.chromium.org/433793002/

[michaeln, 2014-09-06 00:56:40]

michaeln@chromium.org changed reviewers:
+ michaeln@chromium.org

[michaeln, 2014-09-06 00:56:41]

https://codereview.chromium.org/474593002/diff/140001/content/common/service_...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/474593002/diff/140001/content/common/service_...
content/common/service_worker/service_worker_messages.h:197:
IPC_MESSAGE_ROUTED4(ServiceWorkerHostMsg_CacheMatch,
The embedded worker routed message passing scheme only works from within service
worker scripts, so if/when this is made callable from within pages, stuff will
have to be rearranged a little. We could tie an instance of the SWCacheListener
in the browser process with an instance of a SWProviderHost and route all cache
related messages that way based on provider_id.

https://codereview.chromium.org/474593002/diff/140001/content/common/service_...
content/common/service_worker/service_worker_messages.h:203:
IPC_MESSAGE_ROUTED4(ServiceWorkerHostMsg_CacheMatchAll,
Does it make sense to handle match and matchAll on the renderer-side? When
opening a cache, load up enough info about it to perform those functions w/o
further roundtrips?

https://codereview.chromium.org/474593002/diff/140001/content/common/service_...
content/common/service_worker/service_worker_messages.h:221: // been provided to
a particular ServiceWorker.
Having this listener class be scope to a particular execution context is very
nice for resource reclamation purposes!

https://codereview.chromium.org/474593002/diff/140001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/140001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:285:
web_cache = new WebCache(this, cache_id);
Was wondering how the ownership semantics of this are handled, very sneaky.

[jsbell, 2014-09-18 22:55:57]

jsbell@chromium.org changed reviewers:
+ tsepez@chromium.org

[jsbell, 2014-09-18 22:55:58]

lgtm I believe with one real nit.

But (1) I'm not in OWNERS and (2) also needs IPC review (tsepez@ ?)

https://codereview.chromium.org/474593002/diff/180001/content/common/service_...
File content/common/service_worker/service_worker_types.cc (right):

https://codereview.chromium.org/474593002/diff/180001/content/common/service_...
content/common/service_worker/service_worker_types.cc:23: is_reload(is_reload)
{}
(I have this vague feeling that we decided that breaking here was preferred in
the implementation and {} was only acceptable in the headers, but it's not clear
in the style guide.)

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.h
(right):

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:114:
void onWebCacheDestruction(int cache_id);
nit: Chromium naming style

[Tom Sepez, 2014-09-18 23:00:15]

Messages LGTM.

[michaeln, 2014-09-19 20:14:46]

r/s lgtm

[gavinp, 2014-09-23 16:02:27]

This is now updated per the change in the blink CL, the complicated ownership
scheme is gone. It's pretty small the change here, but here it is. Now we're
just waiting on upstream.

[jsbell, 2014-09-23 17:32:19]

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.h
(right):

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:114:
void onWebCacheDestruction(int cache_id);
On 2014/09/18 22:55:58, jsbell wrote:
> nit: Chromium naming style

This nit is still valid with the latest patch. Should be 'OnWebCacheDestruction'

[gavinp, 2014-09-23 18:20:16]

Fixed your nit!

https://codereview.chromium.org/474593002/diff/180001/content/common/service_...
File content/common/service_worker/service_worker_types.cc (right):

https://codereview.chromium.org/474593002/diff/180001/content/common/service_...
content/common/service_worker/service_worker_types.cc:23: is_reload(is_reload)
{}
On 2014/09/18 22:55:58, jsbell wrote:
> (I have this vague feeling that we decided that breaking here was preferred in
> the implementation and {} was only acceptable in the headers, but it's not
clear
> in the style guide.)

I've come across this in reviews quite a bit; I just comply. Since you're on the
fence, I won't in this case. There's a lot of examples in both you can find in a
grepfight.

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.h
(right):

https://codereview.chromium.org/474593002/diff/180001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.h:114:
void onWebCacheDestruction(int cache_id);
On 2014/09/23 17:32:19, jsbell wrote:
> On 2014/09/18 22:55:58, jsbell wrote:
> > nit: Chromium naming style
> 
> This nit is still valid with the latest patch. Should be
'OnWebCacheDestruction'

Yes, it is valid. Thanks for the reminder. Now done.

[jsbell, 2014-09-23 18:25:41]

grepfight! grepfight! ;-)

still lgtm !

[gavinp, 2014-09-24 15:06:44]

On 2014/09/23 18:25:41, jsbell wrote:
> grepfight! grepfight! ;-)
> 
> still lgtm !

Upstream has all landed, we are now waiting for gardening to advance enough for
us to land this.

[gavinp, 2014-09-25 15:36:26]

I spoke too soon earlier; upstream reverted, then relanded. However that's done:
https://codereview.chromium.org/603673002/ is the commit on blink side; that is
now gardened in to trunk, so this can run on trybots now.

YAY! Thanks everyone!!!!

[gavinp, 2014-09-25 16:57:24]

The CQ bit was checked by gavinp@chromium.org

[commit-bot: I haz the power, 2014-09-25 16:58:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/474593002/250001

[commit-bot: I haz the power, 2014-09-25 17:03:18]

Committed patchset #14 (id:250001) as ced14e2a597a37bc364ac4f50f9fcbc8dceffa2c

[commit-bot: I haz the power, 2014-09-25 17:03:59]

Patchset 14 (id:??) landed as
https://crrev.com/203f6295dc41e5172931661cfa106ce9a66da92e
Cr-Commit-Position: refs/heads/master@{#296726}

[jochen (gone - plz use gerrit), 2015-01-21 13:55:13]

jochen@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2015-01-21 13:55:14]

https://codereview.chromium.org/474593002/diff/250001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/250001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:70:
blink::WebReferrerPolicy::WebReferrerPolicyNever);
this is not correct. You can't set a referrer and the "never" policy.

[jochen (gone - plz use gerrit), 2015-01-21 13:58:27]

https://codereview.chromium.org/474593002/diff/250001/content/renderer/servic...
File content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc
(right):

https://codereview.chromium.org/474593002/diff/250001/content/renderer/servic...
content/renderer/service_worker/service_worker_cache_storage_dispatcher.cc:70:
blink::WebReferrerPolicy::WebReferrerPolicyNever);
On 2015/01/21 at 13:55:14, jochen (slow) wrote:
> this is not correct. You can't set a referrer and the "never" policy.

The fact that referrer is a URL is also a bug :-/