memory-infra: Move dump level check to observer and rename session state

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 48ae4547c7120f273d76eaa747c21d51ad526d89..f6c78634072174cf181483b82bb2053c40b96868 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -163,6 +163,9 @@ class MockMemoryDumpProvider : public MemoryDumpProvider {
           // invoking a memory dump. The problem might arise in race conditions
           // like crbug.com/600570 .
           EXPECT_TRUE(pmd->session_state().get() != nullptr);
+          pmd->CreateAllocatorDump("example")->AddScalar(

[Primiano Tucci (use gerrit), 2017/05/05 13:38:25]

do you need this or is this the leftover from some local debugging?

[hjd, 2017/05/05 14:50:53]

Yeah, removed. Thanks!

+              MemoryAllocatorDump::kNameSize, MemoryAllocatorDump::kUnitsBytes,
+              1024);
           return true;
         }));
 
@@ -416,23 +419,27 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) {
   RegisterDumpProvider(&mdp2, nullptr);
 
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
-  const MemoryDumpSessionState* session_state =
-      mdm_->session_state_for_testing().get();
+  const MemoryDumpSessionState* heap_profiler_serialization_state =
+      mdm_->heap_profiler_serialization_state_for_testing().get();
   EXPECT_CALL(global_dump_handler_, RequestGlobalMemoryDump(_, _)).Times(2);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
       .Times(2)
-      .WillRepeatedly(Invoke([session_state](const MemoryDumpArgs&,
-                                             ProcessMemoryDump* pmd) -> bool {
-        EXPECT_EQ(session_state, pmd->session_state().get());
-        return true;
-      }));
+      .WillRepeatedly(
+          Invoke([heap_profiler_serialization_state](
+                     const MemoryDumpArgs&, ProcessMemoryDump* pmd) -> bool {
+            EXPECT_EQ(heap_profiler_serialization_state,
+                      pmd->session_state().get());
+            return true;
+          }));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _))
       .Times(2)
-      .WillRepeatedly(Invoke([session_state](const MemoryDumpArgs&,
-                                             ProcessMemoryDump* pmd) -> bool {
-        EXPECT_EQ(session_state, pmd->session_state().get());
-        return true;
-      }));
+      .WillRepeatedly(
+          Invoke([heap_profiler_serialization_state](
+                     const MemoryDumpArgs&, ProcessMemoryDump* pmd) -> bool {
+            EXPECT_EQ(heap_profiler_serialization_state,
+                      pmd->session_state().get());
+            return true;
+          }));
 
   for (int i = 0; i < 2; ++i) {
     RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
@@ -1250,6 +1257,9 @@ TEST_F(MemoryDumpManagerTest, TestWhitelistingMDP) {
 TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) {
   InitializeMemoryDumpManager(true /* is_coordinator */);
 
+  MockMemoryDumpProvider mdp;
+  RegisterDumpProvider(&mdp, ThreadTaskRunnerHandle::Get());

[hjd, 2017/05/05 11:42:27]

We now need an mdp to hit the code path where the dump will be rejected now
(since rejection happens just before serialization.

[Primiano Tucci (use gerrit), 2017/05/05 13:38:25]

Acknowledged. Maybe add a comment to explain this in the code?

+
   RunLoop run_loop;
   auto test_task_runner = ThreadTaskRunnerHandle::Get();
   auto quit_closure = run_loop.QuitClosure();
@@ -1272,6 +1282,8 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) {
       TraceConfigMemoryTestUtil::GetTraceConfig_BackgroundTrigger(
           1 /* period_ms */));
 
+  run_loop.Run();

[hjd, 2017/05/05 11:42:27]

Previously this was racy, we relied on the 6 background dumps executing before
the light and detailed dumps.

[Primiano Tucci (use gerrit), 2017/05/05 13:38:25]

Well I think that became just racy now. Before we wouldn't have started the
dumps, so we would have seen only the background dumps. Instead now they would
be interleaved (because we nack only after reaching the serialization point), so
this is now required.

[hjd, 2017/05/05 14:50:53]

On master if I run the test on its own it hangs forever about 50% of the time.
(Although after your explanation I'm not sure why.)

+
   // Only background mode dumps should be allowed with the trace config.
   last_callback_success_ = false;
   RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
@@ -1283,7 +1295,6 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) {
   EXPECT_FALSE(last_callback_success_);
 
   ASSERT_TRUE(IsPeriodicDumpingEnabled());
-  run_loop.Run();
   DisableTracing();
 }