Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Regularize ownerships and lifecycle for storage monitor platform clas…

…ses.

This change regularizes the construction lifecycle of the StorageMonitor implementations so that they always exist for the construction of the Profile, but don't issue any notifications until after that construction is complete. This allows profile-based listeners to be sure they're synchronized with the monitor.

It also regularizes ownership of all storage-monitor-related objects. The base platform-specific object is owned by the ChromeBrowserMain object, and owns all subsidiary platform-specific objects needed to monitor and/or deal with volumes of various types (i.e. MTP).

R=vandebo@chromium.org
BUG=None


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

Review URL: https://chromiumcodereview.appspot.com/12334096

git-svn-id: http://src.chromium.org/svn/trunk/src/chrome/browser@189078 4ff67af0-8c30-449e-8e8b-ad334ec8d88c
  • Loading branch information...
commit adac7682a544ddc94b2769dcc141644bad5f0533 1 parent 3aafb03
gbillock@chromium.org authored
34 chrome_browser_main_linux.cc
@@ -4,11 +4,6 @@
4 4
5 5 #include "chrome/browser/chrome_browser_main_linux.h"
6 6
7   -#include "base/message_loop_proxy.h"
8   -#include "chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.h"
9   -#include "chrome/common/chrome_switches.h"
10   -#include "device/media_transfer_protocol/media_transfer_protocol_manager.h"
11   -
12 7 #if !defined(OS_CHROMEOS)
13 8 #include "chrome/browser/storage_monitor/storage_monitor_linux.h"
14 9 #include "content/public/browser/browser_thread.h"
@@ -21,6 +16,7 @@
21 16 #include "base/linux_util.h"
22 17 #include "base/prefs/pref_service.h"
23 18 #include "chrome/app/breakpad_linux.h"
  19 +#include "chrome/common/chrome_switches.h"
