Commit 97206c26 authored by primiano's avatar primiano Committed by Commit bot

[tracing] Fix MemoryDumpManager testing instance teardown pattern.

crrev.com/933613002 introduced a ODR violtaion by adding the
DeleteForTesting method (other definition in trace_event_impl.cc).
This was causing an ASan assertion (see crbug.com/459556) and failures
of base_unittests in debug builds (see crrev.com/933613002 #33).
This CL fixes the DeleteForTesting pattern removing the awkward ODR
violation.

BUG=459556,458295
TBR=nduca@chromium.org

Review URL: https://codereview.chromium.org/942803002

Cr-Commit-Position: refs/heads/master@{#317316}
parent 6b24b209
......@@ -10,21 +10,13 @@
#include "base/trace_event/memory_dump_provider.h"
#include "base/trace_event/process_memory_dump.h"
// TODO(primiano): in a separate CL rename DeleteTraceLogForTesting into
// something like base::internal::TeardownSingletonForTesting so we don't have
// to add a new friend to singleton each time.
class DeleteTraceLogForTesting {
public:
static void Delete() {
Singleton<
base::trace_event::MemoryDumpManager,
LeakySingletonTraits<base::trace_event::MemoryDumpManager>>::OnExit(0);
}
};
namespace base {
namespace trace_event {
namespace {
MemoryDumpManager* g_instance_for_testing = nullptr;
}
// TODO(primiano): this should be smarter and should do something similar to
// trace event synthetic delays.
const char MemoryDumpManager::kTraceCategory[] =
......@@ -32,13 +24,16 @@ const char MemoryDumpManager::kTraceCategory[] =
// static
MemoryDumpManager* MemoryDumpManager::GetInstance() {
if (g_instance_for_testing)
return g_instance_for_testing;
return Singleton<MemoryDumpManager,
LeakySingletonTraits<MemoryDumpManager>>::get();
}
// static
void MemoryDumpManager::DeleteForTesting() {
DeleteTraceLogForTesting::Delete();
void MemoryDumpManager::SetInstanceForTesting(MemoryDumpManager* instance) {
g_instance_for_testing = instance;
}
MemoryDumpManager::MemoryDumpManager() : memory_tracing_enabled_(0) {
......
......@@ -50,17 +50,17 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
void OnTraceLogDisabled() override;
private:
friend struct DefaultDeleter<MemoryDumpManager>; // For the testing instance.
friend struct DefaultSingletonTraits<MemoryDumpManager>;
friend class MemoryDumpManagerTest;
static const char kTraceCategory[];
static void SetInstanceForTesting(MemoryDumpManager* instance);
MemoryDumpManager();
virtual ~MemoryDumpManager();
// Tears down the singleton instance.
static void DeleteForTesting();
// Broadcasts the dump requests to the other processes.
void BroadcastDumpRequest();
......
......@@ -17,14 +17,16 @@ namespace trace_event {
class MemoryDumpManagerTest : public testing::Test {
public:
void SetUp() override {
mdm_.reset(new MemoryDumpManager());
MemoryDumpManager::SetInstanceForTesting(mdm_.get());
ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance());
MemoryDumpManager::GetInstance()->Initialize();
mdm_ = MemoryDumpManager::GetInstance();
}
void TearDown() override {
MemoryDumpManager::DeleteForTesting();
MemoryDumpManager::SetInstanceForTesting(nullptr);
mdm_.reset();
TraceLog::DeleteForTesting();
mdm_ = NULL;
}
protected:
......@@ -37,7 +39,7 @@ class MemoryDumpManagerTest : public testing::Test {
void DisableTracing() { TraceLog::GetInstance()->SetDisabled(); }
MemoryDumpManager* mdm_;
scoped_ptr<MemoryDumpManager> mdm_;
private:
// We want our singleton torn down after each test.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment