Expose ServiceWorkerCache objects to ServiceWorkerCacheStorageManager clients.

Expose ServiceWorkerCache objects to ServiceWorkerCacheStorageManager clients.

The CL provides reference counted ServiceWorkerCaches on ServiceWorkerCacheStorage::Get and Create.  Previously only CacheIDs were provided from those functions and there was no way for the client to access the ServiceWorkerCache itself.

This CL makes the following modifications:

1. Make ServiceWorkerCache refcounted
2. Return ServiceWorkerCache* instead of a CacheID from Create() and Put()
3. Removed CacheID from ServiceWorkerCache
4. Removed CacheContext from ServiceWorkerCache (since it really only existed to hold a CacheID)
5. Now keeps track of CacheIDs in ServiceWorkerCacheListener
6. Added memory unit tests

Related CLs:
1. https://crrev.com/542703002: Change ownership of the parameters to ServiceWorkerCache:: Put and Match.
2. https://crrev.com/545533002: Move ServiceWorkerCache backend creation to a lazy init function.
3. https://crrev.com/548533002: Make ServiceWorkerCacheStorage::CacheLoader::LoadCache synchronous
* 4. https://crrev.com/549493002: Expose ServiceWorkerCache objects to ServiceWorkerCacheStorageManager clients.

BUG=392621
Committed: https://crrev.com/69909ccc9c1163bf4718ebb87ea1be42fef87ae4
Cr-Commit-Position: refs/heads/master@{#294392}

[jkarlin, 2014-09-05 18:12:11]

Patchset #2 (id:10009) has been deleted

[jkarlin, 2014-09-05 18:13:06]

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

[jkarlin, 2014-09-05 18:13:07]

Thanks!

[jkarlin, 2014-09-09 18:11:21]

Patchset #2 (id:30001) has been deleted

[jkarlin, 2014-09-09 18:35:07]

This one is now ready for review again, thanks!

[michaeln, 2014-09-09 20:04:59]

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.h (right):

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
content/browser/service_worker/service_worker_cache.h:72: // |response| must
remain valid until the callback is called.
not due to changes in this cl, but the comment is a little stale (on other
public methods too)

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_listener.h (right):

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.h:42: typedef
int32_t CacheID;  // TODO(jkarlin): Bump to 64 bit.
Up till now cacheID uniquely identified a particular cache for an origin (modulo
browser restarts). With this change, cacheID now identifies an instance of a
scoped_refptr<> to a cache, so two different invocations on
OnCacheStorageGet('fred') will return two different ids. In other places we use
the term 'handle' for this sort of indirection.

Does this change matter to the client-side logic? Seems like it would? It can be
made to work either way, but I think stable cacheids would help, otherwise
maintaining a single client-side cache for a given swcache will be more
difficult.

If the indirection sticks, not sure it makes sense to bump to int64, and working
'handle' into the name might be good.

If the indirection doesn't stick and 'ids' identify caches, maybe SWCache can
have an id data member.

[jkarlin, 2014-09-10 15:06:13]

Thanks!

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.h (right):

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
content/browser/service_worker/service_worker_cache.h:72: // |response| must
remain valid until the callback is called.
On 2014/09/09 20:04:58, michaeln wrote:
> not due to changes in this cl, but the comment is a little stale (on other
> public methods too)

Done.

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_listener.h (right):

https://codereview.chromium.org/549493002/diff/70001/content/browser/service_...
content/browser/service_worker/service_worker_cache_listener.h:42: typedef
int32_t CacheID;  // TODO(jkarlin): Bump to 64 bit.
Nice catch.  Fixed.  CacheListener now only stores 1 reference to the
ServiceWorkerCache because the renderer is only going to call drop once, when
it's done with all references to the cacheid.

On 2014/09/09 20:04:58, michaeln wrote:
> Up till now cacheID uniquely identified a particular cache for an origin
(modulo
> browser restarts). With this change, cacheID now identifies an instance of a
> scoped_refptr<> to a cache, so two different invocations on
> OnCacheStorageGet('fred') will return two different ids. In other places we
use
> the term 'handle' for this sort of indirection.
> 
> Does this change matter to the client-side logic? Seems like it would? It can
be
> made to work either way, but I think stable cacheids would help, otherwise
> maintaining a single client-side cache for a given swcache will be more
> difficult.
> 
> If the indirection sticks, not sure it makes sense to bump to int64, and
working
> 'handle' into the name might be good.
> 
> If the indirection doesn't stick and 'ids' identify caches, maybe SWCache can
> have an id data member.

[michaeln, 2014-09-10 18:49:31]

lgtm

https://codereview.chromium.org/549493002/diff/130001/content/browser/service...
File content/browser/service_worker/service_worker_cache_listener.cc (right):

https://codereview.chromium.org/549493002/diff/130001/content/browser/service...
content/browser/service_worker/service_worker_cache_listener.cc:238: DCHECK(it
!= id_to_cache_map_.end());
might want to be more defensive here and early return instead of relying on the
renderer not giving us garbage

[jkarlin, 2014-09-11 13:16:14]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-09-11 13:16:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549493002/150001

[jkarlin, 2014-09-11 13:16:38]

The CQ bit was unchecked by jkarlin@chromium.org

[jkarlin, 2014-09-11 13:26:01]

https://codereview.chromium.org/549493002/diff/130001/content/browser/service...
File content/browser/service_worker/service_worker_cache_listener.cc (right):

https://codereview.chromium.org/549493002/diff/130001/content/browser/service...
content/browser/service_worker/service_worker_cache_listener.cc:238: DCHECK(it
!= id_to_cache_map_.end());
On 2014/09/10 18:49:31, michaeln wrote:
> might want to be more defensive here and early return instead of relying on
the
> renderer not giving us garbage

Done.

[jkarlin, 2014-09-11 13:26:08]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-09-11 13:26:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549493002/170001

[commit-bot: I haz the power, 2014-09-11 13:44:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-11 13:44:02]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jkarlin, 2014-09-11 13:55:14]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-09-11 13:55:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549493002/190001

[commit-bot: I haz the power, 2014-09-11 15:02:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-11 15:02:16]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jkarlin, 2014-09-11 15:04:11]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-09-11 15:05:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549493002/190001

[commit-bot: I haz the power, 2014-09-11 15:45:15]

Committed patchset #9 (id:190001) as fab5ebd2abc5e52eca01f2c8d3ee9fbed294693d

[commit-bot: I haz the power, 2014-09-11 15:48:25]

Patchset 9 (id:??) landed as
https://crrev.com/69909ccc9c1163bf4718ebb87ea1be42fef87ae4
Cr-Commit-Position: refs/heads/master@{#294392}