[MemoryInfra] Support dump providers running on SequencedTaskRunner

base/trace_event/memory_dump_manager_unittest.cc

Index: base/trace_event/memory_dump_manager_unittest.cc
diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc
index c0ee57de96f49bc7c4e419917aadac51fb0eda9f..858e696953a1e991de88f668d19d1be1931d306c 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -18,6 +18,7 @@
 #include "base/test/trace_event_analyzer.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/platform_thread.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/process_memory_dump.h"
@@ -61,6 +62,17 @@ void RegisterDumpProvider(MemoryDumpProvider* mdp) {
   RegisterDumpProvider(mdp, nullptr, MemoryDumpProvider::Options());
 }
 
+void RegisterDumpProviderWithSequencedTaskRunner(

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

this seems a copy-paste of the above.
Maybe you want to have a common RegisterDumpProvider which taks a SeqTaskRun as
arugment and a boolean (register_seq_task_runn) and have a wrapping function to
call the right version

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

Done.

+    MemoryDumpProvider* mdp,
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+    const MemoryDumpProvider::Options& options) {
+  MemoryDumpManager* mdm = MemoryDumpManager::GetInstance();
+  mdm->set_dumper_registrations_ignored_for_testing(false);
+  mdm->RegisterDumpProviderWithSequencedTaskRunner(mdp, "TestDumpProvider",
+                                                   task_runner, options);
+  mdm->set_dumper_registrations_ignored_for_testing(true);
+}
+
 void OnTraceDataCollected(Closure quit_closure,
                           trace_event::TraceResultBuffer* buffer,
                           const scoped_refptr<RefCountedString>& json,
@@ -110,6 +122,48 @@ class MockMemoryDumpProvider : public MemoryDumpProvider {
   bool enable_mock_destructor;
 };
 
+class TestSequencedTaskRunner : public SequencedTaskRunner {

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

why do you need to create your own SeqTaskRunner? Can't you use an existing one?

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

There seems to be no SeqTaskRunner implementation in base. I am using the
SequencedWorkerPool with single token to make a SeqTaskRunner.

+ public:
+  TestSequencedTaskRunner()
+      : worker_pool_(
+            new SequencedWorkerPool(2 /* max_threads */, "Test Task Runner")),
+        disabled_(false),
+        no_of_post_tasks_(0) {}
+
+  void set_enabled() { disabled_ = false; }
+  void set_disabled() { disabled_ = true; }
+  unsigned no_of_post_tasks() const { return no_of_post_tasks_; }
+
+  bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
+                                  const Closure& task,
+                                  TimeDelta delay) override {
+    NOTREACHED();
+    return false;
+  }
+
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       const Closure& task,
+                       TimeDelta delay) override {
+    EXPECT_EQ(TimeDelta(), delay);
+    no_of_post_tasks_++;
+    if (disabled_)
+      return false;
+    return worker_pool_->PostSequencedWorkerTask(token_, from_here, task);
+  }
+
+  bool RunsTasksOnCurrentThread() const override {
+    return worker_pool_->IsRunningSequenceOnCurrentThread(token_);
+  }
+
+ private:
+  ~TestSequencedTaskRunner() override {}
+
+  scoped_refptr<SequencedWorkerPool> worker_pool_;
+  const SequencedWorkerPool::SequenceToken token_;
+  bool disabled_;
+  unsigned no_of_post_tasks_;
+};
+
 class MemoryDumpManagerTest : public testing::Test {
  public:
   MemoryDumpManagerTest() : testing::Test(), kDefaultOptions() {}
@@ -382,65 +436,147 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
   }
 }
 
-// Checks that the MemoryDumpManager respects the thread affinity when a
-// MemoryDumpProvider specifies a task_runner(). The test starts creating 8
-// threads and registering a MemoryDumpProvider on each of them. At each
-// iteration, one thread is removed, to check the live unregistration logic.
+// Checks that the MemoryDumpManager respects the task runner affinity when a
+// MemoryDumpProvider specifies task runner. The test starts creating 8 threads
+// and sequenced task runners and registering a MemoryDumpProvider on each of
+// them. At each iteration, one task runner of each kind is removed, to check
+// the live unregistration logic.
 TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
   InitializeMemoryDumpManager(false /* is_coordinator */);
