[Tracing] Disable registration of regular dump providers during tests

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 "base/bind_helpers.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
@@ skipping 34 matching lines @@
 };
 
 class MemoryDumpManagerTest : public testing::Test {
  public:
   void SetUp() override {
     last_callback_success_ = false;
     message_loop_.reset(new MessageLoop());
     mdm_.reset(new MemoryDumpManager());
     MemoryDumpManager::SetInstanceForTesting(mdm_.get());
     ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance());
-    MemoryDumpManager::GetInstance()->Initialize();
-    MemoryDumpManager::GetInstance()->SetDelegate(&delegate_);
+    mdm_->set_ignore_dumper_registrations_for_testing(true);
+    mdm_->Initialize();
+    mdm_->SetDelegate(&delegate_);

[picksi, 2015/09/22 10:53:38]

Random drive-by thought. For other users (i.e. outside of these tests) does the
order of these calls need to be maintained for correct execution? If so can this
be enforced somehow (at compile time), e.g. Add and InitialzizeWithDelegate()
function or make Initilize return a delegate-handler (?) that is used to set a
delegate... etc.

[Ruud van Asseldonk, 2015/09/29 09:12:27]

The |SetDelegate| method did not survive Primiano’s Great Refactor™. In fact,
the new |Initialize| method takes a delegate.

   }
 
   void TearDown() override {
     MemoryDumpManager::SetInstanceForTesting(nullptr);
     mdm_.reset();
     message_loop_.reset();
     TraceLog::DeleteForTesting();
   }
 
   void DumpCallbackAdapter(scoped_refptr<SingleThreadTaskRunner> task_runner,
                            Closure closure,
                            uint64 dump_guid,
                            bool success) {
     last_callback_success_ = success;
     task_runner->PostTask(FROM_HERE, closure);
   }
 
  protected:
   const char* kTraceCategory = MemoryDumpManager::kTraceCategoryForTesting;
 
   void EnableTracing(const char* category) {
+    bool registrations_ignored =
+        mdm_->ignore_dumper_registrations_for_testing();

[picksi, 2015/09/22 10:53:38]

Nit: This function name makes it sound like you are ignoring dumper
registration, rather than getting it. Should this be renamed to something like
are_dumper_registrations_ignored()?

[Ruud van Asseldonk, 2015/09/29 09:12:27]

Not according to the style guide (unless I also rename the variable) which says:
Do you think the variable should be renamed?

[picksi, 2015/09/29 10:55:56]

You could rename the variable 'dumper_registrations_ignored_for_testing' which
would be less confusing and is in line with your local variable
'registrations_ignored'.

FWIW Are you also able to drop 'dumper_' as a prefix? (I think) the context of
the call, e.g. mdm->registrations_ignored_for_testing(), makes it obvious that
it's a dumper. Then you'd end up with 'registrations_ignored_for_testing'. WDYT?

+    mdm_->set_ignore_dumper_registrations_for_testing(true);
     TraceLog::GetInstance()->SetEnabled(
         TraceConfig(category, ""), TraceLog::RECORDING_MODE);
+    mdm_->set_ignore_dumper_registrations_for_testing(registrations_ignored);
   }
 
   void DisableTracing() { TraceLog::GetInstance()->SetDisabled(); }
 
+  void EnableMemoryDumpProviderRegistrations() {
+    mdm_->set_ignore_dumper_registrations_for_testing(false);
+  }
+
+  void DisableMemoryDumpProviderRegistrations() {

[picksi, 2015/09/22 10:53:38]

nit: Should you be using this function in EnableTracing() instead of directly
calling the setter?

[Ruud van Asseldonk, 2015/09/29 09:12:27]

In |UnregisterDumperFromThreadWhileDumping|, registrations need to be
enabled/disabled at a different point, so a method is necessary either way.

+    mdm_->set_ignore_dumper_registrations_for_testing(true);
+  }
+
   scoped_ptr<MemoryDumpManager> mdm_;
   bool last_callback_success_;
 
  private:
   scoped_ptr<MessageLoop> message_loop_;
   MemoryDumpManagerDelegateForTesting delegate_;
 
   // We want our singleton torn down after each test.
   ShadowingAtExitManager at_exit_manager_;
 };
