[StorageMonitor] Move StorageMonitor ownership to BrowserProcessImpl

chrome/browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc

 // Copyright 2013 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.
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/message_loop.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/media_galleries/media_file_system_registry.h"
 #include "chrome/browser/media_galleries/media_galleries_test_util.h"
 #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"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chrome {
 
 namespace {
 
 typedef std::map<MediaGalleryPrefId, MediaFileSystemInfo> FSInfoMap;
@@ skipping 19 matching lines @@
                      const base::FilePath::StringType& location);
   std::string AttachDevice(StorageInfo::Type type,
                            const std::string& unique_id,
                            const base::FilePath& location);
   void CheckGalleryInfo(const MediaFileSystemInfo& info,
                         const string16& name,
                         const base::FilePath& path,
                         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_;
 };
 
 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();
 
   extensions::TestExtensionSystem* extension_system(
       static_cast<extensions::TestExtensionSystem*>(
           extensions::ExtensionSystem::Get(profile())));
   extension_system->CreateExtensionService(
       CommandLine::ForCurrentProcess(), base::FilePath(), false);
 
   std::vector<std::string> all_permissions;
   all_permissions.push_back("allAutoDetected");
   all_permissions.push_back("read");
   extension_ = AddMediaGalleriesApp("all", all_permissions, profile());
 }
 
 void MTPDeviceDelegateImplWinTest::TearDown() {
-  monitor_.reset();
+  // Windows storage monitor must be destroyed on the same thread
+  // as construction.
+  test::TestStorageMonitor::RemoveSingleton();
 
   ChromeRenderViewHostTestHarness::TearDown();
 }
 
 void MTPDeviceDelegateImplWinTest::ProcessAttach(
     const std::string& id,
     const string16& label,
     const base::FilePath::StringType& location) {
   StorageInfo info(id, string16(), location, label, string16(), string16(), 0);
   monitor_->receiver()->ProcessAttach(info);
@@ skipping 51 matching lines @@
     if (info.path == location) {
       CheckGalleryInfo(info, location.LossyDisplayName(), location, true, true);
       checked = true;
       break;
     }
   }
   EXPECT_TRUE(checked);
 }
 
 }  // namespace chrome