[tracing] MemoryDumpManager uses TraceConfig to set the periodic dump timers

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 4d0372fd3243286fca9fbca439a89108970be143..07078a3269214190db42808d982d7521aa9c40db 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -25,8 +25,8 @@ using testing::Return;
 namespace base {
 namespace trace_event {
 namespace {
-MemoryDumpArgs high_detail_args = {MemoryDumpArgs::LevelOfDetail::HIGH};
-MemoryDumpArgs low_detail_args = {MemoryDumpArgs::LevelOfDetail::LOW};
+MemoryDumpArgs g_high_detail_args = {MemoryDumpArgs::LevelOfDetail::HIGH};
+MemoryDumpArgs g_low_detail_args = {MemoryDumpArgs::LevelOfDetail::LOW};
 }
 
 // Testing MemoryDumpManagerDelegate which short-circuits dump requests locally
@@ -44,6 +44,51 @@ class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate {
   }
 };
 
+class MemoryDumpManagerDelegateForPeriodicDumpTest
+    : public MemoryDumpManagerDelegateForTesting {

[Primiano Tucci (use gerrit), 2015/09/03 22:44:51]

Ah also why doesn't this inherit just from MemoryDumpManagerDelegate ?

[ssid, 2015/09/04 10:43:47]

Because the delegate objects are owned by MemoryDumpManagerTest. It cannot have
a scoped ptr of MemoryDumpManagerDelegate, since it is virtual. So, it holds
objects of MemoryDumpManagerDelegateForTesting instead.
Also, there is one extra ,method that needs to be copied.

+ public:
+  MemoryDumpManagerDelegateForPeriodicDumpTest(int dump_calls_to_quit,
+                                               int expected_high_dump_ratio,
+                                               base::Closure quit_closure)
+      : dump_call_count_(0),
+        dump_calls_to_quit_(dump_calls_to_quit),
+        expected_high_dump_ratio_(expected_high_dump_ratio),
+        quit_closure_(quit_closure) {}
+
+  ~MemoryDumpManagerDelegateForPeriodicDumpTest() override {}
+
+  MOCK_METHOD2(RequestGlobalMemoryDump,
+               void(const MemoryDumpRequestArgs& args,
+                    const MemoryDumpCallback& callback));
+
+  // RequestGlobalMemoryDump override for SchedulePeriodicDumpsFromTraceConfig
+  // test.
+  void RequestGlobalMemoryDump_QuitAfterCalls(
+      const MemoryDumpRequestArgs& args,
+      const MemoryDumpCallback& callback) {
+    CreateProcessDump(args, callback);

[Primiano Tucci (use gerrit), 2015/09/03 22:44:13]

No need to create any process dump

[ssid, 2015/09/04 10:43:47]

Done.

+    if (dump_call_count_ % expected_high_dump_ratio_ == 0) {

[Primiano Tucci (use gerrit), 2015/09/03 22:44:13]

I don't think you need all this logic here. You can handle the sequencing with
the EXPECT_CALL + predicate matching in the TEST_F.
In other words I think you need a super simple mock delegate (with just the MOCK
method) and then, in the TEST_F

EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(g_high_detail_args,
_)).times(1)
EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(g_low_detail_args,
_)).times(2)
EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(g_high_detail_args,
_)).times(1)
EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(g_low_detail_args,
_)).times(2)
EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(g_high_detail_args,
_)).WillOnce(Invoke(... lambda for posting the closure)

[ssid, 2015/09/04 10:43:47]

No, I cannot do that. I can define only one expect_call for a function. That i
taken as the total number of calls the function gets in the exec of the test. If
I define multiple times, the test will surely fail at multiple places saying the
function is called 5 times but it is said as 2 / 1 here.
So I have to define something like called with high args 2 times and called with
low args 3 times.
But for this I need to have an "==" defined in MemoryDumpRequestArgs. This
struct holds MemoryDumpArgs, which means I need an == defined in here as well.
Instead of defining multiple "==" in production code, I made the test logic
complicated. Also, this tests for the correct order of dump calls, not just the
number of high and low calls.

+      EXPECT_EQ(MemoryDumpArgs::LevelOfDetail::HIGH,
+                args.dump_args.level_of_detail);
+    } else {
+      EXPECT_EQ(MemoryDumpArgs::LevelOfDetail::LOW,
+                args.dump_args.level_of_detail);
+    }
+    if (++dump_call_count_ == dump_calls_to_quit_) {
+      TraceLog::GetInstance()->SetDisabled();
+      ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure_);
+    }
+  }
+
+  bool IsCoordinatorProcess() const override { return true; }
+
+ private:
+  int dump_call_count_;
+  int dump_calls_to_quit_;
+  int expected_high_dump_ratio_;
+  base::Closure quit_closure_;
+};
+
 class MemoryDumpManagerTest : public testing::Test {
  public:
   void SetUp() override {
@@ -53,7 +98,6 @@ class MemoryDumpManagerTest : public testing::Test {
     MemoryDumpManager::SetInstanceForTesting(mdm_.get());
     ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance());
     MemoryDumpManager::GetInstance()->Initialize();
-    MemoryDumpManager::GetInstance()->SetDelegate(&delegate_);
   }
 
   void TearDown() override {
@@ -72,19 +116,33 @@ class MemoryDumpManagerTest : public testing::Test {
   }
 
  protected:
+  // This enalbes tracing using the legacy category filter string.
   void EnableTracing(const char* category) {
+    delegate_.reset(new MemoryDumpManagerDelegateForTesting());
+    MemoryDumpManager::GetInstance()->SetDelegate(delegate_.get());
     TraceLog::GetInstance()->SetEnabled(
         TraceConfig(category, ""), TraceLog::RECORDING_MODE);
   }
 
+  void SetDelegate(scoped_ptr<MemoryDumpManagerDelegateForTesting> delegate) {
+    delegate_ = delegate.Pass();
+    MemoryDumpManager::GetInstance()->SetDelegate(delegate_.get());
+  }
+
+  void EnableTracingWithTraceConfig(const char* trace_config) {
+    DCHECK(delegate_);
+    TraceConfig tc(trace_config);
+    TraceLog::GetInstance()->SetEnabled(tc, TraceLog::RECORDING_MODE);
+  }
+
   void DisableTracing() { TraceLog::GetInstance()->SetDisabled(); }
 
   scoped_ptr<MemoryDumpManager> mdm_;
   bool last_callback_success_;
+  scoped_ptr<MemoryDumpManagerDelegateForTesting> delegate_;
 
  private:
   scoped_ptr<MessageLoop> message_loop_;
-  MemoryDumpManagerDelegateForTesting delegate_;
 
   // We want our singleton torn down after each test.
   ShadowingAtExitManager at_exit_manager_;
@@ -170,7 +228,7 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) {
   EnableTracing("foo-and-bar-but-not-memory");
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   DisableTracing();
 
   // Now repeat enabling the memory category and check that the dumper is
@@ -179,7 +237,7 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) {
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(3).WillRepeatedly(Return(true));
   for (int i = 0; i < 3; ++i)
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
   DisableTracing();
 
   mdm_->UnregisterDumpProvider(&mdp);
@@ -188,7 +246,7 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) {
   EnableTracing(MemoryDumpManager::kTraceCategory);
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   TraceLog::GetInstance()->SetDisabled();
 }
 
@@ -205,7 +263,7 @@ TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
           Invoke(&mdp_high_detail,
                  &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   DisableTracing();
   mdm_->UnregisterDumpProvider(&mdp_high_detail);
 
@@ -221,7 +279,7 @@ TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
           Invoke(&mdp_low_detail,
                  &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          low_detail_args);
+                          g_low_detail_args);
   DisableTracing();
   mdm_->UnregisterDumpProvider(&mdp_low_detail);
 }