24 20 #include "chrome/common/env_vars.h"
25 21 #include "chrome/common/pref_names.h"
26 22
@@ -108,13 +104,10 @@ bool IsCrashReportingEnabled(const PrefService* local_state) {
108 104
109 105 ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
110 106 const content::MainFunctionParams& parameters)
111   - : ChromeBrowserMainPartsPosix(parameters),
112   - initialized_media_transfer_protocol_manager_(false) {
  107 + : ChromeBrowserMainPartsPosix(parameters) {
113 108 }
114 109
115 110 ChromeBrowserMainPartsLinux::~ChromeBrowserMainPartsLinux() {
116   - if (initialized_media_transfer_protocol_manager_)
117   - device::MediaTransferProtocolManager::Shutdown();
118 111 }
119 112
120 113 void ChromeBrowserMainPartsLinux::PreProfileInit() {
@@ -134,30 +127,15 @@ void ChromeBrowserMainPartsLinux::PreProfileInit() {
134 127 #if !defined(OS_CHROMEOS)
135 128 const base::FilePath kDefaultMtabPath("/etc/mtab");
136 129 storage_monitor_ = new chrome::StorageMonitorLinux(kDefaultMtabPath);
137   - storage_monitor_->Init();
138 130 #endif
139 131
140   - if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
141   - scoped_refptr<base::MessageLoopProxy> loop_proxy;
142   -#if !defined(OS_CHROMEOS)
143   - loop_proxy = content::BrowserThread::GetMessageLoopProxyForThread(
144   - content::BrowserThread::FILE);
145   -#endif
146   - device::MediaTransferProtocolManager::Initialize(loop_proxy);
147   - initialized_media_transfer_protocol_manager_ = true;
148   - }
149   -
150 132 ChromeBrowserMainPartsPosix::PreProfileInit();
151 133 }
152 134
153 135 void ChromeBrowserMainPartsLinux::PostProfileInit() {
154   - // TODO(gbillock): Make this owned by StorageMonitorLinux.
155   - if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
156   - media_transfer_protocol_device_observer_.reset(
157   - new chrome::MediaTransferProtocolDeviceObserverLinux());
158   - media_transfer_protocol_device_observer_->SetNotifications(
159   - chrome::StorageMonitor::GetInstance()->receiver());
160   - }
  136 +#if !defined(OS_CHROMEOS)
  137 + storage_monitor_->Init();
  138 +#endif
161 139
162 140 ChromeBrowserMainPartsPosix::PostProfileInit();
163 141 }
@@ -170,7 +148,5 @@ void ChromeBrowserMainPartsLinux::PostMainMessageLoopRun() {
170 148 storage_monitor_ = NULL;
171 149 #endif
172 150
173   - media_transfer_protocol_device_observer_.reset();
174   -
175 151 ChromeBrowserMainPartsPosix::PostMainMessageLoopRun();
176 152 }
7 chrome_browser_main_linux.h
@@ -17,10 +17,6 @@ class StorageMonitorLinux;
17 17 }
18 18 #endif
19 19
20   -namespace chrome {
21   -class MediaTransferProtocolDeviceObserverLinux;
22   -}
23   -
24 20 class ChromeBrowserMainPartsLinux : public ChromeBrowserMainPartsPosix {
25 21 public:
26 22 explicit ChromeBrowserMainPartsLinux(
@@ -36,9 +32,6 @@ class ChromeBrowserMainPartsLinux : public ChromeBrowserMainPartsPosix {
36 32 #if !defined(OS_CHROMEOS)
37 33 scoped_refptr<chrome::StorageMonitorLinux> storage_monitor_;
38 34 #endif
39   - scoped_ptr<chrome::MediaTransferProtocolDeviceObserverLinux>
40   - media_transfer_protocol_device_observer_;
41   - bool initialized_media_transfer_protocol_manager_;
42 35
43 36 DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsLinux);
44 37 };
4 chrome_browser_main_mac.h
@@ -9,7 +9,6 @@
9 9 #include "base/memory/ref_counted.h"
10 10
11 11 namespace chrome {
12   -class ImageCaptureDeviceManager;
13 12 class StorageMonitorMac;
14 13 }
15 14
@@ -23,6 +22,7 @@ class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {
23 22 virtual void PreEarlyInitialization() OVERRIDE;
24 23 virtual void PreMainMessageLoopStart() OVERRIDE;
25 24 virtual void PreProfileInit() OVERRIDE;
  25 + virtual void PostProfileInit() OVERRIDE;
26 26
27 27 // Perform platform-specific work that needs to be done after the main event
28 28 // loop has ended. The embedder must be sure to call this.
@@ -31,8 +31,6 @@ class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {
31 31 private:
32 32 scoped_refptr<chrome::StorageMonitorMac> storage_monitor_;
33 33
34   - scoped_ptr<chrome::ImageCaptureDeviceManager> image_capture_device_manager_;
35   -
36 34 DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsMac);
37 35 };
38 36
13 chrome_browser_main_mac.mm
@@ -22,7 +22,6 @@
22 22 #include "chrome/browser/mac/keychain_reauthorize.h"
23 23 #import "chrome/browser/mac/keystone_glue.h"
24 24 #include "chrome/browser/metrics/metrics_service.h"
25   -#include "chrome/browser/storage_monitor/image_capture_device_manager.h"
26 25 #include "chrome/browser/storage_monitor/storage_monitor_mac.h"
27 26 #include "chrome/common/chrome_paths.h"
28 27 #include "chrome/common/chrome_switches.h"
@@ -283,16 +282,16 @@ int DoUninstallTasks(bool chrome_still_running) {
283 282
284 283 void ChromeBrowserMainPartsMac::PreProfileInit() {
285 284 storage_monitor_ = new chrome::StorageMonitorMac();
286   - // TODO(gbillock): Make the ImageCapture manager owned by StorageMonitorMac.
287   - if (base::mac::IsOSLionOrLater()) {
288   - image_capture_device_manager_.reset(new chrome::ImageCaptureDeviceManager);
289   - image_capture_device_manager_->SetNotifications(
290   - storage_monitor_->receiver());
291   - }
292 285
293 286 ChromeBrowserMainPartsPosix::PreProfileInit();
294 287 }
295 288
  289 +void ChromeBrowserMainPartsMac::PostProfileInit() {
  290 + storage_monitor_->Init();
  291 +
  292 + ChromeBrowserMainPartsPosix::PostProfileInit();
  293 +}
  294 +
296 295 void ChromeBrowserMainPartsMac::DidEndMainMessageLoop() {
297 296 AppController* appController = [NSApp delegate];
298 297 [appController didEndMainMessageLoop];
4 chromeos/chrome_browser_main_chromeos.cc
@@ -545,6 +545,8 @@ void ChromeBrowserMainPartsChromeos::PreProfileInit() {
545 545 &tracker_);
546 546 #endif
547 547
  548 + storage_monitor_ = new StorageMonitorCros();
  549 +
548 550 // In Aura builds this will initialize ash::Shell.
549 551 ChromeBrowserMainPartsLinux::PreProfileInit();
550 552 }
@@ -620,7 +622,7 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
620 622 }
621 623 chromeos::accessibility::Initialize();
622 624
623   - storage_monitor_ = new StorageMonitorCros();
  625 + storage_monitor_->Init();
624 626
625 627 // Initialize the network portal detector for Chrome OS. The network
626 628 // portal detector starts to listen for notifications from
3  storage_monitor/media_transfer_protocol_device_observer_linux.cc
@@ -127,6 +127,7 @@ MediaTransferProtocolDeviceObserverLinux()
127 127
128 128 device::MediaTransferProtocolManager* mtp_manager =
129 129 device::MediaTransferProtocolManager::GetInstance();
  130 + DCHECK(mtp_manager);
130 131 mtp_manager->AddObserver(this);
131 132 EnumerateStorages();
132 133 }
@@ -137,8 +138,6 @@ MediaTransferProtocolDeviceObserverLinux(
137 138 GetStorageInfoFunc get_storage_info_func)
138 139 : get_storage_info_func_(get_storage_info_func),
139 140 notifications_(NULL) {
140   - // In unit tests, we don't have a media transfer protocol manager.
141   - DCHECK(!device::MediaTransferProtocolManager::GetInstance());
142 141 DCHECK(!g_mtp_device_observer);
143 142 g_mtp_device_observer = this;
144 143 }
5 storage_monitor/storage_monitor.h
@@ -25,6 +25,11 @@ class TransientDeviceIds;
25 25
26 26 // Base class for platform-specific instances watching for removable storage
27 27 // attachments/detachments.
  28 +// Lifecycle contracts: This class is created by ChromeBrowserMain
  29 +// implementations before the profile is initialized, so listeners can be
  30 +// created during profile construction. The platform-specific initialization,
  31 +// which can lead to calling registered listeners with notifications of
  32 +// attached volumes, will happen after profile construction.
