Fix race condition when dump providers invoked within destroyed state

base/trace_event/memory_dump_manager_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/trace_event/memory_dump_manager.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <vector>
@@ skipping 99 matching lines @@
   // Promote the CreateProcessDump to public so it can be used by test fixtures.
   using MemoryDumpManagerDelegate::CreateProcessDump;
 };
 
 class MockMemoryDumpProvider : public MemoryDumpProvider {
  public:
   MOCK_METHOD0(Destructor, void());
   MOCK_METHOD2(OnMemoryDump,
                bool(const MemoryDumpArgs& args, ProcessMemoryDump* pmd));
 
-  MockMemoryDumpProvider() : enable_mock_destructor(false) {}
+  MockMemoryDumpProvider() : enable_mock_destructor(false) {
+    ON_CALL(*this, OnMemoryDump(_, _))
+        .WillByDefault(Invoke([](const MemoryDumpArgs&,
+                                 ProcessMemoryDump* pmd) -> bool {
+          EXPECT_TRUE(pmd->session_state().get() != nullptr);

[Primiano Tucci (use gerrit), 2016/04/13 20:19:16]

Plz add a comment here explaining that under no circumstance the session_state
should be null if we do a dump.

+          return true;
+        }));
+  }
   ~MockMemoryDumpProvider() override {
     if (enable_mock_destructor)
       Destructor();
   }
 
   bool enable_mock_destructor;
 };
 
 class TestSequencedTaskRunner : public SequencedTaskRunner {
  public:
@@ skipping 800 matching lines @@
 
   EXPECT_FALSE(last_callback_success_);
 }
 
 // Tests against race conditions that can happen if tracing is disabled before
 // the CreateProcessDump() call. Real-world regression: crbug.com/580295 .
 TEST_F(MemoryDumpManagerTest, DisableTracingRightBeforeStartOfDump) {
   base::WaitableEvent tracing_disabled_event(false, false);
   InitializeMemoryDumpManager(false /* is_coordinator */);
 
-  MockMemoryDumpProvider mdp;
-  RegisterDumpProvider(&mdp);
+  std::unique_ptr<Thread> thread = WrapUnique(new Thread("test thread"));

[Primiano Tucci (use gerrit), 2016/04/13 20:19:16]

as commented before, this WrapUnique is unnecessary.
std::unique_ptr<Thread> thread(new Thread(...))

[Primiano Tucci (use gerrit), 2016/04/13 20:19:16]

s/thread/mdp_thread/ for consistency with the code above

+  thread->Start();
+
+  // Create both same-thread MDP and another MDP with dedicated thread
+  MockMemoryDumpProvider mdp1;
+  RegisterDumpProvider(&mdp1);
+  MockMemoryDumpProvider mdp2;
+  RegisterDumpProvider(&mdp2, thread->task_runner(), kDefaultOptions);
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
 
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _))
       .WillOnce(Invoke([this](const MemoryDumpRequestArgs& args,
                               const MemoryDumpCallback& callback) {
         DisableTracing();
         delegate_->CreateProcessDump(args, callback);
       }));
 
+  // If tracing is disabled for current session CreateProcessDump() should NOT
+  // request dumps from providers. Real-world regression: crbug.com/600570 .
+  EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(0);
+  EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(0);
+
   last_callback_success_ = true;
   RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::DETAILED);
   EXPECT_FALSE(last_callback_success_);
 }
 
 TEST_F(MemoryDumpManagerTest, DumpOnBehalfOfOtherProcess) {
   using trace_analyzer::Query;
 
   InitializeMemoryDumpManager(false /* is_coordinator */);
@@ skipping 111 matching lines @@
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2);
   for (int i = 0; i < 2; ++i) {
     RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
                              MemoryDumpLevelOfDetail::DETAILED);
   }
   DisableTracing();
 }
 
 }  // namespace trace_event
 }  // namespace base