[Memory-UMA] Implement basic working prototype.

chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc

Index: chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
diff --git a/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc b/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
index 173ebe02612d4185a20e5d9a3f6f364494234f22..a6bd7c1ed0e3ba4de3defaf70edf0aee05e24531 100644
--- a/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
+++ b/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
@@ -2,17 +2,44 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#if !defined(ADDRESS_SANITIZER)

[Primiano Tucci (use gerrit), 2017/05/15 03:48:09]

any reason why we are not using the MAYBE_ pattern? never seen this in this way.
Also, is this causing ASAN issues? what is the reason for this?

[erikchen, 2017/05/15 17:29:15]

I wrapped the only relevant call in a "!defined(ADDRESS_SANTIZER)." block.

+
 #include "chrome/browser/metrics/process_memory_metrics_emitter.h"
 
+#include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
+#include "base/test/histogram_tester.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/trace_config_memory_test_util.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/tracing.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "url/gurl.h"
 
 namespace {
 
+using base::trace_event::MemoryDumpType;
+
+void RequestGlobalDumpCallback(base::Closure quit_closure,
+                               uint64_t,
+                               bool success) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure);
+  ASSERT_TRUE(success);
+}
+
+void OnStartTracingDoneCallback(
+    base::trace_event::MemoryDumpLevelOfDetail explicit_dump_type,
+    base::Closure quit_closure) {
+  base::trace_event::MemoryDumpManager::GetInstance()->RequestGlobalDump(
+      MemoryDumpType::EXPLICITLY_TRIGGERED, explicit_dump_type,
+      Bind(&RequestGlobalDumpCallback, quit_closure));
+}
+
 class ProcessMemoryMetricsEmitterFake : public ProcessMemoryMetricsEmitter {
  public:
-  ProcessMemoryMetricsEmitterFake() {}
+  explicit ProcessMemoryMetricsEmitterFake(base::RunLoop* run_loop)
+      : run_loop_(run_loop) {}
 
  private:
   ~ProcessMemoryMetricsEmitterFake() override {}
@@ -22,12 +49,47 @@ class ProcessMemoryMetricsEmitterFake : public ProcessMemoryMetricsEmitter {
       bool success,
       memory_instrumentation::mojom::GlobalMemoryDumpPtr ptr) override {
     EXPECT_TRUE(success);
-    base::MessageLoop::current()->QuitWhenIdle();
+    ProcessMemoryMetricsEmitter::ReceivedMemoryDump(dump_guid, success,
+                                                    std::move(ptr));
+    if (run_loop_)
+      run_loop_->Quit();
   }
 
+  base::RunLoop* run_loop_;
+
   DISALLOW_COPY_AND_ASSIGN(ProcessMemoryMetricsEmitterFake);
 };
 
