From ba4ca8421832350dd3335a8bd7abea9f0e1311b6 Mon Sep 17 00:00:00 2001 From: primiano Date: Thu, 10 Sep 2015 09:50:59 -0700 Subject: [PATCH] [tracing] Fix MemoryDumpManager to support startup tracing This CL simplifies a lot the design of the MemoryDumpManager (MDM) initialization (\o/). Before this change the MDM had a two-stage initialization: 1. Initialize() that was registering to the TraceLog enabled stateobserver; 2. SetDelegate() that was setting the inversion-of-control delegate. This design was fragile as it required to think about the two possible sequences of events (1 -> 2 or 2 -> 1) and not really necessary. The simplification herein introduced consists in: - Drop the IsCoordinatorProcess() method from the delegate. That information is not supposed to change over time and is known at MDM initialization time. - Merge the SetDelegate() into the Initialize() call. There is no point allowing to initialize the MDM without a delegate, so why bother? Moved all the initialization to the point where the delegate (concretelly the TracingControllerImpl) is ready. As a side effect, startup tracing now works properly (tests are coming in a separate CL). BUG=524057 TEST=--trace-startup=-*,disabled-by-default-memory-infra --trace-startup-file=/tmp/startup.json Review URL: https://codereview.chromium.org/1333873002 Cr-Commit-Position: refs/heads/master@{#348168} --- base/trace_event/memory_dump_manager.cc | 62 +++++++-------- base/trace_event/memory_dump_manager.h | 29 +++---- base/trace_event/memory_dump_manager_unittest.cc | 88 +++++++++++++--------- .../child_memory_dump_manager_delegate_impl.cc | 10 +-- .../child_memory_dump_manager_delegate_impl.h | 1 - content/browser/browser_main_loop.cc | 4 +- .../browser/tracing/memory_tracing_browsertest.cc | 9 ++- content/browser/tracing/tracing_controller_impl.cc | 7 +- content/browser/tracing/tracing_controller_impl.h | 1 - content/child/child_thread_impl.cc | 3 - 10 files changed, 109 insertions(+), 105 deletions(-) diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index 55c4748a685f..7fb882557615 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc @@ -106,6 +106,7 @@ void MemoryDumpManager::SetInstanceForTesting(MemoryDumpManager* instance) { MemoryDumpManager::MemoryDumpManager() : delegate_(nullptr), + is_coordinator_(false), memory_tracing_enabled_(0), tracing_process_id_(kInvalidTracingProcessId), skip_core_dumpers_auto_registration_for_testing_(false), @@ -117,10 +118,15 @@ MemoryDumpManager::~MemoryDumpManager() { TraceLog::GetInstance()->RemoveEnabledStateObserver(this); } -void MemoryDumpManager::Initialize() { - TRACE_EVENT0(kTraceCategory, "init"); // Add to trace-viewer category list. - - TraceLog::GetInstance()->AddEnabledStateObserver(this); +void MemoryDumpManager::Initialize(MemoryDumpManagerDelegate* delegate, + bool is_coordinator) { + { + AutoLock lock(lock_); + DCHECK(delegate); + DCHECK(!delegate_); + delegate_ = delegate; + is_coordinator_ = is_coordinator; + } // Enable the core dump providers. if (!skip_core_dumpers_auto_registration_for_testing_) { @@ -141,12 +147,14 @@ void MemoryDumpManager::Initialize() { RegisterDumpProvider(WinHeapDumpProvider::GetInstance()); #endif } // !skip_core_dumpers_auto_registration_for_testing_ -} -void MemoryDumpManager::SetDelegate(MemoryDumpManagerDelegate* delegate) { - AutoLock lock(lock_); - DCHECK_EQ(static_cast(nullptr), delegate_); - delegate_ = delegate; + // If tracing was enabled before initializing MemoryDumpManager, we missed the + // OnTraceLogEnabled() event. Synthetize it so we can late-join the party. + bool is_tracing_already_enabled = TraceLog::GetInstance()->IsEnabled(); + TRACE_EVENT0(kTraceCategory, "init"); // Add to trace-viewer category list. + TraceLog::GetInstance()->AddEnabledStateObserver(this); + if (is_tracing_already_enabled) + OnTraceLogEnabled(); } void MemoryDumpManager::RegisterDumpProvider( @@ -210,22 +218,21 @@ void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type, const uint64 guid = TraceLog::GetInstance()->MangleEventId(g_next_guid.GetNext()); - // The delegate_ is supposed to be thread safe, immutable and long lived. - // No need to keep the lock after we ensured that a delegate has been set. + // Technically there is no need to grab the |lock_| here as the delegate is + // long-lived and can only be set by Initialize(), which is locked and + // necessarily happens before memory_tracing_enabled_ == true. + // Not taking the |lock_|, though, is lakely make TSan barf and, at this point + // (memory-infra is enabled) we're not in the fast-path anymore. MemoryDumpManagerDelegate* delegate; { AutoLock lock(lock_); delegate = delegate_; } - if (delegate) { - // The delegate is in charge to coordinate the request among all the - // processes and call the CreateLocalDumpPoint on the local process. - MemoryDumpRequestArgs args = {guid, dump_type, dump_args}; - delegate->RequestGlobalMemoryDump(args, callback); - } else if (!callback.is_null()) { - callback.Run(guid, false /* success */); - } + // The delegate will coordinate the IPC broadcast and at some point invoke + // CreateProcessDump() to get a dump for the current process. + MemoryDumpRequestArgs args = {guid, dump_type, dump_args}; + delegate->RequestGlobalMemoryDump(args, callback); } void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type, @@ -394,11 +401,10 @@ void MemoryDumpManager::AbortDumpLocked( } void MemoryDumpManager::OnTraceLogEnabled() { - // TODO(primiano): at this point we query TraceLog::GetCurrentCategoryFilter - // to figure out (and cache) which dumpers should be enabled or not. - // For the moment piggy back everything on the generic "memory" category. bool enabled; TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &enabled); + if (!enabled) + return; // Initialize the TraceLog for the current thread. This is to avoid that the // TraceLog memory dump provider is registered lazily in the PostTask() below @@ -407,13 +413,7 @@ void MemoryDumpManager::OnTraceLogEnabled() { AutoLock lock(lock_); - // There is no point starting the tracing without a delegate. - if (!enabled || !delegate_) { - // Disable all the providers. - for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it) - it->disabled = true; - return; - } + DCHECK(delegate_); // At this point we must have a delegate. session_state_ = new MemoryDumpSessionState(); for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it) { @@ -425,9 +425,9 @@ void MemoryDumpManager::OnTraceLogEnabled() { // TODO(primiano): This is a temporary hack to disable periodic memory dumps // when running memory benchmarks until telemetry uses TraceConfig to - // enable/disable periodic dumps. + // enable/disable periodic dumps. See crbug.com/529184 . // The same mechanism should be used to disable periodic dumps in tests. - if (!delegate_->IsCoordinatorProcess() || + if (!is_coordinator_ || CommandLine::ForCurrentProcess()->HasSwitch( "enable-memory-benchmarking") || disable_periodic_dumps_for_testing_) { diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h index 760ad7b871de..1092a264830a 100644 --- a/base/trace_event/memory_dump_manager.h +++ b/base/trace_event/memory_dump_manager.h @@ -40,12 +40,19 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { static MemoryDumpManager* GetInstance(); - // Invoked once per process to register the TraceLog observer. - void Initialize(); - - // See the lifetime and thread-safety requirements on the delegate below in - // the |MemoryDumpManagerDelegate| docstring. - void SetDelegate(MemoryDumpManagerDelegate* delegate); + // Invoked once per process to listen to trace begin / end events. + // Initialization can happen after (Un)RegisterMemoryDumpProvider() calls + // and the MemoryDumpManager guarantees to support this. + // On the other side, the MemoryDumpManager will not be fully operational + // (i.e. will NACK any RequestGlobalMemoryDump()) until initialized. + // Arguments: + // is_coordinator: if true this MemoryDumpManager instance will act as a + // coordinator and schedule periodic dumps (if enabled via TraceConfig); + // false when the MemoryDumpManager is initialized in a slave process. + // delegate: inversion-of-control interface for embedder-specific behaviors + // (multiprocess handshaking). See the lifetime and thread-safety + // requirements in the |MemoryDumpManagerDelegate| docstring. + void Initialize(MemoryDumpManagerDelegate* delegate, bool is_coordinator); // MemoryDumpManager does NOT take memory ownership of |mdp|, which is // expected to either be a singleton or unregister itself. @@ -108,9 +115,6 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { friend struct DefaultSingletonTraits; friend class MemoryDumpManagerDelegate; friend class MemoryDumpManagerTest; - FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, DisableFailingDumpers); - FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, - UnregisterDumperFromThreadWhileDumping); // Descriptor struct used to hold information about registered MDPs. It is // deliberately copyable, in order to allow it to be used as std::set value. @@ -208,6 +212,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { MemoryDumpManagerDelegate* delegate_; // Not owned. + // When true, this instance is in charge of coordinating periodic dumps. + bool is_coordinator_; + // Protects from concurrent accesses to the |dump_providers_*| and |delegate_| // to guard against disabling logging while dumping on another thread. Lock lock_; @@ -241,10 +248,6 @@ class BASE_EXPORT MemoryDumpManagerDelegate { virtual void RequestGlobalMemoryDump(const MemoryDumpRequestArgs& args, const MemoryDumpCallback& callback) = 0; - // Determines whether the MemoryDumpManager instance should be the master - // (the ones which initiates and coordinates the multiprocess dumps) or not. - virtual bool IsCoordinatorProcess() const = 0; - // Returns tracing process id of the current process. This is used by // MemoryDumpManager::GetTracingProcessId. virtual uint64 GetTracingProcessId() const = 0; diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc index 62a5c62d2393..4510cfff8229 100644 --- a/base/trace_event/memory_dump_manager_unittest.cc +++ b/base/trace_event/memory_dump_manager_unittest.cc @@ -29,6 +29,10 @@ namespace trace_event { namespace { MemoryDumpArgs g_high_detail_args = {MemoryDumpArgs::LevelOfDetail::HIGH}; MemoryDumpArgs g_low_detail_args = {MemoryDumpArgs::LevelOfDetail::LOW}; +const char kTraceConfigWithTriggersFmt[] = + "{\"included_categories\":[\"%s\"],\"memory_dump_config\":{\"triggers\":[" + "{\"mode\":\"light\", \"periodic_interval_ms\":1}," + "{\"mode\":\"detailed\", \"periodic_interval_ms\":5}]}}"; } // namespace // GTest matchers for MemoryDumpRequestArgs arguments. @@ -54,7 +58,6 @@ MATCHER(IsLowDetailArgs, "") { class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate { public: MemoryDumpManagerDelegateForTesting() { - EXPECT_CALL(*this, IsCoordinatorProcess()).WillRepeatedly(Return(false)); ON_CALL(*this, RequestGlobalMemoryDump(_, _)) .WillByDefault(Invoke( this, &MemoryDumpManagerDelegateForTesting::CreateProcessDump)); @@ -64,8 +67,6 @@ class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate { void(const MemoryDumpRequestArgs& args, const MemoryDumpCallback& callback)); - MOCK_CONST_METHOD0(IsCoordinatorProcess, bool()); - uint64 GetTracingProcessId() const override { NOTREACHED(); return MemoryDumpManager::kInvalidTracingProcessId; @@ -86,9 +87,7 @@ class MemoryDumpManagerTest : public testing::Test { mdm_.reset(new MemoryDumpManager()); MemoryDumpManager::SetInstanceForTesting(mdm_.get()); ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance()); - mdm_->Initialize(); delegate_.reset(new MemoryDumpManagerDelegateForTesting); - mdm_->SetDelegate(delegate_.get()); } void TearDown() override { @@ -108,7 +107,10 @@ class MemoryDumpManagerTest : public testing::Test { } protected: - // This enables tracing using the legacy category filter string. + void InitializeMemoryDumpManager(bool is_coordinator) { + mdm_->Initialize(delegate_.get(), is_coordinator); + } + void EnableTracingWithLegacyCategories(const char* category) { TraceLog::GetInstance()->SetEnabled(TraceConfig(category, ""), TraceLog::RECORDING_MODE); @@ -125,6 +127,10 @@ class MemoryDumpManagerTest : public testing::Test { return mdm_->periodic_dump_timer_.IsRunning(); } + int GetMaxConsecutiveFailuresCount() const { + return MemoryDumpManager::kMaxConsecutiveFailuresCount; + } + scoped_ptr mdm_; scoped_ptr delegate_; bool last_callback_success_; @@ -139,6 +145,7 @@ class MemoryDumpManagerTest : public testing::Test { // Basic sanity checks. Registers a memory dump provider and checks that it is // called, but only when memory-infra is enabled. TEST_F(MemoryDumpManagerTest, SingleDumper) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp; mdm_->RegisterDumpProvider(&mdp); @@ -175,6 +182,7 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) { // Checks that requesting dumps with high level of detail actually propagates // the level of the detail properly to OnMemoryDump() call on dump providers. TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp; mdm_->RegisterDumpProvider(&mdp); @@ -200,6 +208,7 @@ TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) { // Checks that the SharedSessionState object is acqually shared over time. TEST_F(MemoryDumpManagerTest, SharedSessionState) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; mdm_->RegisterDumpProvider(&mdp1); @@ -232,6 +241,7 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) { // Checks that the (Un)RegisterDumpProvider logic behaves sanely. TEST_F(MemoryDumpManagerTest, MultipleDumpers) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; @@ -270,6 +280,7 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) { // Checks that the dump provider invocations depend only on the current // registration state and not on previous registrations and dumps. TEST_F(MemoryDumpManagerTest, RegistrationConsistency) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp; mdm_->RegisterDumpProvider(&mdp); @@ -325,6 +336,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) { // 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) { + InitializeMemoryDumpManager(false /* is_coordinator */); const uint32 kNumInitialThreads = 8; ScopedVector threads; @@ -391,6 +403,7 @@ TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) { // Checks that providers get disabled after 3 consecutive failures, but not // otherwise (e.g., if interleaved). TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; @@ -398,11 +411,11 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) { mdm_->RegisterDumpProvider(&mdp2); EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); - const int kNumDumps = 2 * MemoryDumpManager::kMaxConsecutiveFailuresCount; + const int kNumDumps = 2 * GetMaxConsecutiveFailuresCount(); EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(kNumDumps); EXPECT_CALL(mdp1, OnMemoryDump(_, _)) - .Times(MemoryDumpManager::kMaxConsecutiveFailuresCount) + .Times(GetMaxConsecutiveFailuresCount()) .WillRepeatedly(Return(false)); EXPECT_CALL(mdp2, OnMemoryDump(_, _)) @@ -424,6 +437,7 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) { // Sneakily registers 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) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; @@ -458,6 +472,7 @@ TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) { // Like RegisterDumperWhileDumping, but unregister the dump provider instead. TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; @@ -494,6 +509,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) { // Checks that the dump does not abort when unregistering a provider while // dumping from a different thread than the dumping thread. TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) { + InitializeMemoryDumpManager(false /* is_coordinator */); ScopedVector threads; ScopedVector mdps; @@ -534,9 +550,10 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) { MessageLoop::current()->task_runner(), run_loop.QuitClosure()); EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); - MemoryDumpRequestArgs request_args = {0, MemoryDumpType::EXPLICITLY_TRIGGERED, - g_high_detail_args}; - mdm_->CreateProcessDump(request_args, callback); + + EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(1); + mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED, + g_high_detail_args, callback); run_loop.Run(); @@ -549,6 +566,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) { // Checks that a NACK callback is invoked if RequestGlobalDump() is called when // tracing is not enabled. TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) { + InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; mdm_->RegisterDumpProvider(&mdp1); @@ -568,10 +586,12 @@ TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) { EXPECT_FALSE(last_callback_success_); } -// This test crystallizes the expectations of the chrome://tracing UI and -// chrome telemetry w.r.t. periodic dumps in memory-infra, handling gracefully -// the transition between the legacy and the new-style (JSON-based) TraceConfig. +// This test (and the MemoryDumpManagerTestCoordinator below) crystallizes the +// expectations of the chrome://tracing UI and chrome telemetry w.r.t. periodic +// dumps in memory-infra, handling gracefully the transition between the legacy +// and the new-style (JSON-based) TraceConfig. TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { + InitializeMemoryDumpManager(false /* is_coordinator */); MemoryDumpManagerDelegateForTesting& delegate = *delegate_; // Don't trigger the default behavior of the mock delegate in this test, @@ -583,14 +603,27 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { // Enabling memory-infra in a non-coordinator process should not trigger any // periodic dumps. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(false)); EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); EXPECT_FALSE(IsPeriodicDumpingEnabled()); DisableTracing(); + // Enabling memory-infra with the new (JSON) TraceConfig in a non-coordinator + // process with a fully defined trigger config should NOT enable any periodic + // dumps. + const std::string kTraceConfigWithTriggers = StringPrintf( + kTraceConfigWithTriggersFmt, MemoryDumpManager::kTraceCategory); + EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str()); + EXPECT_FALSE(IsPeriodicDumpingEnabled()); + DisableTracing(); +} + +TEST_F(MemoryDumpManagerTest, TraceConfigExpectationsWhenIsCoordinator) { + InitializeMemoryDumpManager(true /* is_coordinator */); + MemoryDumpManagerDelegateForTesting& delegate = *delegate_; + ON_CALL(delegate, RequestGlobalMemoryDump(_, _)).WillByDefault(Return()); + // Enabling memory-infra with the legacy TraceConfig (category filter) in // a coordinator process should enable periodic dumps. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true)); EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); EXPECT_TRUE(IsPeriodicDumpingEnabled()); DisableTracing(); @@ -599,7 +632,6 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { // process without specifying any "memory_dump_config" section should enable // periodic dumps. This is to preserve the behavior chrome://tracing UI, that // is: ticking memory-infra should dump periodically with the default config. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true)); const std::string kTraceConfigWithNoMemorDumpConfig = StringPrintf( "{\"included_categories\":[\"%s\"]}", MemoryDumpManager::kTraceCategory); EnableTracingWithTraceConfig(kTraceConfigWithNoMemorDumpConfig.c_str()); @@ -610,7 +642,6 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { // process with an empty "memory_dump_config" should NOT enable periodic // dumps. This is the way telemetry is supposed to use memory-infra with // only explicitly triggered dumps. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true)); const std::string kTraceConfigWithNoTriggers = StringPrintf( "{\"included_categories\":[\"%s\"], \"memory_dump_config\":{}", MemoryDumpManager::kTraceCategory); @@ -618,28 +649,9 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { EXPECT_FALSE(IsPeriodicDumpingEnabled()); DisableTracing(); - // Enabling memory-infra with the new (JSON) TraceConfig in a non-coordinator - // process with a fully defined trigger config should NOT enable any periodic - // dumps. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(false)); - const std::string kTraceConfigWithTriggers = StringPrintf( - "{\"included_categories\":[\"%s\"]," - "\"memory_dump_config\":{" - "\"triggers\":[" - "{\"mode\":\"light\", \"periodic_interval_ms\":1}," - "{\"mode\":\"detailed\", \"periodic_interval_ms\":5}" - "]" - "}" - "}", MemoryDumpManager::kTraceCategory); - EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str()); - EXPECT_FALSE(IsPeriodicDumpingEnabled()); - DisableTracing(); - // Enabling memory-infra with the new (JSON) TraceConfig in a coordinator // process with a fully defined trigger config should cause periodic dumps to // be performed in the correct order. - EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true)); - RunLoop run_loop; auto quit_closure = run_loop.QuitClosure(); @@ -658,6 +670,8 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) { // Swallow all the final spurious calls until tracing gets disabled. EXPECT_CALL(delegate, RequestGlobalMemoryDump(_, _)).Times(AnyNumber()); + const std::string kTraceConfigWithTriggers = StringPrintf( + kTraceConfigWithTriggersFmt, MemoryDumpManager::kTraceCategory); EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str()); run_loop.Run(); DisableTracing(); diff --git a/components/tracing/child_memory_dump_manager_delegate_impl.cc b/components/tracing/child_memory_dump_manager_delegate_impl.cc index c32222df1622..2b537c98bef4 100644 --- a/components/tracing/child_memory_dump_manager_delegate_impl.cc +++ b/components/tracing/child_memory_dump_manager_delegate_impl.cc @@ -21,11 +21,11 @@ ChildMemoryDumpManagerDelegateImpl::ChildMemoryDumpManagerDelegateImpl() : ctmf_(nullptr), tracing_process_id_( base::trace_event::MemoryDumpManager::kInvalidTracingProcessId) { - base::trace_event::MemoryDumpManager::GetInstance()->SetDelegate(this); + base::trace_event::MemoryDumpManager::GetInstance()->Initialize( + this /* delegate */, false /* is_coordinator */); } -ChildMemoryDumpManagerDelegateImpl::~ChildMemoryDumpManagerDelegateImpl() { -} +ChildMemoryDumpManagerDelegateImpl::~ChildMemoryDumpManagerDelegateImpl() {} void ChildMemoryDumpManagerDelegateImpl::SetChildTraceMessageFilter( ChildTraceMessageFilter* ctmf) { @@ -70,10 +70,6 @@ void ChildMemoryDumpManagerDelegateImpl::RequestGlobalMemoryDump( ctmf_->SendGlobalMemoryDumpRequest(args, callback); } -bool ChildMemoryDumpManagerDelegateImpl::IsCoordinatorProcess() const { - return false; -} - uint64 ChildMemoryDumpManagerDelegateImpl::GetTracingProcessId() const { return tracing_process_id_; } diff --git a/components/tracing/child_memory_dump_manager_delegate_impl.h b/components/tracing/child_memory_dump_manager_delegate_impl.h index c83fce22fd94..099948e64185 100644 --- a/components/tracing/child_memory_dump_manager_delegate_impl.h +++ b/components/tracing/child_memory_dump_manager_delegate_impl.h @@ -33,7 +33,6 @@ class ChildMemoryDumpManagerDelegateImpl void RequestGlobalMemoryDump( const base::trace_event::MemoryDumpRequestArgs& args, const base::trace_event::MemoryDumpCallback& callback) override; - bool IsCoordinatorProcess() const override; uint64 GetTracingProcessId() const override; void SetChildTraceMessageFilter(ChildTraceMessageFilter* ctmf); diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc index b8c41e07d787..eb294ef5a467 100644 --- a/content/browser/browser_main_loop.cc +++ b/content/browser/browser_main_loop.cc @@ -642,9 +642,7 @@ void BrowserMainLoop::PostMainMessageLoopStart() { DOMStorageArea::EnableAggressiveCommitDelay(); } - base::trace_event::MemoryDumpManager::GetInstance()->Initialize(); - - // Enable the dump providers. + // Enable memory-infra dump providers. base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( HostSharedBitmapManager::current()); base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( diff --git a/content/browser/tracing/memory_tracing_browsertest.cc b/content/browser/tracing/memory_tracing_browsertest.cc index 90ddb5bf3b9e..8b0a1a9e8ebc 100644 --- a/content/browser/tracing/memory_tracing_browsertest.cc +++ b/content/browser/tracing/memory_tracing_browsertest.cc @@ -68,13 +68,14 @@ class MemoryTracingTest : public ContentBrowserTest { callback_call_count_ = 0; last_callback_dump_guid_ = 0; last_callback_success_ = false; - MemoryDumpManager::GetInstance()->Initialize(); - mock_dump_provider_.reset(new MockDumpProvider()); - MemoryDumpManager::GetInstance()->RegisterDumpProvider( - mock_dump_provider_.get()); + // TODO(primiano): This should be done via TraceConfig. // See https://goo.gl/5Hj3o0. MemoryDumpManager::GetInstance()->DisablePeriodicDumpsForTesting(); + + mock_dump_provider_.reset(new MockDumpProvider()); + MemoryDumpManager::GetInstance()->RegisterDumpProvider( + mock_dump_provider_.get()); ContentBrowserTest::SetUp(); } diff --git a/content/browser/tracing/tracing_controller_impl.cc b/content/browser/tracing/tracing_controller_impl.cc index d69cab97c67f..708841bf40ef 100644 --- a/content/browser/tracing/tracing_controller_impl.cc +++ b/content/browser/tracing/tracing_controller_impl.cc @@ -59,7 +59,8 @@ TracingControllerImpl::TracingControllerImpl() is_recording_(TraceLog::GetInstance()->IsEnabled()), is_monitoring_(false), is_power_tracing_(false) { - base::trace_event::MemoryDumpManager::GetInstance()->SetDelegate(this); + base::trace_event::MemoryDumpManager::GetInstance()->Initialize( + this /* delegate */, true /* is_coordinator */); // Deliberately leaked, like this class. base::FileTracing::SetProvider(new FileTracingProviderImpl); @@ -846,10 +847,6 @@ void TracingControllerImpl::RequestGlobalMemoryDump( tmf->SendProcessMemoryDumpRequest(args); } -bool TracingControllerImpl::IsCoordinatorProcess() const { - return true; -} - uint64 TracingControllerImpl::GetTracingProcessId() const { return ChildProcessHost::kBrowserTracingProcessId; } diff --git a/content/browser/tracing/tracing_controller_impl.h b/content/browser/tracing/tracing_controller_impl.h index ad6126acea75..9af728ef0697 100644 --- a/content/browser/tracing/tracing_controller_impl.h +++ b/content/browser/tracing/tracing_controller_impl.h @@ -59,7 +59,6 @@ class TracingControllerImpl void RequestGlobalMemoryDump( const base::trace_event::MemoryDumpRequestArgs& args, const base::trace_event::MemoryDumpCallback& callback) override; - bool IsCoordinatorProcess() const override; uint64 GetTracingProcessId() const override; class TraceMessageFilterObserver { diff --git a/content/child/child_thread_impl.cc b/content/child/child_thread_impl.cc index 8f0d93c67c36..08c2075f7fae 100644 --- a/content/child/child_thread_impl.cc +++ b/content/child/child_thread_impl.cc @@ -28,7 +28,6 @@ #include "base/synchronization/lock.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread_local.h" -#include "base/trace_event/memory_dump_manager.h" #include "base/tracked_objects.h" #include "components/tracing/child_trace_message_filter.h" #include "content/child/child_discardable_shared_memory_manager.h" @@ -495,8 +494,6 @@ void ChildThreadImpl::Init(const Options& options) { ::HeapProfilerStop, ::GetHeapProfile)); #endif - base::trace_event::MemoryDumpManager::GetInstance()->Initialize(); - shared_bitmap_manager_.reset( new ChildSharedBitmapManager(thread_safe_sender())); -- 2.11.4.GIT