[MemoryInfra] Support dump providers running on SequencedTaskRunner

base/trace_event/memory_dump_manager.h

Index: base/trace_event/memory_dump_manager.h
diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h
index b7130795ff7140f353d5e0f0d04fdd6e194fe287..c19d04fe739be987f2afac237674a5fceb404595 100644
--- a/base/trace_event/memory_dump_manager.h
+++ b/base/trace_event/memory_dump_manager.h
@@ -69,9 +69,11 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   //  - name: a friendly name (duplicates allowed). Used for debugging and
   //      run-time profiling of memory-infra internals. Must be a long-lived
   //      C string.
-  //  - task_runner: if non-null, all the calls to |mdp| will be
-  //      issued on the given thread. Otherwise, |mdp| should be able to
-  //      handle calls on arbitrary threads.
+  //  - task_runner: either a SingleThreadTaskRunner or SequencedTaskRunner. All
+  //      the calls to |mdp| will be run on the given |task_runner|.
+  //      RegisterDumpProviderWithSequencedTaskRunner does not accept null
+  //      |task_runner|, otherwise if passed null |mdp| should be able to handle

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

I'd make this comment a bit simpler and skip the part 
"RegisterDumpProviderWithSequencedTaskRunner does not accept null"
should be kind of obvious and if not, they will hit a dcheck.

[ssid, 2016/02/11 01:45:03]

Done.

+  //      calls on arbitrary threads.
   //  - options: extra optional arguments. See memory_dump_provider.h.
   void RegisterDumpProvider(
       MemoryDumpProvider* mdp,
@@ -81,7 +83,12 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
       MemoryDumpProvider* mdp,
       const char* name,
       const scoped_refptr<SingleThreadTaskRunner>& task_runner,
-      const MemoryDumpProvider::Options& options);
+      MemoryDumpProvider::Options options);
+  void RegisterDumpProviderWithSequencedTaskRunner(
+      MemoryDumpProvider* mdp,
+      const char* name,
+      const scoped_refptr<SequencedTaskRunner>& task_runner,
+      MemoryDumpProvider::Options options);
   void UnregisterDumpProvider(MemoryDumpProvider* mdp);
 
   // Unregisters an unbound dump provider and takes care about its deletion
@@ -153,14 +160,15 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   //   inside ProcessMemoryDumpAsyncState is removed.
   // - In most cases, the MDPInfo is destroyed within UnregisterDumpProvider().
   // - If UnregisterDumpProvider() is called while a dump is in progress, the
-  //   MDPInfo is destroyed in the epilogue of ContinueAsyncProcessDump(), when
+  //   MDPInfo is destroyed in the epilogue of SetupNextMemoryDump(), when
   //   the copy inside ProcessMemoryDumpAsyncState is erase()-d.
   // - The non-const fields of MemoryDumpProviderInfo are safe to access only
