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

content/browser/browser_main_loop.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_BROWSER_BROWSER_MAIN_LOOP_H_
 #define CONTENT_BROWSER_BROWSER_MAIN_LOOP_H_
 
 #include <memory>
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/timer/timer.h"
 #include "build/build_config.h"
 #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 {
 class Env;
 }
 #endif
 
 namespace base {
 class CommandLine;
 class FilePath;
@@ skipping 74 matching lines @@
 // See comments in browser_main_parts.h for additional info.
 class CONTENT_EXPORT BrowserMainLoop {
  public:
   // Returns the current instance. This is used to get access to the getters
   // that return objects which are owned by this class.
   static BrowserMainLoop* GetInstance();
 
   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();
 
   // Initializes the toolkit. Returns whether the toolkit initialization was
   // successful or not.
   bool InitializeToolkit();
 
   void PreMainMessageLoopStart();
   void MainMessageLoopStart();
@@ skipping 22 matching lines @@
   }
   media::UserInputMonitor* user_input_monitor() const {
     return user_input_monitor_.get();
   }
   discardable_memory::DiscardableSharedMemoryManager*
   discardable_shared_memory_manager() const {
     return discardable_shared_memory_manager_.get();
   }
   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_;
   }
 
   const base::FilePath& startup_trace_file() const {
     return startup_trace_file_;
   }
 
   void StopStartupTracingTimer();
@@ skipping 150 matching lines @@
   std::unique_ptr<ui::ClientNativePixmapFactory> client_native_pixmap_factory_;
 #endif
 
   std::unique_ptr<LoaderDelegateImpl> loader_delegate_;
   std::unique_ptr<ResourceDispatcherHostImpl> resource_dispatcher_host_;
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
   std::unique_ptr<SpeechRecognitionManagerImpl> speech_recognition_manager_;
   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.
 
   DISALLOW_COPY_AND_ASSIGN(BrowserMainLoop);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_BROWSER_MAIN_LOOP_H_