Retrieve per host storage usage from QuotaManager.

Retrieve per host storage usage from QuotaManager.

R=kinuko@chromium.org
BUG=none
TEST=QuotaManagerTest.GetUsage

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=103921

[mnaganov (inactive), 2011-09-29 17:16:26]

Hi Kinuko,

This is what I had in mind talking about retrieving current storage usage from
QuotaManager. I had some draft code, so I decided to upload it instead of
creating a bug. If you'll be OK with the approach, I will add tests.

[kinuko, 2011-10-03 07:49:20]

Sorry for my late iteration; basically it looks good to me.  I have a few
comments & questions

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.cc
File webkit/quota/quota_manager.cc (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.cc#n...
webkit/quota/quota_manager.cc:342: AddEntries(type,
manager()->persistent_usage_tracker_.get());
You can use manager()->GetUsageTracker(type) here

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.h
File webkit/quota/quota_manager.h (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.h#ne...
webkit/quota/quota_manager.h:84: int usage;
int64?

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_types.h
File webkit/quota/quota_types.h (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_types.h#newc...
webkit/quota/quota_types.h:59: typedef Callback1<const UsageInfoEntries&>::Type
GetUsageCallback;
the name GetUsageCallback is used in QuotaClients and might be confusing; how
about naming this GetUsageInfoCallback?

http://codereview.chromium.org/8079004/diff/1/webkit/quota/usage_tracker.cc
File webkit/quota/usage_tracker.cc (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/usage_tracker.cc#n...
webkit/quota/usage_tracker.cc:281:
iter->second->GetCachedHostsUsage(host_usage);
Don't we want to aggregate the usage across different clients?

[mnaganov (inactive), 2011-10-04 03:00:30]

Thanks! I've addressed your comments and added a test.

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.cc
File webkit/quota/quota_manager.cc (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.cc#n...
webkit/quota/quota_manager.cc:342: AddEntries(type,
manager()->persistent_usage_tracker_.get());
On 2011/10/03 07:49:20, kinuko wrote:
> You can use manager()->GetUsageTracker(type) here

Neat! Thanks.

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.h
File webkit/quota/quota_manager.h (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_manager.h#ne...
webkit/quota/quota_manager.h:84: int usage;
On 2011/10/03 07:49:20, kinuko wrote:
> int64?

Done. Thanks!

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_types.h
File webkit/quota/quota_types.h (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/quota_types.h#newc...
webkit/quota/quota_types.h:59: typedef Callback1<const UsageInfoEntries&>::Type
GetUsageCallback;
On 2011/10/03 07:49:20, kinuko wrote:
> the name GetUsageCallback is used in QuotaClients and might be confusing; how
> about naming this GetUsageInfoCallback?

Done.

http://codereview.chromium.org/8079004/diff/1/webkit/quota/usage_tracker.cc
File webkit/quota/usage_tracker.cc (right):

http://codereview.chromium.org/8079004/diff/1/webkit/quota/usage_tracker.cc#n...
webkit/quota/usage_tracker.cc:281:
iter->second->GetCachedHostsUsage(host_usage);
On 2011/10/03 07:49:20, kinuko wrote:
> Don't we want to aggregate the usage across different clients?

Right. Fixed in ClientUsageTracker::GetCachedHostsUsage.

[kinuko, 2011-10-04 07:43:03]

LGTM, except one more naming nit!

http://codereview.chromium.org/8079004/diff/7001/webkit/quota/quota_manager.cc
File webkit/quota/quota_manager.cc (right):

http://codereview.chromium.org/8079004/diff/7001/webkit/quota/quota_manager.c...
webkit/quota/quota_manager.cc:293: class QuotaManager::GetUsageTask : public
QuotaTask {
A subtle issue... GetUsageTask may sound too generic while this file has tons of
similar looking tasks; now that we renamed the callback to GetUsageInfoCallback
probably we can also rename GetUsageTask to GetUsageInfoTask (or
GatherUsageInfoTask maybe)?

Similarly probably GetUsage() could be renamed to GetUsageInfo or
GatherUsageInfo.

[mnaganov (inactive), 2011-10-04 16:38:41]

Thank you very much, Kinuko!

http://codereview.chromium.org/8079004/diff/7001/webkit/quota/quota_manager.cc
File webkit/quota/quota_manager.cc (right):

http://codereview.chromium.org/8079004/diff/7001/webkit/quota/quota_manager.c...
webkit/quota/quota_manager.cc:293: class QuotaManager::GetUsageTask : public
QuotaTask {
On 2011/10/04 07:43:04, kinuko wrote:
> A subtle issue... GetUsageTask may sound too generic while this file has tons
of
> similar looking tasks; now that we renamed the callback to
GetUsageInfoCallback
> probably we can also rename GetUsageTask to GetUsageInfoTask (or
> GatherUsageInfoTask maybe)?
> 
> Similarly probably GetUsage() could be renamed to GetUsageInfo or
> GatherUsageInfo.

OK, let's rename to GetUsageInfo. Initially I was avoiding this name because the
"Info" suffix doesn't actually add any meaning, but I you have noticed, this
help to distinguish it from other similarly looking names.

[tzik, 2013-05-24 12:03:29]

https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker.cc
File webkit/quota/usage_tracker.cc (right):

https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker...
webkit/quota/usage_tracker.cc:412: GetCachedHostUsage(host_iter->first);
I'm cleaning up around UsageTracker now.
Was there any reason to avoid (*host_usage)[host_itr->first]?

If no, I'll replace this.

[mnaganov (inactive), 2013-05-24 12:06:58]

https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker.cc
File webkit/quota/usage_tracker.cc (right):

https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker...
webkit/quota/usage_tracker.cc:412: GetCachedHostUsage(host_iter->first);
On 2013/05/24 12:03:29, tzik wrote:
> I'm cleaning up around UsageTracker now.
> Was there any reason to avoid (*host_usage)[host_itr->first]?
> 
> If no, I'll replace this.

I just didn't think about this possibility then. Please replace.

[tzik, 2013-05-24 14:29:15]

On 2013/05/24 12:06:58, Mikhail Naganov (Chromium) wrote:
>
https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker.cc
> File webkit/quota/usage_tracker.cc (right):
> 
>
https://codereview.chromium.org/8079004/diff/11002/webkit/quota/usage_tracker...
> webkit/quota/usage_tracker.cc:412: GetCachedHostUsage(host_iter->first);
> On 2013/05/24 12:03:29, tzik wrote:
> > I'm cleaning up around UsageTracker now.
> > Was there any reason to avoid (*host_usage)[host_itr->first]?
> > 
> > If no, I'll replace this.
> 
> I just didn't think about this possibility then. Please replace.

OK, so I will.