memory-infra: Finish moving memory_infra from TracingController

services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h

Index: services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h
diff --git a/services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h b/services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h
index 117661997368983b166c9a275728e054f488a42b..e9f65c0abf93e9a522908fa27ef5b8a4eb6e375c 100644
--- a/services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h
+++ b/services/resource_coordinator/public/cpp/memory/memory_dump_manager_delegate_impl.h
@@ -5,6 +5,9 @@
 #ifndef SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_MEMORY_DUMP_MANAGER_DELEGATE_IMPL_H_
 #define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_MEMORY_DUMP_MANAGER_DELEGATE_IMPL_H_
 
+#include "base/lazy_instance.h"
+#include "base/single_thread_task_runner.h"
+#include "base/synchronization/lock.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/memory_dump_request_args.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -18,30 +21,57 @@ class MemoryDumpManagerDelegateImpl
     : public base::trace_event::MemoryDumpManagerDelegate,
       public mojom::ProcessLocalDumpManager {
  public:
+  static MemoryDumpManagerDelegateImpl* GetInstance();
+
   // Use to bind to a remote coordinator.
-  MemoryDumpManagerDelegateImpl(
+  void InitializeWithInterfaceProvider(
       service_manager::InterfaceProvider* interface_provider);
 
   // Use to bind to a coordinator in the same process.
-  MemoryDumpManagerDelegateImpl(Coordinator* coordinator);
-  ~MemoryDumpManagerDelegateImpl() override;
+  void InitializeWithCoordinator(
+      Coordinator* coordinator,
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
-  bool IsCoordinator() const;
+  bool IsCoordinator() const override;
 
   // The base::trace_event::MemoryDumpManager calls this.
   void RequestGlobalMemoryDump(
       const base::trace_event::MemoryDumpRequestArgs& args,
       const base::trace_event::MemoryDumpCallback& callback) override;
 
+  void SetAsNonCoordinatorForTesting();
+
  private:
+  friend std::default_delete<MemoryDumpManagerDelegateImpl>;  // For testing
+  friend class MemoryDumpManagerDelegateImplTest;             // For testing
+  friend struct base::DefaultLazyInstanceTraits<MemoryDumpManagerDelegateImpl>;
+
+  MemoryDumpManagerDelegateImpl();
+  ~MemoryDumpManagerDelegateImpl() override;
+
   // The ProcessLocalDumpManager interface. The coordinator calls this.
   void RequestProcessMemoryDump(
       const base::trace_event::MemoryDumpRequestArgs& args,
       const RequestProcessMemoryDumpCallback& callback) override;
 
+  // A proxy callback for updating |pending_memory_dump_guid_|.
+  void MemoryDumpCallbackProxy(
+      const base::trace_event::MemoryDumpCallback& callback,
+      uint64_t dump_guid,
+      bool success);
+
+  bool initialized_;
   bool is_coordinator_;
   mojom::CoordinatorPtr coordinator_;
   mojo::Binding<mojom::ProcessLocalDumpManager> binding_;
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

[ssid, 2017/02/24 23:19:05]

Why do we have task runner here? It sounds cleaner if we do the same thing for
all processes in the delegate instead of posting tasks.
The CoordinatorImpl::RequestGlobalMemoryDump could handle the thread jumps,
right?

I'd prefer not to have is_coordintor_ flag also here. But, we need it
temporarily here.

[chiniforooshan, 2017/02/25 00:04:51]

In the case of child processes, CoordinatorImpl will be called on the correct
thread automatically. Mojo handles that for us.

In the case of the process that runs the Coordinator service (i.e. browser), the
delegate has to call CoordinatorImpl on the correct thread or else Mojo will
complain.

So, I think CoordinatorImpl should just assume that it is called on the correct
thread and the browser delegate should do the thread hopping.

I think is_coordinator_ is equivalent to task_runner_ == nullptr and so is
redundant. I deleted it.

[ssid, 2017/02/25 16:07:21]

Hmm thanks for explaining. Sg

+  uint64_t pending_memory_dump_guid_;
+
+  // Prevents racy access to |pending_memory_dump_guid_|.
+  base::Lock pending_memory_dump_guid_lock_;
+
+  // Prevents racy access to |initialized_|.
+  base::Lock initialized_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryDumpManagerDelegateImpl);
 };