[SystemInfo API] Implement systemInfo.storage.onAvailableCapacityChanged event

This patch is to implement. storage.onAvailableCapacityChanged event.
Current implementation adopts polling-based method to check the 
free space change periodically at one second time interval. 

Once a listener is attached to the onAvailableCapacityChanged event,
all storages are being watched, and the event will be emitted as soon
as the free space changes. The watching will be stopped when the
last event listener is removed.

BUG=163862
TEST=browser_tests --gtest_filter=SystemInfoStorageApiTest.Storage

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

[Hongbo Min, 2013-01-28 05:54:56]

benwells@, since https://codereview.chromium.org/11419279/ is a big patch to
implement a full-version onChanged event, it might not be convenient for code
review, so I split it into several parts. This patch is the first step towards
the final goal.

Please have a review. Thanks.

[Hongbo Min, 2013-01-28 05:59:44]

Next step is to implement a fine-grained onAvailableCapacityChanged event, e.g.
allow developer to configure the watching interval and size threshold.

[benwells, 2013-01-31 04:04:57]

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/api/system_info_storage/storage_info_observer.h
(right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/api/system_info_storage/storage_info_observer.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
Nit: the (c) should be removed, and 2012->2013.

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/system_info_event_router.cc (right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/system_info_event_router.cc:53:
watching_event_set_.insert(event_name);
Nit: add DCHECK that are on the UI thread.

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/system_info_event_router.cc:107: const std::string&
id, double new_value, double old_value) {
I don't understand why the threading stuff changed here. Isn't this still called
from the blocking pool?

[Hongbo Min, 2013-02-01 07:06:49]

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/api/system_info_storage/storage_info_observer.h
(right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/api/system_info_storage/storage_info_observer.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/01/31 04:04:57, benwells wrote:
> Nit: the (c) should be removed, and 2012->2013.

Done.

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/system_info_event_router.cc (right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/system_info_event_router.cc:53:
watching_event_set_.insert(event_name);
On 2013/01/31 04:04:57, benwells wrote:
> Nit: add DCHECK that are on the UI thread.

Done for both AddEventListener and RemoveEventListener.

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/system_info_event_router.cc:107: const std::string&
id, double new_value, double old_value) {
On 2013/01/31 04:04:57, benwells wrote:
> I don't understand why the threading stuff changed here. Isn't this still
called
> from the blocking pool?

Since the observers of StorageInfoProvider are hold in a ObserverListThreadSafe
list, which means that the observer needs to only care about on which thread it
is added, so no need to check the thread type to ensure it running on UI thread.

[benwells, 2013-02-04 04:21:41]

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/api/system_info_storage/storage_info_observer.h
(right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/api/system_info_storage/storage_info_observer.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/02/01 07:06:50, Hongbo Min wrote:
> On 2013/01/31 04:04:57, benwells wrote:
> > Nit: the (c) should be removed, and 2012->2013.
> 
> Done.

Still-a-nit: the (c) should be removed for new copyright headers

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/system_info_event_router.cc (right):

https://codereview.chromium.org/12084017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/system_info_event_router.cc:107: const std::string&
id, double new_value, double old_value) {
On 2013/02/01 07:06:50, Hongbo Min wrote:
> On 2013/01/31 04:04:57, benwells wrote:
> > I don't understand why the threading stuff changed here. Isn't this still
> called
> > from the blocking pool?
> 
> Since the observers of StorageInfoProvider are hold in a
ObserverListThreadSafe
> list, which means that the observer needs to only care about on which thread
it
> is added, so no need to check the thread type to ensure it running on UI
thread.
> 

Oh wow, I didn't realize that class was so smart. Neat!

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
File chrome/browser/extensions/system_info_event_router.cc (right):

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:69: if (!success) return;
nit: return on next line.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:71: for
(StorageInfo::const_iterator it = info.begin(); it != info.end(); ++it) {
Is the list of StorageInfos constant? i.e. can new StorageInfos appear in this
iterator dynamically?

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:72:
StorageInfoProvider::Get()->StartWatching((*it)->id);
nit: indenting << 2

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:79: if (!success) return;
Nit: return on next line.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:82:
StorageInfoProvider::Get()->StopWatching((*it)->id);
nit: indenting << 2

[Hongbo Min, 2013-02-04 04:53:28]

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
File chrome/browser/extensions/system_info_event_router.cc (right):

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:69: if (!success) return;
On 2013/02/04 04:21:41, benwells wrote:
> nit: return on next line.

Done.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:71: for
(StorageInfo::const_iterator it = info.begin(); it != info.end(); ++it) {
On 2013/02/04 04:21:41, benwells wrote:
> Is the list of StorageInfos constant? i.e. can new StorageInfos appear in this
> iterator dynamically?

Yes, no new storage info will appear dynamically.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:72:
StorageInfoProvider::Get()->StartWatching((*it)->id);
On 2013/02/04 04:21:41, benwells wrote:
> nit: indenting << 2

Done.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:79: if (!success) return;
On 2013/02/04 04:21:41, benwells wrote:
> Nit: return on next line.

Done.

https://codereview.chromium.org/12084017/diff/32003/chrome/browser/extensions...
chrome/browser/extensions/system_info_event_router.cc:82:
StorageInfoProvider::Get()->StopWatching((*it)->id);
On 2013/02/04 04:21:41, benwells wrote:
> nit: indenting << 2

Done.

[benwells, 2013-02-04 04:55:04]

lgtm

[Hongbo Min, 2013-02-04 06:18:48]

sky@, need your lgtm for gyp changes.

[benwells, 2013-02-04 06:23:18]

On 2013/02/04 06:18:48, Hongbo Min wrote:
> sky@, need your lgtm for gyp changes.

You can TBR sky for gyp changes.

[sky, 2013-02-04 15:45:35]

LGTM

https://codereview.chromium.org/12084017/diff/37007/chrome/chrome_browser_ext...
File chrome/chrome_browser_extensions.gypi (right):

https://codereview.chromium.org/12084017/diff/37007/chrome/chrome_browser_ext...
chrome/chrome_browser_extensions.gypi:389:
'browser/extensions/api/system_info_storage/storage_info_observer.h',
.cc first

[Hongbo Min, 2013-02-05 02:17:26]

https://codereview.chromium.org/12084017/diff/37007/chrome/chrome_browser_ext...
File chrome/chrome_browser_extensions.gypi (right):

https://codereview.chromium.org/12084017/diff/37007/chrome/chrome_browser_ext...
chrome/chrome_browser_extensions.gypi:389:
'browser/extensions/api/system_info_storage/storage_info_observer.h',
On 2013/02/04 15:45:35, sky wrote:
> .cc first

It is 'storage_info_observer'...not storage_info_provider.

[commit-bot: I haz the power, 2013-02-05 03:33:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hongbo.min@intel.com/12084017/37007

[commit-bot: I haz the power, 2013-02-05 07:41:29]

Change committed as 180626