[Media Galleries] Lazily initialize the storage monitor.

chrome/browser/storage_monitor/storage_monitor.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_STORAGE_MONITOR_STORAGE_MONITOR_H_
 #define CHROME_BROWSER_STORAGE_MONITOR_STORAGE_MONITOR_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 21 matching lines @@
 class MediaFileSystemRegistryTest;
 class RemovableStorageObserver;
 class TransientDeviceIds;
 
 // Base class for platform-specific instances watching for removable storage
 // attachments/detachments.
 // Lifecycle contracts: This class is created by ChromeBrowserMain
 // implementations before the profile is initialized, so listeners can be
 // created during profile construction. The platform-specific initialization,
 // which can lead to calling registered listeners with notifications of
-// attached volumes, will happen after profile construction.
+// attached volumes, can be done lazily at first use through the async

[vandebo (ex-Chrome), 2013/05/31 18:05:03]

nit: can be done => are done

+// |Initialize()| method. That must be done before any of the registered
+// listeners will receive updates or calls to other API methods return
+// meaningful results.
+// For platform implementations, the intention is that notifications be sent

[vandebo (ex-Chrome), 2013/05/31 18:05:03]

This implies that we try to suppress duplicates, but may not get all of them. 
But we don't try.  Since the current consumers aren't in this situation, just
update the comment to say that duplicates would be sent.

[Greg Billock, 2013/06/01 01:48:46]

OK, this was a bit premature, as we haven't done that yet. We have to remember
to update the comment on that change, although I think the gist is going to be
the same -- "call GetAttachedStorage and register a listener to get everything,
but you may get duplicates".

+// on storages attached/detached after initialization is complete. However,
+// not all platforms can guarantee this process is race-free, so clients
+// should not depend on notifications and an initial |GetAttachedStorage()|
+// call having no duplicates.
 class StorageMonitor {
  public:
   // This interface is provided to generators of storage notifications.
   class Receiver {
    public:
     virtual ~Receiver();
 
     virtual void ProcessAttach(const StorageInfo& info) = 0;
     virtual void ProcessDetach(const std::string& id) = 0;
     virtual void MarkInitialized() = 0;
@@ skipping 93 matching lines @@
   typedef std::map<std::string, StorageInfo> RemovableStorageMap;
 
   void ProcessAttach(const StorageInfo& storage);
   void ProcessDetach(const std::string& id);
 
   scoped_ptr<Receiver> receiver_;
 
   scoped_refptr<ObserverListThreadSafe<RemovableStorageObserver> >
       observer_list_;
 
+  bool initializing_;

[vandebo (ex-Chrome), 2013/05/31 18:05:03]

nit: instead of two bools, an enum might be better.  Feel free to do in a follow
up CL.

[Greg Billock, 2013/06/01 01:48:46]

I think two bools are easier to understand -- one governs internally-observable
state, the other is the externally-observable state.

[vandebo (ex-Chrome), 2013/06/01 15:47:24]

Together they specify the state of the object.  If you want a subset of that
state to be publicly visible, that's easy enough to do.  Further, some
combinations don't make sense, which argues for an enum.  i.e. initialized_ ==
true && initializing_ == true

[Greg Billock, 2013/06/03 15:30:57]

True, but still I think this is more clear.

   bool initialized_;
   std::vector<base::Closure> on_initialize_callbacks_;
 
   // For manipulating removable_storage_map_ structure.
   mutable base::Lock storage_lock_;
 
   // Map of all the attached removable storage devices.
   RemovableStorageMap storage_map_;
 
   scoped_ptr<TransientDeviceIds> transient_device_ids_;
 };
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_STORAGE_MONITOR_STORAGE_MONITOR_H_