+void CheckMemoryMetric(const std::string& name,
+                       const base::HistogramTester& histogram_tester,
+                       int count) {
+  histogram_tester.ExpectTotalCount(name, count);
+  std::unique_ptr<base::HistogramSamples> samples(
+      histogram_tester.GetHistogramSamplesSinceCreation(name));
+  ASSERT_TRUE(samples);
+  EXPECT_GT(samples->sum(), 0u) << name;
+
+  // As a sanity check, no memory stat should exceed 1 GB.
+  int64_t maximum_expected_size = 1ll << 30;
+  EXPECT_LT(samples->sum(), maximum_expected_size) << name;
+}
+
+void CheckAllMemoryMetrics(const base::HistogramTester& histogram_tester,
+                           int count) {
+  CheckMemoryMetric("Memory.Experimental.Browser2.Malloc", histogram_tester,
+                    count);
+  CheckMemoryMetric("Memory.Experimental.Browser2.Resident", histogram_tester,
+                    count);
+  CheckMemoryMetric("Memory.Experimental.Renderer2.Malloc", histogram_tester,
+                    count);
+  CheckMemoryMetric("Memory.Experimental.Renderer2.Resident", histogram_tester,
+                    count);
+  CheckMemoryMetric("Memory.Experimental.Renderer2.BlinkGC", histogram_tester,
+                    count);
+  CheckMemoryMetric("Memory.Experimental.Renderer2.V8", histogram_tester,
+                    count);
+}
+
 }  // namespace
 
 class ProcessMemoryMetricsEmitterTest : public InProcessBrowserTest {
@@ -40,13 +102,69 @@ class ProcessMemoryMetricsEmitterTest : public InProcessBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ProcessMemoryMetricsEmitterTest, FetchAndEmitMetrics) {
+  base::HistogramTester histogram_tester;
+  base::RunLoop run_loop;
+
+  // Intentionally let emitter leave scope to check that it correctly keeps
+  // itself alive.
+  {
+    scoped_refptr<ProcessMemoryMetricsEmitterFake> emitter(
+        new ProcessMemoryMetricsEmitterFake(&run_loop));
+    emitter->FetchAndEmitProcessMemoryMetrics();
+  }
+
+  run_loop.Run();
+
+  CheckAllMemoryMetrics(histogram_tester, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(ProcessMemoryMetricsEmitterTest, FetchDuringTrace) {
+  base::HistogramTester histogram_tester;
+  base::RunLoop run_loop;
+
+  base::trace_event::TraceConfig trace_config(
+      base::trace_event::TraceConfigMemoryTestUtil::
+          GetTraceConfig_EmptyTriggers());
+  ASSERT_TRUE(tracing::BeginTracingWithTraceConfig(
+      trace_config, Bind(&OnStartTracingDoneCallback,
+                         base::trace_event::MemoryDumpLevelOfDetail::DETAILED,
+                         run_loop.QuitClosure())));
+
   // Intentionally let emitter leave scope to check that it correctly keeps
   // itself alive.
   {
     scoped_refptr<ProcessMemoryMetricsEmitterFake> emitter(
-        new ProcessMemoryMetricsEmitterFake);
+        new ProcessMemoryMetricsEmitterFake(&run_loop));
     emitter->FetchAndEmitProcessMemoryMetrics();
   }
 
-  content::RunMessageLoop();
+  GURL url1("about:blank");
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), url1, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  run_loop.Run();
+
+  std::string json_events;
+  ASSERT_TRUE(tracing::EndTracing(&json_events));
+  EXPECT_NE(0u, json_events.length());

[Primiano Tucci (use gerrit), 2017/05/15 03:48:09]

this one won't guarantee that the dump is in the trace. Metadata events are
added anyways, so even if the memory-infra event fails for some reason, you'd
still pass this test.

I thin you want something like:

 trace_analyzer::TraceEventVector events;
  analyzer->FindEvents(Query::EventPhaseIs(TRACE_EVENT_PHASE_MEMORY_DUMP),
                       &events);

  ASSERT_EQ(1u, events.size());
  ASSERT_TRUE(trace_analyzer::CountMatches(
      events, Query::EventNameIs(MemoryDumpTypeToString(
                  MemoryDumpType::EXPLICITLY_TRIGGERED))));

[erikchen, 2017/05/15 17:29:15]

Done.

+
+  CheckAllMemoryMetrics(histogram_tester, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(ProcessMemoryMetricsEmitterTest, FetchThreeTimes) {
+  base::HistogramTester histogram_tester;
+  base::RunLoop run_loop;
+
+  int count = 3;
+  for (int i = 0; i < count; ++i) {
+    // Only the last emitter should stop the run loop.
+    auto emitter = base::MakeShared<ProcessMemoryMetricsEmitterFake>(
+        (i == count - 1) ? &run_loop : nullptr);
+    emitter->FetchAndEmitProcessMemoryMetrics();
+  }
+
+  run_loop.Run();
+
+  CheckAllMemoryMetrics(histogram_tester, count);
 }
+#endif  // !defined(ADDRESS_SANITIZER)