-  const uint32_t kNumInitialThreads = 8;
+  const uint32_t kNumTaskRunners = 8;
 
   std::vector<scoped_ptr<Thread>> threads;
-  std::vector<scoped_ptr<MockMemoryDumpProvider>> mdps;
+  std::vector<scoped_refptr<TestSequencedTaskRunner>> sequenced_task_runners;
+  std::vector<scoped_ptr<MockMemoryDumpProvider>> thread_mdps;

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

I'd call them threaded_mdps and sequenced_mdps

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

Done.

+  std::vector<scoped_ptr<MockMemoryDumpProvider>> sequenced_runner_mdps;
 
   // Create the threads and setup the expectations. Given that at each iteration
-  // we will pop out one thread/MemoryDumpProvider, each MDP is supposed to be
-  // invoked a number of times equal to its index.
-  for (uint32_t i = kNumInitialThreads; i > 0; --i) {
+  // we will pop out one task runner of each kind and it's MDP, each MDP is
+  // supposed to be invoked a number of times equal to its index.
+  for (uint32_t i = kNumTaskRunners; i > 0; --i) {
     threads.push_back(make_scoped_ptr(new Thread("test thread")));
     auto thread = threads.back().get();
     thread->Start();
-    scoped_refptr<SingleThreadTaskRunner> task_runner = thread->task_runner();
-    mdps.push_back(make_scoped_ptr(new MockMemoryDumpProvider()));
-    auto mdp = mdps.back().get();
-    RegisterDumpProvider(mdp, task_runner, kDefaultOptions);
-    EXPECT_CALL(*mdp, OnMemoryDump(_, _))
+    scoped_refptr<SingleThreadTaskRunner> thread_task_runner =
+        thread->task_runner();
+    sequenced_task_runners.push_back(
+        make_scoped_refptr(new TestSequencedTaskRunner()));
+    auto sequenced_task_runner = sequenced_task_runners.back().get();
+
+    thread_mdps.push_back(make_scoped_ptr(new MockMemoryDumpProvider()));
+    auto thread_mdp = thread_mdps.back().get();
+    sequenced_runner_mdps.push_back(
+        make_scoped_ptr(new MockMemoryDumpProvider()));
+    auto sequenced_runner_mdp = sequenced_runner_mdps.back().get();
+
+    RegisterDumpProvider(thread_mdp, thread_task_runner, kDefaultOptions);
+    RegisterDumpProviderWithSequencedTaskRunner(
+        sequenced_runner_mdp, sequenced_task_runner, kDefaultOptions);
+
+    EXPECT_CALL(*thread_mdp, OnMemoryDump(_, _))
         .Times(i)
-        .WillRepeatedly(Invoke(
-            [task_runner](const MemoryDumpArgs&, ProcessMemoryDump*) -> bool {
-              EXPECT_TRUE(task_runner->RunsTasksOnCurrentThread());
+        .WillRepeatedly(
+            Invoke([thread_task_runner](const MemoryDumpArgs&,
+                                        ProcessMemoryDump*) -> bool {
+              EXPECT_TRUE(thread_task_runner->RunsTasksOnCurrentThread());
               return true;
             }));
+
+    EXPECT_CALL(*sequenced_runner_mdp, OnMemoryDump(_, _))
+        .Times(i)
+        .WillRepeatedly(Invoke([sequenced_task_runner](
+                                   const MemoryDumpArgs&,
+                                   ProcessMemoryDump* pmd) -> bool {
+          EXPECT_TRUE(
+              SequencedWorkerPool::GetSequenceTokenForCurrentThread().Equals(
+                  SequencedWorkerPool::SequenceToken()));
+          EXPECT_TRUE(sequenced_task_runner->RunsTasksOnCurrentThread());
+          return true;
+        }));
   }
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
 
-  while (!threads.empty()) {
+  for (uint32_t i = 1; i <= kNumTaskRunners; ++i) {
     last_callback_success_ = false;
     EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(1);
     RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
                              MemoryDumpLevelOfDetail::DETAILED);
     EXPECT_TRUE(last_callback_success_);
+    // Check that each dump call was posted for the last sequenced task runner.
+    EXPECT_EQ(i, sequenced_task_runners.back()->no_of_post_tasks());
 
-    // Unregister a MDP and destroy one thread at each iteration to check the
-    // live unregistration logic. The unregistration needs to happen on the same
-    // thread the MDP belongs to.
+    // Destroy one thread and one sequenced task runner and their corresponding
+    // MDPs at each iteration to check the live unregistration logic. The
+    // unregistration needs to happen on the same task runner the MDP belongs
+    // to.
     {
       RunLoop run_loop;
-      Closure unregistration =
+      Closure unregistration1 =
           Bind(&MemoryDumpManager::UnregisterDumpProvider,
-               Unretained(mdm_.get()), Unretained(mdps.back().get()));
-      threads.back()->task_runner()->PostTaskAndReply(FROM_HERE, unregistration,
-                                                      run_loop.QuitClosure());
+               Unretained(mdm_.get()), Unretained(thread_mdps.back().get()));
+      Closure unregistration2 = Bind(
+          &MemoryDumpManager::UnregisterDumpProvider, Unretained(mdm_.get()),
+          Unretained(sequenced_runner_mdps.back().get()));
+      threads.back()->task_runner()->PostTask(FROM_HERE, unregistration1);
+      sequenced_task_runners.back()->PostTaskAndReply(
+          FROM_HERE, unregistration2, run_loop.QuitClosure());
       run_loop.Run();
     }
-    mdps.pop_back();
+
+    thread_mdps.pop_back();
+    sequenced_runner_mdps.pop_back();
     threads.back()->Stop();
+    sequenced_task_runners.pop_back();
     threads.pop_back();
   }
 
   DisableTracing();
 }
 
+// Check that the memory dump calls are always posted on task runner for
+// SequencedTaskRunner case and that the dump provider gets disabled when
+// PostTask fails, but the dump still succeeds.
+TEST_F(MemoryDumpManagerTest, PostTaskForSequencedTaskRunner) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
+  std::vector<MockMemoryDumpProvider> mdps(3);
+  scoped_refptr<TestSequencedTaskRunner> task_runner1(
+      make_scoped_refptr(new TestSequencedTaskRunner()));
+  scoped_refptr<TestSequencedTaskRunner> task_runner2(
+      make_scoped_refptr(new TestSequencedTaskRunner()));
+  RegisterDumpProviderWithSequencedTaskRunner(&mdps[0], task_runner1,
+                                              kDefaultOptions);
+  RegisterDumpProviderWithSequencedTaskRunner(&mdps[1], task_runner2,
+                                              kDefaultOptions);
+  RegisterDumpProviderWithSequencedTaskRunner(&mdps[2], task_runner2,
+                                              kDefaultOptions);
+  task_runner1->set_disabled();
+  EXPECT_CALL(mdps[0], OnMemoryDump(_, _)).Times(0);
+  EXPECT_CALL(mdps[1], OnMemoryDump(_, _)).Times(2);
+  EXPECT_CALL(mdps[2], OnMemoryDump(_, _)).Times(2);
+  EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2);
+
+  EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
+  last_callback_success_ = false;
+  RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                           MemoryDumpLevelOfDetail::DETAILED);
+  // Tasks should be posted even if |mdps[1]| and |mdps[2]| belong to same task
+  // runner.
+  EXPECT_EQ(1u, task_runner1->no_of_post_tasks());
+  EXPECT_EQ(2u, task_runner2->no_of_post_tasks());
+  EXPECT_TRUE(last_callback_success_);
+
+  // Check that |mdps[0]|'s OnMemoryDump is not called even if the task runner
+  // is enabled since |mdps[0]| was disabled previously.
+  task_runner1->set_enabled();
+  last_callback_success_ = false;
+  RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                           MemoryDumpLevelOfDetail::DETAILED);
+  EXPECT_EQ(2u, task_runner1->no_of_post_tasks());
+  EXPECT_EQ(4u, task_runner2->no_of_post_tasks());
+  EXPECT_TRUE(last_callback_success_);
+  DisableTracing();
+}
+
 // Checks that providers get disabled after 3 consecutive failures, but not
 // otherwise (e.g., if interleaved).
 TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {