[ServiceWorker] Added size deltas and total size computation for QuotaM.

[ServiceWorker] Added size deltas and total size computation for QuotaM.

BUG=

Committed: https://crrev.com/66f514ae41532124cd90a4d6c51f594de83853e4
Cr-Commit-Position: refs/heads/master@{#302503}

[dmurph, 2014-10-23 01:24:13]

dmurph@chromium.org changed reviewers:
+ jsbell@chromium.org, michaeln@chromium.org

[dmurph, 2014-10-23 01:24:14]

Hey guys,
PTAL a look at the current patch for adding size deltas and total size
computation.  I believe this is the best approach, but let me know if I missed
something.  Tests TODO

[michaeln, 2014-10-24 23:15:47]

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.cc:192: auto*
usage_map_out = new std::map<GURL, int64>();
It took me a while to understand the reason for the multiple calls and the
combining of the results. Wdyt about adding a 'size_on_disk' data member to
ServiceWorkerRegistrationInfo  instead, so everything can be computed with the
one call to GetAllRegistrations().

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_database.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_database.cc:499: if
(old_registration) {
do we have a usage that passes in null? if not, lets DCHECK(old_reg) instead

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_quota_client.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_quota_client.cc:68:
context_->GetAllOriginsInfo(base::Bind(&AccumulateUsage, callback));
The method is supposed to return the amount of data stored by just the one
|origin|.

  // Gets the amount of data stored in the storage specified by
  // |origin_url| and |type|.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1020: // TODO(dmurph):
Add correct byte delta.
it looks correct, is there something missing?

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1058: }
i think we need a NotifyStorageModified call in here too

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1503:
context_->ScheduleDeleteAndStartOver();
also in the deleteAndStartOVer case i think we need to notifyStorageModified

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_storage.h (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.h:108: // is live.
PurgeResources should be called when it's OK to delete them.
how the comment is wrapped is on with one word lines

[dmurph, 2014-10-27 21:37:07]

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.cc:192: auto*
usage_map_out = new std::map<GURL, int64>();
On 2014/10/24 23:15:47, michaeln wrote:
> It took me a while to understand the reason for the multiple calls and the
> combining of the results. Wdyt about adding a 'size_on_disk' data member to
> ServiceWorkerRegistrationInfo  instead, so everything can be computed with the
> one call to GetAllRegistrations().

I talked to Josh about this for a while, and we decided that
ServiceWorkerRegistrationInfo object already filled a pretty separate role and
it would add potentially buggy logic and extra piping to keep that struct in
sync (as it's maintained in memory).  There are also a lot of places where this
object gets created, and it's difficult to figure out where storing the size is
necessary and how to do it.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_database.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_database.cc:499: if
(old_registration) {
On 2014/10/24 23:15:47, michaeln wrote:
> do we have a usage that passes in null? if not, lets DCHECK(old_reg) instead

Done.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_quota_client.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_quota_client.cc:68:
context_->GetAllOriginsInfo(base::Bind(&AccumulateUsage, callback));
On 2014/10/24 23:15:47, michaeln wrote:
> The method is supposed to return the amount of data stored by just the one
> |origin|.
> 
>   // Gets the amount of data stored in the storage specified by
>   // |origin_url| and |type|.

Done.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1020: // TODO(dmurph):
Add correct byte delta.
On 2014/10/24 23:15:47, michaeln wrote:
> it looks correct, is there something missing?

Done.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1058: }
On 2014/10/24 23:15:47, michaeln wrote:
> i think we need a NotifyStorageModified call in here too

Done.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1503:
context_->ScheduleDeleteAndStartOver();
On 2014/10/24 23:15:47, michaeln wrote:
> also in the deleteAndStartOVer case i think we need to notifyStorageModified

Hm, I'm unsure how to do this best.  Should we be asking the database for the
sizes on all origins?  What if it is inconsistant? This happens a lot when there
is an error from the database.

Maybe we should implement a method on quotamanager to tell it that we're
clearing all data for this client.

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_storage.h (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.h:108: // is live.
PurgeResources should be called when it's OK to delete them.
On 2014/10/24 23:15:47, michaeln wrote:
> how the comment is wrapped is on with one word lines

Done.

[dmurph, 2014-10-28 20:04:46]

Regarding ServiceWorkerRegistrationInfo, if you think that's the best approach,
then I can try doing that.  I'm just concerned with the fact it has it's own
separate lifetime.

[michaeln, 2014-10-28 22:00:40]

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.cc:192: auto*
usage_map_out = new std::map<GURL, int64>();
On 2014/10/27 21:37:06, dmurph wrote:
> On 2014/10/24 23:15:47, michaeln wrote:
> > It took me a while to understand the reason for the multiple calls and the
> > combining of the results. Wdyt about adding a 'size_on_disk' data member to
> > ServiceWorkerRegistrationInfo  instead, so everything can be computed with
the
> > one call to GetAllRegistrations().
> 
> I talked to Josh about this for a while, and we decided that
> ServiceWorkerRegistrationInfo object already filled a pretty separate role and
> it would add potentially buggy logic and extra piping to keep that struct in
> sync (as it's maintained in memory).  There are also a lot of places where
this
> object gets created, and it's difficult to figure out where storing the size
is
> necessary and how to do it.

There don't seem to be many places where these are constructed and not very many
usages alltogether. ServiceWorkerRegistration::GetInfo() is one of them and also
in the act of responding to storage->GetAllRegistrations(...) when the
registration is not 'live'.

The size of a registration only changes when storage->StoreRegistration happens.

Where do you see difficulty?

> and extra piping to keep that struct in
> sync (as it's maintained in memory)

I don't understand that comment, can you explain?

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_database.cc:542: RegistrationData
old_registration_on_stack;
is this still needed given DCHECK(old_registration) up top?

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_database.cc:657:
deleted_version->version_id = registration.version_id;
should we assign the entire struct here instead of just id, the recipient won't
get the size otherwise.

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1347:
deleted_version->version_id = kInvalidServiceWorkerVersionId;
this assignement isn't needed since its constructed with versions_id having this
value

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1359:
base::Owned(deleted_version.release()),
when working with scoped_ptrs<> we usually used base::Passed(&scopedptr) to
transfer ownership, and the sig of the bound function accepts a scoped_ptr<T> as
input

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.h:280:
ServiceWorkerDatabase::RegistrationData* deleted_version,
or maybe use a const T& for this arg to simplify the ownership?

[michaeln, 2014-10-28 22:13:11]

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/672813002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_storage.cc:1503:
context_->ScheduleDeleteAndStartOver();
On 2014/10/27 21:37:06, dmurph wrote:
> On 2014/10/24 23:15:47, michaeln wrote:
> > also in the deleteAndStartOVer case i think we need to notifyStorageModified
> 
> Hm, I'm unsure how to do this best.  Should we be asking the database for the
> sizes on all origins?  What if it is inconsistant? This happens a lot when
there
> is an error from the database.
> 
> Maybe we should implement a method on quotamanager to tell it that we're
> clearing all data for this client.

There might be some trickiness with that but sounds like a good idea.

[dmurph, 2014-10-29 18:22:01]

Here is why it seems a bit complicated to add that to the
ServiceWorkerVersionInfo:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

Where am I getting the size from here?  I could store it directly in
service_Worker_version, and add it as a constructor arg.  This would work here
in storage:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

But I'm having trouble finding the best way to expose the size in the register
job here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

Next, I looked at ServiceWorkerRegistration, as that's a constructor arg.  It
again would be easy to pipe that in in storage here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...
But again not clear what I would be doing in the register job here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

Which leads me to a pretty strong question:
Where is the source of truth here?  And what is the best way to expose that?

Next, do we even know the size yet in the register job?
Let me know what your guidance is here :)  Perhaps we should VC?

[michaeln, 2014-10-29 19:51:03]

> Next, I looked at ServiceWorkerRegistration, as that's a constructor arg.  It
> again would be easy to pipe that in in storage here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

^^^ Bingo
 
> But again not clear what I would be doing in the register job here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

It's zero at this point, nothing is stored on disk.

> Where is the source of truth here?  

The value written to the SWDatabase and reflected in the
SWDatabase::RegistrationData struct.

> And what is the best way to expose that?

have a size_on_disk_ data member + getter in ServiceWorkerRegistration class
set the field based on the resources_total_size_bytes from
SWDB::RegistrationData 
update the field when StoreRegistration completes

have a size_on_disk data member in the ServiceWorkerRegistrationInfo struct
when cons'ing up ServiceWorkerRegistrationInfo, set the value, drawing from
either the class or the struct depending on whether the
ServiceWorkerRegistration method is returning it or it's being constructed based
on RegistrationData for a GetAllRegistrations call.

> Let me know what your guidance is here :)  Perhaps we should VC?

wdyt?

[dmurph, 2014-10-29 22:18:25]

Alright, so what do you think about this?  I'm not super happy that I have to
propagate the size everywhere, I feel like it's replicated in too many places
now.  This is mostly due to needing the size in the ServiceWorkerVersion's
GetInfo() method.  Do you have any ideas on ways to improve this?

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_database.cc:542: RegistrationData
old_registration_on_stack;
On 2014/10/28 22:00:40, michaeln wrote:
> is this still needed given DCHECK(old_registration) up top?

Done.

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_database.cc:657:
deleted_version->version_id = registration.version_id;
On 2014/10/28 22:00:40, michaeln wrote:
> should we assign the entire struct here instead of just id, the recipient
won't
> get the size otherwise.

Done.

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1347:
deleted_version->version_id = kInvalidServiceWorkerVersionId;
On 2014/10/28 22:00:40, michaeln wrote:
> this assignement isn't needed since its constructed with versions_id having
this
> value

Done.

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1359:
base::Owned(deleted_version.release()),
On 2014/10/28 22:00:40, michaeln wrote:
> when working with scoped_ptrs<> we usually used base::Passed(&scopedptr) to
> transfer ownership, and the sig of the bound function accepts a scoped_ptr<T>
as
> input

Acknowledged.

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

https://codereview.chromium.org/672813002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_storage.h:280:
ServiceWorkerDatabase::RegistrationData* deleted_version,
On 2014/10/28 22:00:40, michaeln wrote:
> or maybe use a const T& for this arg to simplify the ownership?

Done.

[michaeln, 2014-10-30 01:32:27]

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

https://codereview.chromium.org/672813002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_context_wrapper.cc:38:
GetUsageIfValid(registration_info.installing_version);
Like we chatted about, going with a model where there is one version stored for
a given registration at any given moment simplifies things. I'd vote to put the
size data member in the ServiceWorkerRegistrationInfo struct and have that value
reflect the space used by that one version that is stored. The size on disk
value we'll be reporting only ever changes upon completion of the
StoreRegistration() method.

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

https://codereview.chromium.org/672813002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:473:
registration->set_resources_total_size_bytes(resources_total_size_bytes);
I think this should happen in DidStoreRegistration next to where
NotifyStorageModified is called. Otherwise i think there's can be an
inconsistency in the numbers given to the QM (espcially if StoreRegistration
fails for some reason).

[dmurph, 2014-10-30 22:06:16]

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

https://codereview.chromium.org/672813002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_context_wrapper.cc:38:
GetUsageIfValid(registration_info.installing_version);
On 2014/10/30 01:32:27, michaeln wrote:
> Like we chatted about, going with a model where there is one version stored
for
> a given registration at any given moment simplifies things. I'd vote to put
the
> size data member in the ServiceWorkerRegistrationInfo struct and have that
value
> reflect the space used by that one version that is stored. The size on disk
> value we'll be reporting only ever changes upon completion of the
> StoreRegistration() method.

Done.

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

https://codereview.chromium.org/672813002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:473:
registration->set_resources_total_size_bytes(resources_total_size_bytes);
On 2014/10/30 01:32:27, michaeln wrote:
> I think this should happen in DidStoreRegistration next to where
> NotifyStorageModified is called. Otherwise i think there's can be an
> inconsistency in the numbers given to the QM (espcially if StoreRegistration
> fails for some reason).

Done.

[michaeln, 2014-10-30 22:43:37]

lgtm

we still have to do something about delete and start over, maybe put a todo
somewhere about that

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_info.h (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_info.h:47: size_t
active_version_total_size_bytes);
maybe call this 'stored_version_size' (or even just stored_size for greater
brevity) since it can be either the active or waiting version depending on where
things are in the update algo

also i think int64_t might be a better type since its the sum of multiple file
sizes

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:984: // Doing all size
updates
comment may not be needed

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1340:
original_task_runner->PostTask(FROM_HERE,
might as well make the line wrapping style consistent for the three calls to
PostTask in this method :)

https://codereview.chromium.org/672813002/diff/80001/content/public/browser/s...
File content/public/browser/service_worker_usage_info.h (right):

https://codereview.chromium.org/672813002/diff/80001/content/public/browser/s...
content/public/browser/service_worker_usage_info.h:31: int64 total_size_bytes;
per recent emails, int64_t

[michaeln, 2014-10-30 23:32:24]

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_quota_client.cc (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_quota_client.cc:34: void
AccumulateUsage(const QuotaClient::GetUsageCallback& callback,
oh, the name of this method doesn't fit since its not summing things up

[dmurph, 2014-10-31 19:10:50]

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_info.h (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_info.h:47: size_t
active_version_total_size_bytes);
On 2014/10/30 22:43:37, michaeln wrote:
> maybe call this 'stored_version_size' (or even just stored_size for greater
> brevity) since it can be either the active or waiting version depending on
where
> things are in the update algo
> 
> also i think int64_t might be a better type since its the sum of multiple file
> sizes

Done.

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_quota_client.cc (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_quota_client.cc:34: void
AccumulateUsage(const QuotaClient::GetUsageCallback& callback,
On 2014/10/30 23:32:24, michaeln wrote:
> oh, the name of this method doesn't fit since its not summing things up

Done.

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:984: // Doing all size
updates
On 2014/10/30 22:43:37, michaeln wrote:
> comment may not be needed

Done.

https://codereview.chromium.org/672813002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_storage.cc:1340:
original_task_runner->PostTask(FROM_HERE,
On 2014/10/30 22:43:37, michaeln wrote:
> might as well make the line wrapping style consistent for the three calls to
> PostTask in this method :)

It no longer fits

https://codereview.chromium.org/672813002/diff/80001/content/public/browser/s...
File content/public/browser/service_worker_usage_info.h (right):

https://codereview.chromium.org/672813002/diff/80001/content/public/browser/s...
content/public/browser/service_worker_usage_info.h:31: int64 total_size_bytes;
On 2014/10/30 22:43:37, michaeln wrote:
> per recent emails, int64_t

Done.

[dmurph, 2014-10-31 19:11:31]

The CQ bit was checked by dmurph@chromium.org

[commit-bot: I haz the power, 2014-10-31 19:13:16]

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

[commit-bot: I haz the power, 2014-10-31 19:19:01]

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

[commit-bot: I haz the power, 2014-10-31 19:19:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dmurph, 2014-10-31 20:16:24]

dmurph@chromium.org changed reviewers:
+ jam@chromium.org

[dmurph, 2014-10-31 20:16:25]

+jam for owners of "content/public/browser/service_worker_usage_info"

[dmurph, 2014-10-31 20:18:08]

Also, I'm investigating why the cache size isn't incremented correctly.

[dmurph, 2014-10-31 21:55:38]

Done, and added check in test.

[michaeln, 2014-10-31 23:10:58]

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:386:
live_registration = NULL;
After this point, the size value will have to be read from the db, so this could
be a good place to verify info structs return by cntx.GetAllRegistrations() and
reg.GetInfo() have the right size.

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:524:
EXPECT_EQ(kWorkerSize, info.stored_version_size_bytes);
This test hits the case where the ServiceWorkerRegistration instance is sitting
memory, but this doesn't cover the case where the data is read from the db.

[dmurph, 2014-10-31 23:41:02]

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:386:
live_registration = NULL;
On 2014/10/31 23:10:58, michaeln wrote:
> After this point, the size value will have to be read from the db, so this
could
> be a good place to verify info structs return by cntx.GetAllRegistrations()
and
> reg.GetInfo() have the right size.

Done.

https://codereview.chromium.org/672813002/diff/120001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:524:
EXPECT_EQ(kWorkerSize, info.stored_version_size_bytes);
On 2014/10/31 23:10:58, michaeln wrote:
> This test hits the case where the ServiceWorkerRegistration instance is
sitting
> memory, but this doesn't cover the case where the data is read from the db.

Done.

[jam, 2014-11-03 21:49:23]

On 2014/10/31 20:16:25, dmurph wrote:
> +jam for owners of "content/public/browser/service_worker_usage_info"

lgtm

[dmurph, 2014-11-03 21:49:54]

The CQ bit was checked by dmurph@chromium.org

[commit-bot: I haz the power, 2014-11-03 21:52:20]

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

[commit-bot: I haz the power, 2014-11-03 22:39:28]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-11-03 22:40:24]

Patchset 8 (id:??) landed as
https://crrev.com/66f514ae41532124cd90a4d6c51f594de83853e4
Cr-Commit-Position: refs/heads/master@{#302503}