Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1455)

Unified Diff: components/tracing/common/process_metrics_memory_dump_provider_unittest.cc

Issue 2753723003: Fix crash in InvokeOnMemoryDump when tracing (Closed)
Patch Set: be more explicit Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/tracing/common/process_metrics_memory_dump_provider_unittest.cc
diff --git a/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc b/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc
index 95a565d148ab8b502af840f0339672c10eb4d3c8..f257527c6b5c87ec42b26d7a74630ed8d78d5bbe 100644
--- a/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc
+++ b/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc
@@ -11,10 +11,12 @@
#include "base/files/file_util.h"
#include "base/memory/ptr_util.h"
#include "base/process/process_metrics.h"
+#include "base/trace_event/memory_dump_manager.h"
#include "base/trace_event/process_memory_dump.h"
#include "base/trace_event/process_memory_maps.h"
#include "base/trace_event/process_memory_totals.h"
#include "base/trace_event/trace_event_argument.h"
+#include "testing/gmock/include/gmock/gmock.h"
Primiano Tucci (use gerrit) 2017/03/15 12:44:40 I think this is a leftover, right? you are not usi
hjd 2017/03/15 14:23:12 Done.
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_MACOSX)
@@ -393,4 +395,52 @@ TEST(ProcessMetricsMemoryDumpProviderTest, NoDuplicateRegions) {
#endif // defined(OS_MACOSX)
+class MockMemoryDumpProvider : public ProcessMetricsMemoryDumpProvider {
+ public:
+ MOCK_METHOD2(OnMemoryDump,
Primiano Tucci (use gerrit) 2017/03/15 12:44:40 are you really using this mocked method? I think t
hjd 2017/03/15 14:23:12 Done.
+ bool(const base::trace_event::MemoryDumpArgs& args,
+ base::trace_event::ProcessMemoryDump* pmd));
+
+ MockMemoryDumpProvider(base::ProcessId process);
+ ~MockMemoryDumpProvider() override;
+};
+
+std::unordered_set<MockMemoryDumpProvider*> live_mocks;
Primiano Tucci (use gerrit) 2017/03/15 12:44:40 nit: g_ -> g_live_mocks (same with dead) as these
hjd 2017/03/15 14:23:12 Done.
+std::unordered_set<MockMemoryDumpProvider*> dead_mocks;
+
+MockMemoryDumpProvider::MockMemoryDumpProvider(base::ProcessId process)
+ : ProcessMetricsMemoryDumpProvider(process) {
+ live_mocks.insert(this);
+}
+
+MockMemoryDumpProvider::~MockMemoryDumpProvider() {
+ // ASSERT_TRUE(live_mocks.find(this) != live_mocks.end());
+ live_mocks.erase(this);
+ dead_mocks.insert(this);
+}
+
+TEST(ProcessMetricsMemoryDumpProviderTest, DoubleRegister) {
+ auto factory = [](base::ProcessId process) {
+ return std::unique_ptr<ProcessMetricsMemoryDumpProvider>(
+ new MockMemoryDumpProvider(process));
+ };
+ ProcessMetricsMemoryDumpProvider::factory_for_testing = factory;
+ ProcessMetricsMemoryDumpProvider::RegisterForProcess(1);
+ ProcessMetricsMemoryDumpProvider::RegisterForProcess(1);
+ EXPECT_EQ(1u, live_mocks.size());
Primiano Tucci (use gerrit) 2017/03/15 12:44:40 I'd make this an ASSERT_EQ and remove the if(...==
hjd 2017/03/15 14:23:12 Done.
+ EXPECT_EQ(1u, dead_mocks.size());
+ auto* manager = base::trace_event::MemoryDumpManager::GetInstance();
+ if (live_mocks.size() == 1u) {
+ MockMemoryDumpProvider* live_mock = *live_mocks.begin();
+ EXPECT_TRUE(manager->IsDumpProviderRegisteredForTesting(live_mock));
+ }
+ if (dead_mocks.size() == 1u) {
+ auto* dead_mock = *dead_mocks.begin();
+ EXPECT_FALSE(manager->IsDumpProviderRegisteredForTesting(dead_mock));
+ }
+ ProcessMetricsMemoryDumpProvider::UnregisterForProcess(1);
+ live_mocks.clear();
+ dead_mocks.clear();
+}
+
} // namespace tracing

Powered by Google App Engine
This is Rietveld 408576698