@@ -244,7 +302,7 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) {
 
   for (int i = 0; i < 2; ++i)
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
 
   DisableTracing();
 }
@@ -259,7 +317,7 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   DisableTracing();
 
   // Invert: enable mdp1 and disable mdp2.
@@ -269,7 +327,7 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(0);
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   DisableTracing();
 
   // Enable both mdp1 and mdp2.
@@ -278,7 +336,7 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                          high_detail_args);
+                          g_high_detail_args);
   DisableTracing();
 }
 
@@ -293,7 +351,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
     EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(1);
     EnableTracing(MemoryDumpManager::kTraceCategory);
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
     DisableTracing();
   }
 
@@ -303,7 +361,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
     EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
     EnableTracing(MemoryDumpManager::kTraceCategory);
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
     DisableTracing();
   }
 
@@ -314,7 +372,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
     EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
     EnableTracing(MemoryDumpManager::kTraceCategory);
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
     DisableTracing();
   }
 
@@ -326,7 +384,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
     EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(1);
     EnableTracing(MemoryDumpManager::kTraceCategory);
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
     DisableTracing();
   }
 }
@@ -366,7 +424,7 @@ TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
           Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
                MessageLoop::current()->task_runner(), run_loop.QuitClosure());
       mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                              high_detail_args, callback);