@@ skipping 65 matching lines @@
   MemoryDumpProvider* dump_provider_to_register_or_unregister;
 
  private:
   MemoryDumpSessionState* last_session_state_;
   scoped_refptr<SingleThreadTaskRunner> task_runner_;
   const MemoryDumpArgs::LevelOfDetail level_of_detail_;
 };
 
 TEST_F(MemoryDumpManagerTest, SingleDumper) {
   MockDumpProvider mdp;
+  EnableMemoryDumpProviderRegistrations();

[Primiano Tucci (use gerrit), 2015/09/22 08:07:02]

Hmm this is a unittest. Why do you need all these enable/disable registrations
here? Nothing else should happen (as opposite to a browser test, where random
things can register and unregister)

[Ruud van Asseldonk, 2015/09/29 09:12:27]

I removed |skip_core_dumpers_auto_registration_for_testing_| in favour of
|ignore_dumper_registrations_for_testing_|, so this is required to prevent the
core dumpers from registering.

   mdm_->RegisterDumpProvider(&mdp);
 
   // Check that the dumper is not called if the memory category is not enabled.
   EnableTracing("foo-and-bar-but-not-memory");
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           high_detail_args);
   DisableTracing();
 
   // Now repeat enabling the memory category and check that the dumper is
@@ skipping 12 matching lines @@
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           high_detail_args);
   TraceLog::GetInstance()->SetDisabled();
 }
 
 TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
   // Check that requesting dumps with high level of detail actually propagates
   // to OnMemoryDump() call on dump providers.
   MockDumpProvider mdp_high_detail(MemoryDumpArgs::LevelOfDetail::HIGH);
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp_high_detail);
 
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp_high_detail, OnMemoryDump(_, _))
       .Times(1)
       .WillRepeatedly(
           Invoke(&mdp_high_detail,
                  &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           high_detail_args);
@@ skipping 13 matching lines @@
                  &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           low_detail_args);
   DisableTracing();
   mdm_->UnregisterDumpProvider(&mdp_low_detail);
 }
 
 TEST_F(MemoryDumpManagerTest, SharedSessionState) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp1);
   mdm_->RegisterDumpProvider(&mdp2);
 
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
       .Times(2)
       .WillRepeatedly(
           Invoke(&mdp1, &MockDumpProvider::OnMemoryDump_CheckSessionState));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _))
       .Times(2)
       .WillRepeatedly(
           Invoke(&mdp2, &MockDumpProvider::OnMemoryDump_CheckSessionState));
 
   for (int i = 0; i < 2; ++i)
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                             high_detail_args);
 
   DisableTracing();
 }
 
 TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
 
   // Enable only mdp1.
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp1);
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(0);
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           high_detail_args);
   DisableTracing();
 
   // Invert: enable mdp1 and disable mdp2.
   mdm_->UnregisterDumpProvider(&mdp1);
@@ skipping 12 matching lines @@
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                           high_detail_args);
   DisableTracing();
 }
 
 // Verify that whether OnMemoryDump is called depends only on the current
 // registration state and not on previous registrations and dumps.
 TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
   MockDumpProvider mdp;
+  EnableMemoryDumpProviderRegistrations();
 
   mdm_->RegisterDumpProvider(&mdp);
 
   {
     EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(1);
     EnableTracing(kTraceCategory);
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                             high_detail_args);
     DisableTracing();
   }
@@ skipping 35 matching lines @@
 // Checks that the MemoryDumpManager respects the thread affinity when a
 // MemoryDumpProvider specifies a task_runner(). The test starts creating 8
 // threads and registering a MemoryDumpProvider on each of them. At each
 // iteration, one thread is removed, to check the live unregistration logic.
 TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
   const uint32 kNumInitialThreads = 8;
 
   ScopedVector<Thread> threads;
   ScopedVector<MockDumpProvider> mdps;
 
+  EnableMemoryDumpProviderRegistrations();
+
   // Create the threads and setup the expectations. Given that at each iteration
   // we will pop out one thread/MemoryDumpProvider, each MDP is supposed to be
   // invoked a number of times equal to its index.
   for (uint32 i = kNumInitialThreads; i > 0; --i) {
     threads.push_back(new Thread("test thread"));
     threads.back()->Start();
     mdps.push_back(new MockDumpProvider(threads.back()->task_runner()));
     MockDumpProvider* mdp = mdps.back();
     mdm_->RegisterDumpProvider(mdp, threads.back()->task_runner());
     EXPECT_CALL(*mdp, OnMemoryDump(_, _))
         .Times(i)
         .WillRepeatedly(
             Invoke(mdp, &MockDumpProvider::OnMemoryDump_CheckTaskRunner));
   }
 
   EnableTracing(kTraceCategory);
 