-  //   in the |task_runner| thread, unless the thread has been destroyed.
+  //   on tasks running in the |task_runner|, unless the thread has been
+  //   destroyed.
   struct MemoryDumpProviderInfo
       : public RefCountedThreadSafe<MemoryDumpProviderInfo> {
-    // Define a total order based on the thread (i.e. |task_runner|) affinity,
-    // so that all MDP belonging to the same thread are adjacent in the set.
+    // Define a total order based on the |task_runner| affinity, so that MDPs
+    // belonging to the same SequencedTaskRunner are adjacent in the set.
     struct Comparator {
       bool operator()(const scoped_refptr<MemoryDumpProviderInfo>& a,
                       const scoped_refptr<MemoryDumpProviderInfo>& b) const;
@@ -171,7 +179,7 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
     MemoryDumpProviderInfo(
         MemoryDumpProvider* dump_provider,
         const char* name,
-        const scoped_refptr<SingleThreadTaskRunner>& task_runner,
+        const scoped_refptr<SequencedTaskRunner>& task_runner,
         const MemoryDumpProvider::Options& options);
 
     MemoryDumpProvider* const dump_provider;
@@ -183,9 +191,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
     // Human readable name, for debugging and testing. Not necessarily unique.
     const char* const name;
 
-    // The task_runner affinity. Can be nullptr, in which case the dump provider
+    // The task runner affinity. Can be nullptr, in which case the dump provider
     // will be invoked on |dump_thread_|.
-    const scoped_refptr<SingleThreadTaskRunner> task_runner;
+    const scoped_refptr<SequencedTaskRunner> task_runner;
 
     // The |options| arg passed to RegisterDumpProvider().
     const MemoryDumpProvider::Options options;
@@ -204,8 +212,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   };
 
   // Holds the state of a process memory dump that needs to be carried over
-  // across threads in order to fulfil an asynchronous CreateProcessDump()
-  // request. At any time exactly one thread owns a ProcessMemoryDumpAsyncState.
+  // across task runners in order to fulfil an asynchronous CreateProcessDump()
+  // request. At any time exactly one task runner owns a
+  // ProcessMemoryDumpAsyncState.
   struct ProcessMemoryDumpAsyncState {
     ProcessMemoryDumpAsyncState(
         MemoryDumpRequestArgs req_args,
@@ -249,7 +258,7 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
     // The thread on which unbound dump providers should be invoked.
     // This is essentially |dump_thread_|.task_runner() but needs to be kept
     // as a separate variable as it needs to be accessed by arbitrary dumpers'
-    // threads outside of the lock_ to avoid races when disabling tracing.
+    // task runners outside of the lock_ to avoid races when disabling tracing.

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

keep this as thread, task_runner doesn't add a lot and makes the sentence less
readable

[ssid, 2016/02/11 01:45:03]

Done.

     // It is immutable for all the duration of a tracing session.
     const scoped_refptr<SingleThreadTaskRunner> dump_thread_task_runner;
 
@@ -277,17 +286,37 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   void CreateProcessDump(const MemoryDumpRequestArgs& args,
                          const MemoryDumpCallback& callback);
 
-  // Continues the ProcessMemoryDump started by CreateProcessDump(), hopping
-  // across threads as needed as specified by MDPs in RegisterDumpProvider().
-  void ContinueAsyncProcessDump(
-      ProcessMemoryDumpAsyncState* owned_pmd_async_state);
+  // SetupNextMemoryDump, InvokeOnMemoryDump and FinalizeCurrentDump are called
+  // in order for each dump provider to obtain a memory dump:
+
+  // Calls InvokeOnMemoryDump for the next MDP on the task runner specified by
+  // the MDP while registration. On failure to do so, calls FinalizeCurrentDump
+  // directly.
+  void SetupNextMemoryDump(
+      scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state);
+
+  // Invokes OnMemoryDump of the last MDP and calls FinalizeCurrentDump. Should

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

remove "of the last MDP" it's really misleading.
I know what you mean here, but without context it feels like you use this
function only once at the end, which is not true.

Also "and calls FinalizeCurrentDump" add "at the end"

[ssid, 2016/02/11 01:45:03]

Done.

+  // be called on the MDP task runner.
+  void InvokeOnMemoryDump(ProcessMemoryDumpAsyncState* owned_pmd_async_state);
+
+  // Finishes current dump and calls either SetupNextMemoryDump for next dump
+  // provider or FinalizeDumpAndAddToTrace if we covered all providers.
+  void FinalizeCurrentDump(
+      scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state);
+
+  // Helper for RegierDumpProvider* functions.
+  void RegisterDumpProviderInternal(
+      MemoryDumpProvider* mdp,
+      const char* name,
+      const scoped_refptr<SequencedTaskRunner>& task_runner,
+      const MemoryDumpProvider::Options& options);
 
   // Helper for the public UnregisterDumpProvider* functions.
   void UnregisterDumpProviderInternal(MemoryDumpProvider* mdp,
                                       bool take_mdp_ownership_and_delete_async);
 
-  // An ordererd set of registered MemoryDumpProviderInfo(s), sorted by thread
-  // affinity (MDPs belonging to the same thread are adjacent).
+  // An ordererd set of registered MemoryDumpProviderInfo(s), sorted by task
+  // runner affinity (MDPs belonging to the same task runners are adjacent).
   MemoryDumpProviderInfo::OrderedSet dump_providers_;
 
   // Shared among all the PMDs to keep state scoped to the tracing session.
@@ -309,7 +338,8 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   // For time-triggered periodic dumps.
   RepeatingTimer periodic_dump_timer_;
 
-  // Thread used for MemoryDumpProviders which don't specify a thread affinity.
+  // Thread used for MemoryDumpProviders which don't specify a task runner
+  // affinity.
   scoped_ptr<Thread> dump_thread_;
 
   // The unique id of the child process. This is created only for tracing and is