+                              g_high_detail_args, callback);
       // This nested message loop (|run_loop|) will be quit if and only if
       // the RequestGlobalDump callback is invoked.
       run_loop.Run();
@@ -414,7 +472,7 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
   for (int i = 0; i < 1 + MemoryDumpManager::kMaxConsecutiveFailuresCount;
        i++) {
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
   }
 
   DisableTracing();
@@ -445,7 +503,7 @@ TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) {
 
   for (int i = 0; i < 4; i++) {
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
   }
 
   DisableTracing();
@@ -476,7 +534,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) {
 
   for (int i = 0; i < 4; i++) {
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args);
+                            g_high_detail_args);
   }
 
   DisableTracing();
@@ -526,7 +584,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) {
 
   EnableTracing(MemoryDumpManager::kTraceCategory);
   MemoryDumpRequestArgs request_args = {0, MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                        high_detail_args};
+                                        g_high_detail_args};
   mdm_->CreateProcessDump(request_args, callback);
 
   run_loop.Run();
@@ -552,11 +610,47 @@ TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) {
         Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
              MessageLoop::current()->task_runner(), run_loop.QuitClosure());
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                            high_detail_args, callback);
+                            g_high_detail_args, callback);
     run_loop.Run();
   }
   EXPECT_FALSE(last_callback_success_);
 }
 
+TEST_F(MemoryDumpManagerTest, SchedulePeriodicDumpsFromTraceConfig) {
+  const char kMemoryDumpTraceConfigString[] =
+    "{"
+      "\"included_categories\":["
+        "\"disabled-by-default-memory-infra\""
+      "],"
+      "\"memory_dump_config\":{"
+        "\"triggers\":["
+          "{"
+            "\"mode\":\"light\","
+            "\"periodic_interval_ms\":1"
+          "},"
+          "{"
+            "\"mode\":\"detailed\","
+            "\"periodic_interval_ms\":3"
+          "}"
+        "]"
+      "}"
+    "}";
+
+  RunLoop run_loop;
+  scoped_ptr<MemoryDumpManagerDelegateForPeriodicDumpTest> delegate(
+      new MemoryDumpManagerDelegateForPeriodicDumpTest(
+          5 /* dump_calls_to_quit */, 3 /* expected_high_dump_ratio */,
+          run_loop.QuitClosure()));
+  EXPECT_CALL(*delegate.get(), RequestGlobalMemoryDump(_, _))
+      .Times(5)
+      .WillRepeatedly(
+          Invoke(delegate.get(), &MemoryDumpManagerDelegateForPeriodicDumpTest::
+                                     RequestGlobalMemoryDump_QuitAfterCalls));
+  SetDelegate(delegate.Pass());
+  EnableTracingWithTraceConfig(kMemoryDumpTraceConfigString);
+
+  run_loop.Run();
+}
+
 }  // namespace trace_event
 }  // namespace base