+  DisableMemoryDumpProviderRegistrations();
+
   while (!threads.empty()) {
     last_callback_success_ = false;
     {
       RunLoop run_loop;
       MemoryDumpCallback callback =
           Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
                MessageLoop::current()->task_runner(), run_loop.QuitClosure());
       mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                               high_detail_args, callback);
       // This nested message loop (|run_loop|) will be quit if and only if
@@ skipping 21 matching lines @@
 
   DisableTracing();
 }
 
 // Enable both dump providers, make sure that mdp gets disabled after 3 failures
 // and not disabled after 1.
 TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
 
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp1);
   mdm_->RegisterDumpProvider(&mdp2);
   EnableTracing(kTraceCategory);
 
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
       .Times(MemoryDumpManager::kMaxConsecutiveFailuresCount)
       .WillRepeatedly(Return(false));
 
   EXPECT_CALL(mdp2, OnMemoryDump(_, _))
       .Times(1 + MemoryDumpManager::kMaxConsecutiveFailuresCount)
       .WillOnce(Return(false))
       .WillRepeatedly(Return(true));
   for (int i = 0; i < 1 + MemoryDumpManager::kMaxConsecutiveFailuresCount;
        i++) {
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                             high_detail_args);
   }
 
   DisableTracing();
 }
 
 // Sneakily register an extra memory dump provider while an existing one is
 // dumping and expect it to take part in the already active tracing session.
 TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
 
+  EnableMemoryDumpProviderRegistrations();
   mdp1.dump_provider_to_register_or_unregister = &mdp2;
   mdm_->RegisterDumpProvider(&mdp1);
   EnableTracing(kTraceCategory);
 
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
       .Times(4)
       .WillOnce(Return(true))
       .WillOnce(Invoke(
           &mdp1, &MockDumpProvider::OnMemoryDump_RegisterExtraDumpProvider))
       .WillRepeatedly(Return(true));
@@ skipping 10 matching lines @@
   }
 
   DisableTracing();
 }
 
 // Like the above, but suddenly unregister the dump provider.
 TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
 
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get());
   mdm_->RegisterDumpProvider(&mdp2, ThreadTaskRunnerHandle::Get());
   mdp1.dump_provider_to_register_or_unregister = &mdp2;
   EnableTracing(kTraceCategory);
 
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
       .Times(4)
       .WillOnce(Return(true))
       .WillOnce(
           Invoke(&mdp1, &MockDumpProvider::OnMemoryDump_UnregisterDumpProvider))
@@ skipping 15 matching lines @@
 
 // Verify that the dump does not abort when unregistering a provider while
 // dumping from a different thread than the dumping thread.
 TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) {
   ScopedVector<TestIOThread> threads;
   ScopedVector<MockDumpProvider> mdps;
 
   for (int i = 0; i < 2; i++) {
     threads.push_back(new TestIOThread(TestIOThread::kAutoStart));
     mdps.push_back(new MockDumpProvider(threads.back()->task_runner()));
+    EnableMemoryDumpProviderRegistrations();
     mdm_->RegisterDumpProvider(mdps.back(), threads.back()->task_runner());
+    DisableMemoryDumpProviderRegistrations();
   }
 
   int on_memory_dump_call_count = 0;
   RunLoop run_loop;
 
   // When OnMemoryDump is called on either of the dump providers, it will
   // unregister the other one.
   for (MockDumpProvider* mdp : mdps) {
     int other_idx = (mdps.front() == mdp);
     TestIOThread* other_thread = threads[other_idx];
@@ skipping 30 matching lines @@
   ASSERT_EQ(true, last_callback_success_);
 
   DisableTracing();
 }
 
 // Ensures that a NACK callback is invoked if RequestGlobalDump is called when
 // tracing is not enabled.
 TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) {
   MockDumpProvider mdp1;
 
+  EnableMemoryDumpProviderRegistrations();
   mdm_->RegisterDumpProvider(&mdp1);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(0);
 
   last_callback_success_ = true;
   {
     RunLoop run_loop;
     MemoryDumpCallback callback =
         Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
              MessageLoop::current()->task_runner(), run_loop.QuitClosure());
     mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
                             high_detail_args, callback);
     run_loop.Run();
   }
   EXPECT_FALSE(last_callback_success_);
 }
 
 }  // namespace trace_event
 }  // namespace base