[StorageMonitor] Move StorageMonitor ownership to BrowserProcessImpl

chrome/browser/storage_monitor/test_storage_monitor.cc

Index: chrome/browser/storage_monitor/test_storage_monitor.cc
diff --git a/chrome/browser/storage_monitor/test_storage_monitor.cc b/chrome/browser/storage_monitor/test_storage_monitor.cc
index e28ce2c91529b84667f0ea699928d6c2c454ceb0..2f8b44f08dc68c6e80defce75dbd97f085f50d3c 100644
--- a/chrome/browser/storage_monitor/test_storage_monitor.cc
+++ b/chrome/browser/storage_monitor/test_storage_monitor.cc
@@ -4,7 +4,12 @@
 
 #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"
@@ -25,10 +30,49 @@ TestStorageMonitor::TestStorageMonitor()
 
 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() {