Creates CacheContext for ServiceWorkerCacheStorage

Creates CacheContext for ServiceWorkerCacheStorage.

Creates CacheContext which the CacheStorage uses to keep track of
Cache metadata rather than sticking it in the ServiceWorkerCache where
it doesn't belong.  The CacheContext will be used in a downstream CL
to keep reference count information for each cache.

This CL also changes the CacheMap from an IDMap to a std::map with our own id counting.  
This lets us change to int64 from int32 as well as helps the transition to reference counting the caches.

Some other upcoming CLs affecting CacheStorage:
* Reference count CacheContext so that the deletion of javascript objects can delete the ServiceWorkerCache
* Make the callbacks in ServiceWorkerCacheStorage unnamed functions that don't require weak pointers.
* Change the CacheID to int64.


BUG=392621
Committed: https://crrev.com/098fdfb498f69caee3c83b2f8ee21b085f758731
Cr-Commit-Position: refs/heads/master@{#291906}

[jkarlin, 2014-08-25 19:00:02]

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

[jkarlin, 2014-08-25 19:00:02]

falken: Please review everything.  Thanks!

[falken, 2014-08-26 06:31:33]

lgtm with nits

nit: I'd add "Service Worker" somewhere in the first line of the CL description.
Also, some of the description is wrapped and some isn't.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.cc (right):

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:27: //
TODO(jkarlin): Add reference counting.
Depending on how long you plan to take, it might be good to make this TODO more
specific. Something like "so that the deletion of javascript objects can delete
the ServiceWorkerCache" as in the CL description.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:421:
callback.Run(-1, CACHE_STORAGE_ERROR_EMPTY_KEY);
Should we have a kInvalidCacheId constant?

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:510:
scoped_ptr<CacheContext> cache_context(GetLoadedCache(cache_name));
It's a bit subtle that the raw pointer returned by GetLoadedCache is wrapped in
a scoped_ptr specifically to delete it. Maybe add a brief comment?

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:654:
scoped_ptr<ServiceWorkerCache> sw_cache) {
nit: Why rename? The other ServiceWorkerCache objects in this file are just
"cache".

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.h (right):

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:13: #include
"base/id_map.h"
Can this id_map.h be removed?

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:146: // object.
The CacheIDs are used by JavaScript to reference a
nit: Be consistent about CacheID vs CacheId

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:149: CacheID
next_cache_id_;  // The next CacheID to use in cache_map_;
nit: no need to end comment with semicolon

[jkarlin, 2014-08-26 12:03:10]

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.cc (right):

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:27: //
TODO(jkarlin): Add reference counting.
On 2014/08/26 06:31:32, falken (ooo. back Sep 26) wrote:
> Depending on how long you plan to take, it might be good to make this TODO
more
> specific. Something like "so that the deletion of javascript objects can
delete
> the ServiceWorkerCache" as in the CL description.

Done.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:421:
callback.Run(-1, CACHE_STORAGE_ERROR_EMPTY_KEY);
On 2014/08/26 06:31:33, falken (ooo. back Sep 26) wrote:
> Should we have a kInvalidCacheId constant?

Done. Now used in the tests as well.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:510:
scoped_ptr<CacheContext> cache_context(GetLoadedCache(cache_name));
On 2014/08/26 06:31:32, falken (ooo. back Sep 26) wrote:
> It's a bit subtle that the raw pointer returned by GetLoadedCache is wrapped
in
> a scoped_ptr specifically to delete it. Maybe add a brief comment?

Agree.  Specifically call .reset() now to make it clear.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:654:
scoped_ptr<ServiceWorkerCache> sw_cache) {
On 2014/08/26 06:31:32, falken (ooo. back Sep 26) wrote:
> nit: Why rename? The other ServiceWorkerCache objects in this file are just
> "cache".

Done.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.h (right):

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:13: #include
"base/id_map.h"
On 2014/08/26 06:31:33, falken (ooo. back Sep 26) wrote:
> Can this id_map.h be removed?

Done.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:146: // object.
The CacheIDs are used by JavaScript to reference a
On 2014/08/26 06:31:33, falken (ooo. back Sep 26) wrote:
> nit: Be consistent about CacheID vs CacheId

Done.

https://codereview.chromium.org/503823002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.h:149: CacheID
next_cache_id_;  // The next CacheID to use in cache_map_;
On 2014/08/26 06:31:33, falken (ooo. back Sep 26) wrote:
> nit: no need to end comment with semicolon

Done.

[jkarlin, 2014-08-26 12:16:13]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-08-26 12:17:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jkarlin@chromium.org/503823002/100001

[commit-bot: I haz the power, 2014-08-26 14:49:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-26 15:30:32]

Committed patchset #6 (100001) as 91ae1291b44e34443bec85577b923ba5c40ac40c

[commit-bot: I haz the power, 2014-09-10 02:42:55]

Patchset 6 (id:??) landed as
https://crrev.com/098fdfb498f69caee3c83b2f8ee21b085f758731
Cr-Commit-Position: refs/heads/master@{#291906}