[StorageMonitor] Move StorageMonitor ownership to BrowserProcessImpl

chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc

Index: chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc
diff --git a/chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc b/chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc
index 8f7d042ec2acc1664ea43f62bc257f74db091b44..f749a4c18bed973ec6c96bf27be4ee6e98e801e3 100644
--- a/chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc
+++ b/chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc
@@ -18,9 +18,11 @@
 #include "chrome/browser/storage_monitor/storage_info.h"
 #include "chrome/browser/storage_monitor/storage_monitor.h"
 #include "chrome/browser/storage_monitor/test_portable_device_watcher_win.h"
+#include "chrome/browser/storage_monitor/test_storage_monitor.h"
 #include "chrome/browser/storage_monitor/test_storage_monitor_win.h"
 #include "chrome/browser/storage_monitor/test_volume_mount_watcher_win.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -60,8 +62,8 @@ class MTPDeviceDelegateImplWinTest : public ChromeRenderViewHostTestHarness {
                         bool removable,
                         bool media_device);
 
-  // Note: need to make this weak ptr once ownership moves to g_browser_process
-  scoped_ptr<test::TestStorageMonitorWin> monitor_;
+  // Pointer to the storage monitor. Owned by TestingBrowserProcess.
+  test::TestStorageMonitorWin* monitor_;
   scoped_refptr<extensions::Extension> extension_;
 
   EnsureMediaDirectoriesExists media_directories_;
@@ -69,13 +71,19 @@ class MTPDeviceDelegateImplWinTest : public ChromeRenderViewHostTestHarness {
 
 void MTPDeviceDelegateImplWinTest::SetUp() {
   ChromeRenderViewHostTestHarness::SetUp();
+
+  test::TestStorageMonitor::RemoveSingleton();
   test::TestPortableDeviceWatcherWin* portable_device_watcher =
       new test::TestPortableDeviceWatcherWin;
   test::TestVolumeMountWatcherWin* mount_watcher =
       new test::TestVolumeMountWatcherWin;
   portable_device_watcher->set_use_dummy_mtp_storage_info(true);
-  monitor_.reset(new test::TestStorageMonitorWin(
-      mount_watcher, portable_device_watcher));
+  monitor_ = new test::TestStorageMonitorWin(
+      mount_watcher, portable_device_watcher);
+  TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
+  if (browser_process)

[vandebo (ex-Chrome), 2013/07/10 16:57:45]

When will this be false?  If it is false, monitor_ is leaked.

[Greg Billock, 2013/07/10 21:11:16]

I think this was for platforms where there are still patchy exclusion of this
code. IOS, perhaps? Shall I else { delete monitor_; } on these? It'll likely at
some point become dead code when we get exclusion figured out.

[vandebo (ex-Chrome), 2013/07/11 15:53:00]

The quickest way to make sure you have it all figured out is to change this to a
CHECK and send it to the bot.  I think it has to be dealt with one way or
another (excluded or freed) before check in though -- and it looks like you did
both.

[Greg Billock, 2013/07/11 20:53:35]

Yes. Where I'm confident we're fine I just switched to DCHECK which is better.
Bots seem happy, so I think this is solved.

+    browser_process->SetStorageMonitor(monitor_);
+
   base::RunLoop runloop;
   monitor_->EnsureInitialized(runloop.QuitClosure());
   runloop.Run();
@@ -93,7 +101,9 @@ void MTPDeviceDelegateImplWinTest::SetUp() {
 }
 
 void MTPDeviceDelegateImplWinTest::TearDown() {
-  monitor_.reset();
+  // Windows storage monitor must be destroyed on the same thread
+  // as construction.
+  test::TestStorageMonitor::RemoveSingleton();
 
   ChromeRenderViewHostTestHarness::TearDown();
 }