[memory-infra] Trigger first periodic dump after initial period

content/browser/tracing/memory_tracing_browsertest.cc

Index: content/browser/tracing/memory_tracing_browsertest.cc
diff --git a/content/browser/tracing/memory_tracing_browsertest.cc b/content/browser/tracing/memory_tracing_browsertest.cc
index 2e222a850f78f05696184ea8087528569a016b8b..91b610dd1630fb969f7156042063217eb4a242d9 100644
--- a/content/browser/tracing/memory_tracing_browsertest.cc
+++ b/content/browser/tracing/memory_tracing_browsertest.cc
@@ -8,6 +8,7 @@
 #include "base/callback_forward.h"
 #include "base/command_line.h"
 #include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/memory_dump_provider.h"
@@ -31,6 +32,15 @@ using testing::_;
 using testing::Return;
 
 namespace content {
+namespace {
+
+void OnStartTracingDoneCallback(base::Closure quit_loop,
+                                base::WaitableEvent* evt) {
+  quit_loop.Run();
+  if (evt)

[chiniforooshan, 2017/04/12 19:32:57]

I know that BrowserInitiatedDump is flaky because the global dump request is
sent before tracing is enabled on all processes, but I don't understand why the
signalling changes anything. Doesn't the EnableMemoryTracing wait until the quit
closure is run?

[ssid, 2017/04/12 20:03:18]

Yes, you are right! I just realized both the tests have runloop till tracing is
enabled.

+    evt->Signal();
+}
+}
 
 // A mock dump provider, used to check that dump requests actually end up
 // creating memory dumps.
@@ -110,7 +120,7 @@ class MemoryTracingTest : public ContentBrowserTest {
     ContentBrowserTest::TearDown();
   }
 
-  void EnableMemoryTracing() {
+  void EnableMemoryTracing(base::WaitableEvent* start_tracing_done_signal) {
     // Re-enabling tracing could crash these tests https://crbug.com/657628 .
     if (base::trace_event::TraceLog::GetInstance()->IsEnabled()) {
       FAIL() << "Tracing seems to be already enabled. "
@@ -124,7 +134,9 @@ class MemoryTracingTest : public ContentBrowserTest {
 
     base::RunLoop run_loop;
     bool success = TracingController::GetInstance()->StartTracing(
-      trace_config, run_loop.QuitClosure());
+        trace_config,
+        base::Bind(&OnStartTracingDoneCallback, run_loop.QuitClosure(),
+                   start_tracing_done_signal));
     EXPECT_TRUE(success);
     run_loop.Run();
   }
@@ -186,7 +198,7 @@ IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest,
   EXPECT_CALL(*mock_dump_provider_, OnMemoryDump(_,_)).WillOnce(Return(true));
   EXPECT_CALL(*this, OnMemoryDumpDone(_, true /* success */));
 
-  EnableMemoryTracing();
+  EnableMemoryTracing(nullptr /* start_tracing_done_signal */);
   RequestGlobalDumpAndWait(false /* from_renderer_thread */,
                            MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::DETAILED);
@@ -202,7 +214,7 @@ IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest,
   EXPECT_CALL(*mock_dump_provider_, OnMemoryDump(_,_)).WillOnce(Return(true));
   EXPECT_CALL(*this, OnMemoryDumpDone(_, true /* success */));
 
-  EnableMemoryTracing();
+  EnableMemoryTracing(nullptr /* start_tracing_done_signal */);
   RequestGlobalDumpAndWait(true /* from_renderer_thread */,
                            MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::DETAILED);
@@ -217,7 +229,7 @@ IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest, ManyInterleavedDumps) {
       .WillRepeatedly(Return(true));
   EXPECT_CALL(*this, OnMemoryDumpDone(_, true /* success */)).Times(4);
 
-  EnableMemoryTracing();
+  EnableMemoryTracing(nullptr /* start_tracing_done_signal */);
   RequestGlobalDumpAndWait(true /* from_renderer_thread */,
                            MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::DETAILED);
@@ -240,7 +252,7 @@ IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest, ManyInterleavedDumps) {
 IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest, QueuedDumps) {
   Navigate(shell());
 
-  EnableMemoryTracing();
+  EnableMemoryTracing(nullptr /* start_tracing_done_signal */);
 
   // Issue the following 6 global memory dump requests:
   //
@@ -302,22 +314,21 @@ IN_PROC_BROWSER_TEST_F(SingleProcessMemoryTracingTest, QueuedDumps) {
 
 #endif  // !defined(GOOGLE_CHROME_BUILD)
 
-// Non-deterministic races under TSan. crbug.com/529678
-// Flaky on Linux. crbug.com/709524
-#if defined(THREAD_SANITIZER) || defined(OS_LINUX)
-#define MAYBE_BrowserInitiatedDump DISABLED_BrowserInitiatedDump
-#else
-#define MAYBE_BrowserInitiatedDump BrowserInitiatedDump
-#endif
 // Checks that a memory dump initiated from a the main browser thread ends up in
 // a successful dump.
-IN_PROC_BROWSER_TEST_F(MemoryTracingTest, MAYBE_BrowserInitiatedDump) {
+IN_PROC_BROWSER_TEST_F(MemoryTracingTest, BrowserInitiatedDump) {
   Navigate(shell());
 
   EXPECT_CALL(*mock_dump_provider_, OnMemoryDump(_,_)).WillOnce(Return(true));
   EXPECT_CALL(*this, OnMemoryDumpDone(_, true /* success */));
 
-  EnableMemoryTracing();
+  base::WaitableEvent start_tracing_done(
+      base::WaitableEvent::ResetPolicy::MANUAL,
+      base::WaitableEvent::InitialState::NOT_SIGNALED);
+  EnableMemoryTracing(&start_tracing_done);
+
+  // Wait till child processes gets tracing enabled signal.
+  start_tracing_done.Wait();
   RequestGlobalDumpAndWait(false /* from_renderer_thread */,
                            MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::DETAILED);