[StorageMonitor] Move StorageMonitor ownership to BrowserProcessImpl

chrome/browser/storage_monitor/test_storage_monitor.cc

 // Copyright (c) 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 "chrome/browser/storage_monitor/test_storage_monitor.h"
 
+#include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_impl.h"
 #include "chrome/browser/storage_monitor/storage_info.h"
+#include "chrome/test/base/testing_browser_process.h"
 
 #if defined(OS_LINUX)
 #include "chrome/browser/storage_monitor/test_media_transfer_protocol_manager_linux.h"
 #include "device/media_transfer_protocol/media_transfer_protocol_manager.h"
 #endif
 
 namespace chrome {
 namespace test {
 
 TestStorageMonitor::TestStorageMonitor()
     : StorageMonitor(),
       init_called_(false) {
 #if defined(OS_LINUX)
   media_transfer_protocol_manager_.reset(
       new TestMediaTransferProtocolManagerLinux());
 #endif
 }
 
 TestStorageMonitor::~TestStorageMonitor() {}
 
-TestStorageMonitor*
-TestStorageMonitor::CreateForBrowserTests() {
-  StorageMonitor::RemoveSingletonForTesting();
-  return new TestStorageMonitor();
+TestStorageMonitor* TestStorageMonitor::Create() {
+  RemoveSingleton();
+  TestStorageMonitor* monitor = new TestStorageMonitor();
+  monitor->Init();
+  monitor->MarkInitialized();
+  TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();

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

This pattern looks like it is repeated in a lot of unit tests, could you just
call this method instead?

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

I do where possible. Perhaps I should split out the installer, though. Where it
isn't the TestStorageMonitor, we install platform-specific ones.

+  if (browser_process)

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

leak

[Greg Billock, 2013/07/10 21:57:17]

Done.

+    browser_process->SetStorageMonitor(monitor);
+
+  return monitor;
+}
+
+TestStorageMonitor* TestStorageMonitor::CreateForBrowserTests() {
+  TestStorageMonitor* return_monitor = new TestStorageMonitor();
+  return_monitor->Init();
+  return_monitor->MarkInitialized();
+
+  scoped_ptr<StorageMonitor> monitor(return_monitor);
+  BrowserProcessImpl* browser_process =
+      static_cast<BrowserProcessImpl*>(g_browser_process);
+  browser_process->set_storage_monitor_for_test(monitor.Pass());
+  return return_monitor;
+}
+
+void TestStorageMonitor::RemoveSingleton() {
+  TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
+  if (browser_process)
+    browser_process->SetStorageMonitor(NULL);
+}
+
+// static
+void TestStorageMonitor::SyncInitialize() {

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

It seems like this method is only used from one test.  Would it make more sense
to move this code into that test?

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

I thought about that, but it seems more a util fixture method -- isn't specific
to any StoraegMonitor.

+  StorageMonitor* monitor = g_browser_process->storage_monitor();
+  if (monitor->IsInitialized())
+    return;
+
+  base::WaitableEvent event(true, false);
+  monitor->EnsureInitialized(base::Bind(&base::WaitableEvent::Signal,
+                             base::Unretained(&event)));
+  while (!event.IsSignaled()) {
+    base::RunLoop().RunUntilIdle();
+  }
+  DCHECK(monitor->IsInitialized());
 }
 
 void TestStorageMonitor::Init() {
   init_called_ = true;
 }
 
 void TestStorageMonitor::MarkInitialized() {
   StorageMonitor::MarkInitialized();
 }
 
@@ skipping 35 matching lines @@
 
 void TestStorageMonitor::EjectDevice(
     const std::string& device_id,
     base::Callback<void(EjectStatus)> callback) {
   ejected_device_ = device_id;
   callback.Run(EJECT_OK);
 }
 
 }  // namespace test
 }  // namespace chrome