28 33 class StorageMonitor {
29 34 public:
30 35 // This interface is provided to generators of storage notifications.
26 storage_monitor/storage_monitor_chromeos.cc
@@ -6,6 +6,7 @@
6 6
7 7 #include "chrome/browser/storage_monitor/storage_monitor_chromeos.h"
8 8
  9 +#include "base/command_line.h"
9 10 #include "base/files/file_path.h"
10 11 #include "base/logging.h"
11 12 #include "base/stl_util.h"
@@ -13,8 +14,11 @@
13 14 #include "base/strings/string_number_conversions.h"
14 15 #include "base/utf_string_conversions.h"
15 16 #include "chrome/browser/storage_monitor/media_storage_util.h"
  17 +#include "chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.h"
16 18 #include "chrome/browser/storage_monitor/removable_device_constants.h"
  19 +#include "chrome/common/chrome_switches.h"
17 20 #include "content/public/browser/browser_thread.h"
  21 +#include "device/media_transfer_protocol/media_transfer_protocol_manager.h"
18 22
19 23 namespace chromeos {
20 24
@@ -104,18 +108,34 @@ using content::BrowserThread;
104 108 using chrome::StorageInfo;
105 109
106 110 StorageMonitorCros::StorageMonitorCros() {
107   - DCHECK(disks::DiskMountManager::GetInstance());
108   - disks::DiskMountManager::GetInstance()->AddObserver(this);
109   - CheckExistingMountPointsOnUIThread();
110 111 }
111 112
112 113 StorageMonitorCros::~StorageMonitorCros() {
  114 + if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
  115 + device::MediaTransferProtocolManager::Shutdown();
  116 + }
  117 +
113 118 disks::DiskMountManager* manager = disks::DiskMountManager::GetInstance();
114 119 if (manager) {
115 120 manager->RemoveObserver(this);
116 121 }
117 122 }
118 123
  124 +void StorageMonitorCros::Init() {
  125 + DCHECK(disks::DiskMountManager::GetInstance());
  126 + disks::DiskMountManager::GetInstance()->AddObserver(this);
  127 + CheckExistingMountPointsOnUIThread();
  128 +
  129 + if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
  130 + scoped_refptr<base::MessageLoopProxy> loop_proxy;
  131 + device::MediaTransferProtocolManager::Initialize(loop_proxy);
  132 +
  133 + media_transfer_protocol_device_observer_.reset(
  134 + new chrome::MediaTransferProtocolDeviceObserverLinux());
  135 + media_transfer_protocol_device_observer_->SetNotifications(receiver());
  136 + }
  137 +}
  138 +
