Allow the consumer of storage monitor to register callback when the initialization is completed.

Allow the consumer of storage monitor to register callback when the initialization is completed.

It is intended to allow the consumer of storage monitor to decide
calling GetAllAvailableStorages or GetAttachedStorage to fetch the
correct results.

BUG=None

[Hongbo Min, 2013-05-29 06:48:37]

Please have a review. Thanks.

[Hongbo Min, 2013-05-29 06:53:00]

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor.cc (right):

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor.cc:64: void
StorageMonitor::Initialize(base::Closure callback) {
What is the Initialize used for?

If the StorageMonitor is changing to be created only when needed, could we use
RegisterInitCompletionCallback to register the callbacks wanted and then call
Init to query storage metadata?

[wrong vandebo, 2013-05-29 15:08:34]

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor.h (right):

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor.h:71: void
RegisterInitCompletedCallback(base::Closure callback);
I don't understand why you would want this method.  If you want to register an
observer, then you presumably want that observer to be called.  The observer
won't be called until initialization is complete.

[Hongbo Min, 2013-05-29 15:15:39]

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor.h (right):

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor.h:71: void
RegisterInitCompletedCallback(base::Closure callback);
On 2013/05/29 15:08:35, vandebo1 wrote:
> I don't understand why you would want this method.  If you want to register an
> observer, then you presumably want that observer to be called.  The observer
> won't be called until initialization is complete.

Since the RemovableStorageObserver only gets notified when a storage is
attached/detached, the thing I want to do is, I need a way to tell me when it is
safe to call GetAttachedStorage/GetAllAvailableStorages to return the correct
results. 

Without this method, the consumer of StorageMonitor doesn't know the proper
timing of calling GetAttachedStorage at all.

Also, it might be wrong to invoke Initialize with a callback since the Init
method will be called by Initialize again.

[vandebo (ex-Chrome), 2013-05-29 15:20:27]

On 2013/05/29 15:15:39, Hongbo Min wrote:
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> File chrome/browser/storage_monitor/storage_monitor.h (right):
> 
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> chrome/browser/storage_monitor/storage_monitor.h:71: void
> RegisterInitCompletedCallback(base::Closure callback);
> On 2013/05/29 15:08:35, vandebo1 wrote:
> > I don't understand why you would want this method.  If you want to register
an
> > observer, then you presumably want that observer to be called.  The observer
> > won't be called until initialization is complete.
> 
> Since the RemovableStorageObserver only gets notified when a storage is
> attached/detached, the thing I want to do is, I need a way to tell me when it
is
> safe to call GetAttachedStorage/GetAllAvailableStorages to return the correct
> results. 
> 
> Without this method, the consumer of StorageMonitor doesn't know the proper
> timing of calling GetAttachedStorage at all.

If you want to call GetAttachedStorage, you should call Init (or be in a code
path where you can be sure it was already called) and call GetSttachedStorage
after the callback.  Otherwise, you might be waiting forever.
 
> Also, it might be wrong to invoke Initialize with a callback since the Init
> method will be called by Initialize again.

It is safe to call Initialize() multiple times.

[Hongbo Min, 2013-05-29 15:25:55]

On 2013/05/29 15:20:27, vandebo wrote:
> On 2013/05/29 15:15:39, Hongbo Min wrote:
> >
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> > File chrome/browser/storage_monitor/storage_monitor.h (right):
> > 
> >
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> > chrome/browser/storage_monitor/storage_monitor.h:71: void
> > RegisterInitCompletedCallback(base::Closure callback);
> > On 2013/05/29 15:08:35, vandebo1 wrote:
> > > I don't understand why you would want this method.  If you want to
register
> an
> > > observer, then you presumably want that observer to be called.  The
observer
> > > won't be called until initialization is complete.
> > 
> > Since the RemovableStorageObserver only gets notified when a storage is
> > attached/detached, the thing I want to do is, I need a way to tell me when
it
> is
> > safe to call GetAttachedStorage/GetAllAvailableStorages to return the
correct
> > results. 
> > 
> > Without this method, the consumer of StorageMonitor doesn't know the proper
> > timing of calling GetAttachedStorage at all.
> 
> If you want to call GetAttachedStorage, you should call Init (or be in a code
> path where you can be sure it was already called) and call GetSttachedStorage
> after the callback.  Otherwise, you might be waiting forever.
>  
> > Also, it might be wrong to invoke Initialize with a callback since the Init
> > method will be called by Initialize again.
> 
> It is safe to call Initialize() multiple times.

ok...I am mislead by its name. shall we have a better name instead?

[vandebo (ex-Chrome), 2013-05-29 15:42:44]

On 2013/05/29 15:25:55, Hongbo Min wrote:
> On 2013/05/29 15:20:27, vandebo wrote:
> > On 2013/05/29 15:15:39, Hongbo Min wrote:
> > >
> >
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> > > File chrome/browser/storage_monitor/storage_monitor.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
> > > chrome/browser/storage_monitor/storage_monitor.h:71: void
> > > RegisterInitCompletedCallback(base::Closure callback);
> > > On 2013/05/29 15:08:35, vandebo1 wrote:
> > > > I don't understand why you would want this method.  If you want to
> register
> > an
> > > > observer, then you presumably want that observer to be called.  The
> observer
> > > > won't be called until initialization is complete.
> > > 
> > > Since the RemovableStorageObserver only gets notified when a storage is
> > > attached/detached, the thing I want to do is, I need a way to tell me when
> it
> > is
> > > safe to call GetAttachedStorage/GetAllAvailableStorages to return the
> correct
> > > results. 
> > > 
> > > Without this method, the consumer of StorageMonitor doesn't know the
proper
> > > timing of calling GetAttachedStorage at all.
> > 
> > If you want to call GetAttachedStorage, you should call Init (or be in a
code
> > path where you can be sure it was already called) and call
GetSttachedStorage
> > after the callback.  Otherwise, you might be waiting forever.
> >  
> > > Also, it might be wrong to invoke Initialize with a callback since the
Init
> > > method will be called by Initialize again.
> > 
> > It is safe to call Initialize() multiple times.
> 
> ok...I am mislead by its name. shall we have a better name instead?

I guess EnsureInitialized() would be a more descriptive name.  If you make that
change, can you also add a DCHECK that it is only called from one (UI?) thread?

[Greg Billock, 2013-05-29 17:46:33]

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
File chrome/browser/storage_monitor/storage_monitor.cc (right):

https://codereview.chromium.org/16187002/diff/3001/chrome/browser/storage_mon...
chrome/browser/storage_monitor/storage_monitor.cc:64: void
StorageMonitor::Initialize(base::Closure callback) {
On 2013/05/29 06:53:00, Hongbo Min wrote:
> What is the Initialize used for?
> 
> If the StorageMonitor is changing to be created only when needed, could we use
> RegisterInitCompletionCallback to register the callbacks wanted and then call
> Init to query storage metadata? 

If your code needs an initialized storage monitor, you should just call
Initialize() yourself.

Meaning, when the monitor is lazily initialized, you must initialize it before
it'll start working. I think Steve suggested "EnsureInitialized" might be a
better name, which is probably true.