memory-infra: Finish moving to Mojo (3nd attempt)

content/browser/browser_main_loop.h

Index: content/browser/browser_main_loop.h
diff --git a/content/browser/browser_main_loop.h b/content/browser/browser_main_loop.h
index 8c535ff302d97492b17642d5154d53bd5528f912..57a6b55a93f31ad0edc36a0f12940f68aa08cb06 100644
--- a/content/browser/browser_main_loop.h
+++ b/content/browser/browser_main_loop.h
@@ -15,6 +15,7 @@
 #include "content/browser/browser_process_sub_thread.h"
 #include "content/public/browser/browser_main_runner.h"
 #include "media/audio/audio_manager.h"
+#include "services/resource_coordinator/memory/coordinator/coordinator_impl.h"
 
 #if defined(USE_AURA)
 namespace aura {
@@ -109,6 +110,12 @@ class CONTENT_EXPORT BrowserMainLoop {
   explicit BrowserMainLoop(const MainFunctionParams& parameters);
   virtual ~BrowserMainLoop();
 
+  // The getter method for memory_instrumentation_coordinator_. It has a
+  // static getter so that we can access it from all threads. GetInstace can be
+  // accessed from the UI thread, only.
+  static memory_instrumentation::CoordinatorImpl*

[Primiano Tucci (use gerrit), 2017/03/13 21:31:10]

hmm this seems a bit of an unprecedented pattern. Make sure you check this with
content/browser owners.
Why can't we use BML::GetInsance()->memory_instrumentation() ?

[chiniforooshan, 2017/03/14 16:06:36]

The GPUProcessHost is created in the IO thread, not the UI thread. So, when
registering the coordinator interface, it cannot directly call GetInstance(),
because GetInstance() should be called on the UI thread.

On the other hand, if it does a PostTask to register the coordinator interface
on the UI thread, then the GPU process may try to connect to the interface
before the interface is registered and so fail.

So, I thought I expose only the coordinator in the IO thread because accessing
this special field is not racy. It is created before and parallel read access
should be fine.

I'll ask jam@ to pay special attention to this part.

[jam, 2017/03/15 17:14:45]

BrowserMainLoop shouldn't be accessed on non-UI threads, even internally. Why
not have a getter in
memory_instrumentation::CoordinatorImp?

[chiniforooshan, 2017/03/15 18:27:30]

I see. I was hoping to not use global variables in memory_instrumentation; but
looks like, there is no way around it. Done. The creation and deletion of the
CoordinatorImpl is still here, to make its lifetime more explicit (instead of
using leaky lazy instances, e.g.).

+  GetMemoryInstrumentationCoordinator();
+
   void Init();
 
   void EarlyInitialization();
@@ -151,6 +158,10 @@ class CONTENT_EXPORT BrowserMainLoop {
   }
   midi::MidiService* midi_service() const { return midi_service_.get(); }
   base::Thread* indexed_db_thread() const { return indexed_db_thread_.get(); }
+  memory_instrumentation::CoordinatorImpl* memory_instrumentation_coordinator()
+      const {
+    return memory_instrumentation_coordinator_.get();
+  }
 
   bool is_tracing_startup_for_duration() const {
     return is_tracing_startup_for_duration_;
@@ -321,6 +332,8 @@ class CONTENT_EXPORT BrowserMainLoop {
   std::unique_ptr<discardable_memory::DiscardableSharedMemoryManager>
       discardable_shared_memory_manager_;
   scoped_refptr<SaveFileManager> save_file_manager_;
+  std::unique_ptr<memory_instrumentation::CoordinatorImpl>
+      memory_instrumentation_coordinator_;
 
   // DO NOT add members here. Add them to the right categories above.