119 139 void StorageMonitorCros::CheckExistingMountPointsOnUIThread() {
120 140 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
121 141 const disks::DiskMountManager::MountPointMap& mount_point_map =
11 storage_monitor/storage_monitor_chromeos.h
@@ -21,6 +21,10 @@
21 21 #include "chrome/browser/storage_monitor/storage_monitor.h"
22 22 #include "chromeos/disks/disk_mount_manager.h"
23 23
  24 +namespace chrome {
  25 +class MediaTransferProtocolDeviceObserverLinux;
  26 +}
  27 +
24 28 namespace chromeos {
25 29
26 30 class StorageMonitorCros
@@ -31,6 +35,10 @@ class StorageMonitorCros
31 35 // Should only be called by browser start up code. Use GetInstance() instead.
32 36 StorageMonitorCros();
33 37
  38 + // Sets up disk listeners and issues notifications for any discovered
  39 + // mount points. Sets up MTP manager and listeners.
  40 + void Init();
  41 +
34 42 virtual void OnDiskEvent(disks::DiskMountManager::DiskEvent event,
35 43 const disks::DiskMountManager::Disk* disk) OVERRIDE;
36 44 virtual void OnDeviceEvent(disks::DiskMountManager::DeviceEvent event,
@@ -86,6 +94,9 @@ class StorageMonitorCros
86 94 // Only accessed on the UI thread.
87 95 MountMap mount_map_;
88 96
  97 + scoped_ptr<chrome::MediaTransferProtocolDeviceObserverLinux>
  98 + media_transfer_protocol_device_observer_;
  99 +
89 100 DISALLOW_COPY_AND_ASSIGN(StorageMonitorCros);
90 101 };
91 102
21 storage_monitor/storage_monitor_linux.cc
@@ -13,6 +13,7 @@
13 13
14 14 #include "base/basictypes.h"
15 15 #include "base/bind.h"
  16 +#include "base/command_line.h"
16 17 #include "base/files/file_path.h"
17 18 #include "base/metrics/histogram.h"
18 19 #include "base/stl_util.h"
@@ -20,8 +21,11 @@
20 21 #include "base/strings/string_number_conversions.h"
21 22 #include "base/utf_string_conversions.h"
22 23 #include "chrome/browser/storage_monitor/media_storage_util.h"
  24 +#include "chrome/browser/storage_monitor/media_transfer_protocol_device_observer_linux.h"
23 25 #include "chrome/browser/storage_monitor/removable_device_constants.h"
24 26 #include "chrome/browser/storage_monitor/udev_util_linux.h"
  27 +#include "chrome/common/chrome_switches.h"
  28 +#include "device/media_transfer_protocol/media_transfer_protocol_manager.h"
25 29
26 30 namespace chrome {
27 31
@@ -270,6 +274,12 @@ StorageMonitorLinux::StorageMonitorLinux(const base::FilePath& path,
270 274
271 275 StorageMonitorLinux::~StorageMonitorLinux() {
272 276 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
  277 +
  278 + if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
  279 + BrowserThread::PostTask(
  280 + BrowserThread::UI, FROM_HERE,
  281 + base::Bind(&device::MediaTransferProtocolManager::Shutdown));
  282 + }
273 283 }
274 284
275 285 void StorageMonitorLinux::Init() {
@@ -282,6 +292,17 @@ void StorageMonitorLinux::Init() {
282 292 BrowserThread::PostTask(
283 293 BrowserThread::FILE, FROM_HERE,
284 294 base::Bind(&StorageMonitorLinux::InitOnFileThread, this));
  295 +
  296 + if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
  297 + scoped_refptr<base::MessageLoopProxy> loop_proxy;
  298 + loop_proxy = content::BrowserThread::GetMessageLoopProxyForThread(
  299 + content::BrowserThread::FILE);
  300 + device::MediaTransferProtocolManager::Initialize(loop_proxy);
  301 +
  302 + media_transfer_protocol_device_observer_.reset(
  303 + new MediaTransferProtocolDeviceObserverLinux());
  304 + media_transfer_protocol_device_observer_->SetNotifications(receiver());
  305 + }
285 306 }
286 307
287 308 bool StorageMonitorLinux::GetStorageInfoForPath(
5 storage_monitor/storage_monitor_linux.h
@@ -43,6 +43,8 @@ typedef void (*GetDeviceInfoFunc)(const base::FilePath& device_path,
43 43
44 44 namespace chrome {
45 45
  46 +class MediaTransferProtocolDeviceObserverLinux;
  47 +
46 48 class StorageMonitorLinux
47 49 : public StorageMonitor,
48 50 public base::RefCountedThreadSafe<StorageMonitorLinux,
@@ -141,6 +143,9 @@ class StorageMonitorLinux
141 143 // points.
142 144 MountPriorityMap mount_priority_map_;
143 145
  146 + scoped_ptr<MediaTransferProtocolDeviceObserverLinux>
  147 + media_transfer_protocol_device_observer_;
  148 +
144 149 DISALLOW_COPY_AND_ASSIGN(StorageMonitorLinux);
145 150 };
146 151
6 storage_monitor/storage_monitor_mac.h
@@ -15,6 +15,8 @@
15 15
16 16 namespace chrome {
17 17
  18 +class ImageCaptureDeviceManager;
  19 +
18 20 // This class posts notifications to listeners when a new disk
19 21 // is attached, removed, or changed.
20 22 class StorageMonitorMac
@@ -30,6 +32,8 @@ class StorageMonitorMac
30 32 // Should only be called by browser start up code. Use GetInstance() instead.
31 33 StorageMonitorMac();
32 34
  35 + void Init();
  36 +
33 37 void UpdateDisk(const DiskInfoMac& info, UpdateType update_type);
34 38
35 39 virtual bool GetStorageInfoForPath(
@@ -65,6 +69,8 @@ class StorageMonitorMac
65 69 // posted.
66 70 std::map<std::string, DiskInfoMac> disk_info_map_;
67 71
  72 + scoped_ptr<chrome::ImageCaptureDeviceManager> image_capture_device_manager_;
  73 +
68 74 DISALLOW_COPY_AND_ASSIGN(StorageMonitorMac);
69 75 };
70 76
27 storage_monitor/storage_monitor_mac.mm
@@ -4,6 +4,8 @@
4 4
5 5 #include "chrome/browser/storage_monitor/storage_monitor_mac.h"
6 6
  7 +#include "base/mac/mac_util.h"
  8 +#include "chrome/browser/storage_monitor/image_capture_device_manager.h"
7 9 #include "chrome/browser/storage_monitor/media_storage_util.h"
8 10 #include "content/public/browser/browser_thread.h"
9 11
@@ -13,6 +15,9 @@
13 15
14 16 const char kDiskImageModelName[] = "Disk Image";
15 17
  18 +// TODO(gbillock): Make these take weak pointers and don't have
  19 +// StorageMonitorMac be ref counted.
  20 +
16 21 void GetDiskInfoAndUpdateOnFileThread(
17 22 const scoped_refptr<StorageMonitorMac>& monitor,
18 23 base::mac::ScopedCFTypeRef<CFDictionaryRef> dict,
@@ -84,10 +89,17 @@ void EjectDisk(EjectDiskOptions* options) {
84 89 } // namespace
85 90
86 91 StorageMonitorMac::StorageMonitorMac() {
87   - session_.reset(DASessionCreate(NULL));
  92 +}
88 93
89   - DASessionScheduleWithRunLoop(
90   - session_, CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
  94 +StorageMonitorMac::~StorageMonitorMac() {
  95 + if (session_.get()) {
  96 + DASessionUnscheduleFromRunLoop(
  97 + session_, CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
  98 + }
  99 +}
  100 +
  101 +void StorageMonitorMac::Init() {
  102 + session_.reset(DASessionCreate(NULL));
91 103
92 104 // Register for callbacks for attached, changed, and removed devices.
93 105 // This will send notifications for existing devices too.
@@ -107,11 +119,14 @@ void EjectDisk(EjectDiskOptions* options) {
107 119 kDADiskDescriptionWatchVolumePath,
108 120 DiskDescriptionChangedCallback,
109 121 this);
110   -}
111 122
112   -StorageMonitorMac::~StorageMonitorMac() {
113   - DASessionUnscheduleFromRunLoop(
  123 + DASessionScheduleWithRunLoop(
114 124 session_, CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
  125 +
  126 + if (base::mac::IsOSLionOrLater()) {
  127 + image_capture_device_manager_.reset(new chrome::ImageCaptureDeviceManager);
  128 + image_capture_device_manager_->SetNotifications(receiver());
  129 + }
115 130 }
116 131
117 132 void StorageMonitorMac::UpdateDisk(const DiskInfoMac& info,

0 comments on commit adac768

Please sign in to comment.
Something went wrong with that request. Please try again.