Provide monitoring of usage for a storage type and origin

Provide monitoring of usage for a storage type and origin

Introduces a quota::StorageObserver interface for observers that
wish to monitor storage events such as changes in quota and usage.
Observers register themselves with quota::QuotaManager. This patch
only implements tracking of changes in usage.

BUG=347801
TEST=content_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261358

[tmdiep, 2014-03-30 23:19:12]

This is the implementation of the quota monitoring interface proposed in:
https://codereview.chromium.org/196573024

I have only implemented the tracking for changes in usage, which is what I need
for extension system. I have left tracking of quota changes unimplemented, hope
that's ok.

tzik, nhiroki: Thanks in advance for reviewing this patch.

[tzik, 2014-03-31 06:54:16]

Looking now.

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
File webkit/browser/quota/storage_monitor.cc (right):

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_monitor.cc:43: StorageObserverStateMap::iterator it
= observers_.find(observer);
just observers_.erase(observer); works.

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
File webkit/browser/quota/storage_monitor.h (right):

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_monitor.h:81: HostStorageObservers(QuotaManager*
quota_manager);
explicit?

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_monitor.h:103:
base::WeakPtrFactory<HostStorageObservers> weak_factory_;
please make this the last member of the class.

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
File webkit/browser/quota/storage_observer.h (right):

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_observer.h:39: int rate;
Could you change this to TimeDelta?

[tzik, 2014-03-31 07:45:48]

Looks good overall.

Does it make thing easier to separate StorageObserverList and
HostStorageObserver apart?

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
File webkit/browser/quota/storage_monitor.cc (right):

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_monitor.cc:129: DCHECK(observers_.find(observer) !=
observers_.end());
I think
DCHECK(ContainsKey(observers_, observer));
is more readable.

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
File webkit/browser/quota/storage_monitor.h (right):

https://codereview.chromium.org/218793002/diff/1/webkit/browser/quota/storage...
webkit/browser/quota/storage_monitor.h:58: base::Time last_notification_time;
I think base::TimeTicks is better for this.

[tmdiep, 2014-04-01 01:04:52]

Thanks for the review. All your suggestions have been addressed in patch set 2.

> Does it make thing easier to separate StorageObserverList and
> HostStorageObserver apart?

The StorageObserver::Filter was originally more fine-grained (supported
individual quota clients and all clients) and HostStorageObserver used
composition instead of inheritance. After I removed the quota clients from the
filter, I decided not to merge HostStorageObserver and StorageObserverList in
case we add the fine-grained filtering support later.

While writing this, I decided to revert back to using composition as it felt
cleaner. This refactoring is in patch set 3.

[tmdiep, 2014-04-01 01:41:37]

+kinuko for webkit/storage_browser.gyp

[tmdiep, 2014-04-01 01:47:59]

Might help if I add kinuko to reviewers.

+kinuko for webkit/storage_browser.gyp

[tzik, 2014-04-02 05:44:10]

Thanks! LGTM.

[tmdiep, 2014-04-02 22:52:21]

Ping kinuko for webkit/storage_browser.gyp. I'll be going on leave for 1 month
in less than a week's time, so just wanted to get this patch and others that
depend on it committed. Thanks!

[nhiroki, 2014-04-03 01:30:40]

Sorry for my delayed response. LGTM2.

[kinuko, 2014-04-03 03:59:52]

lgtm for webkit/*.gypi, sorry this one had slipped under my radar.

[tmdiep, 2014-04-03 05:34:14]

No problem, thanks all :)

[tmdiep, 2014-04-03 06:15:40]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-04-03 06:16:07]

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

[commit-bot: I haz the power, 2014-04-03 10:12:49]

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

[commit-bot: I haz the power, 2014-04-03 10:12:49]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[tmdiep, 2014-04-03 10:15:27]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-04-03 10:17:04]

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

[commit-bot: I haz the power, 2014-04-03 12:02:24]

Change committed as 261358