MemoryInfra: Remove Summary mode whitelist

The summary and background mode use the same set of MDPs now.
So, remove duplicate list.

BUG=730783

Change-Id: I3568d797d5bab3d36df658feb22df28d8ea786a4
Reviewed-on: https://chromium-review.googlesource.com/961554
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543553}

base/trace_event/memory_dump_manager.cc

@@ -385,18 +385,15 @@ void MemoryDumpManager::RegisterDumpProviderInternal(
   if (dumper_registrations_ignored_for_testing_)
     return;
 
-  // A handful of MDPs are required to compute the summary struct these are
-  // 'whitelisted for summary mode'. These MDPs are a subset of those which
+  // Only a handful of MDPs are required to compute the memory metrics. These
   // have small enough performance overhead that it is resonable to run them
   // in the background while the user is doing other things. Those MDPs are
   // 'whitelisted for background mode'.
   bool whitelisted_for_background_mode = IsMemoryDumpProviderWhitelisted(name);
-  bool whitelisted_for_summary_mode =
-      IsMemoryDumpProviderWhitelistedForSummary(name);
 
-  scoped_refptr<MemoryDumpProviderInfo> mdpinfo = new MemoryDumpProviderInfo(
-      mdp, name, std::move(task_runner), options,
-      whitelisted_for_background_mode, whitelisted_for_summary_mode);
+  scoped_refptr<MemoryDumpProviderInfo> mdpinfo =
+      new MemoryDumpProviderInfo(mdp, name, std::move(task_runner), options,
+                                 whitelisted_for_background_mode);
 
   if (options.is_fast_polling_supported) {
     DCHECK(!mdpinfo->task_runner) << "MemoryDumpProviders capable of fast "
@@ -597,7 +594,11 @@ void MemoryDumpManager::ContinueAsyncProcessDump(
     MemoryDumpProviderInfo* mdpinfo =
         pmd_async_state->pending_dump_providers.back().get();
 
-    if (!IsDumpProviderAllowedToDump(pmd_async_state->req_args, *mdpinfo)) {
+    // If we are in background mode, we should invoke only the whitelisted
+    // providers. Ignore other providers and continue.
+    if (pmd_async_state->req_args.level_of_detail ==
+            MemoryDumpLevelOfDetail::BACKGROUND &&
+        !mdpinfo->whitelisted_for_background_mode) {
       pmd_async_state->pending_dump_providers.pop_back();
       continue;
     }
@@ -647,26 +648,6 @@ void MemoryDumpManager::ContinueAsyncProcessDump(
   FinishAsyncProcessDump(std::move(pmd_async_state));
 }
 
-bool MemoryDumpManager::IsDumpProviderAllowedToDump(
-    const MemoryDumpRequestArgs& req_args,
-    const MemoryDumpProviderInfo& mdpinfo) const {
-  // If we are in background tracing, we should invoke only the whitelisted
-  // providers. Ignore other providers and continue.
-  if (req_args.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND &&
-      !mdpinfo.whitelisted_for_background_mode) {
-    return false;
-  }
-
-  // If we are in summary mode, we only need to invoke the providers
-  // whitelisted for summary mode.
-  if (req_args.dump_type == MemoryDumpType::SUMMARY_ONLY &&
-      !mdpinfo.whitelisted_for_summary_mode) {
-    return false;
-  }
-
-  return true;
-}
-
 // This function is called on the right task runner for current MDP. It is
 // either the task runner specified by MDP or |dump_thread_task_runner| if the
 // MDP did not specify task runner. Invokes the dump provider's OnMemoryDump()

base/trace_event/memory_dump_manager.h

@@ -252,10 +252,6 @@ class BASE_EXPORT MemoryDumpManager {
   void ContinueAsyncProcessDump(
       ProcessMemoryDumpAsyncState* owned_pmd_async_state);
 
-  // Returns true if the given dump type and mode allows the given MDP to dump.
-  bool IsDumpProviderAllowedToDump(const MemoryDumpRequestArgs& req_args,
-                                   const MemoryDumpProviderInfo& mdpinfo) const;
-
   // Invokes OnMemoryDump() of the given MDP. Should be called on the MDP task
   // runner.
   void InvokeOnMemoryDump(MemoryDumpProviderInfo* mdpinfo,

base/trace_event/memory_dump_manager_unittest.cc

@@ -59,12 +59,7 @@ namespace {
 
 const char* kMDPName = "TestDumpProvider";
 const char* kWhitelistedMDPName = "WhitelistedTestDumpProvider";
-const char* kBackgroundButNotSummaryWhitelistedMDPName =
-    "BackgroundButNotSummaryWhitelistedTestDumpProvider";
-const char* const kTestMDPWhitelist[] = {
-    kWhitelistedMDPName, kBackgroundButNotSummaryWhitelistedMDPName, nullptr};
-const char* const kTestMDPWhitelistForSummary[] = {kWhitelistedMDPName,
-                                                   nullptr};
+const char* const kTestMDPWhitelist[] = {kWhitelistedMDPName, nullptr};
 
 void RegisterDumpProvider(
     MemoryDumpProvider* mdp,
@@ -752,23 +747,17 @@ TEST_F(MemoryDumpManagerTest, TriggerDumpWithoutTracing) {
                                         MemoryDumpLevelOfDetail::DETAILED));
 }
 
-TEST_F(MemoryDumpManagerTest, SummaryOnlyWhitelisting) {
-  // Summary only MDPs are a subset of background MDPs.
+TEST_F(MemoryDumpManagerTest, BackgroundWhitelisting) {
   SetDumpProviderWhitelistForTesting(kTestMDPWhitelist);
-  SetDumpProviderSummaryWhitelistForTesting(kTestMDPWhitelistForSummary);
 
   // Standard provider with default options (create dump for current process).
-  MockMemoryDumpProvider summaryMdp;
-  RegisterDumpProvider(&summaryMdp, nullptr, kDefaultOptions,
-                       kWhitelistedMDPName);
   MockMemoryDumpProvider backgroundMdp;
   RegisterDumpProvider(&backgroundMdp, nullptr, kDefaultOptions,
-                       kBackgroundButNotSummaryWhitelistedMDPName);
+                       kWhitelistedMDPName);
 
   EnableForTracing();
 
-  EXPECT_CALL(backgroundMdp, OnMemoryDump(_, _)).Times(0);
-  EXPECT_CALL(summaryMdp, OnMemoryDump(_, _)).Times(1);
+  EXPECT_CALL(backgroundMdp, OnMemoryDump(_, _)).Times(1);
   EXPECT_TRUE(RequestProcessDumpAndWait(MemoryDumpType::SUMMARY_ONLY,
                                         MemoryDumpLevelOfDetail::BACKGROUND));
   DisableTracing();
@@ -1046,7 +1035,6 @@ class SimpleMockMemoryDumpProvider : public MemoryDumpProvider {
 
 TEST_F(MemoryDumpManagerTest, NoStackOverflowWithTooManyMDPs) {
   SetDumpProviderWhitelistForTesting(kTestMDPWhitelist);
-  SetDumpProviderSummaryWhitelistForTesting(kTestMDPWhitelistForSummary);
 
   int kMDPCount = 1000;
   std::vector<std::unique_ptr<SimpleMockMemoryDumpProvider>> mdps;
@@ -1054,11 +1042,6 @@ TEST_F(MemoryDumpManagerTest, NoStackOverflowWithTooManyMDPs) {
     mdps.push_back(std::make_unique<SimpleMockMemoryDumpProvider>(1));
     RegisterDumpProvider(mdps.back().get(), nullptr);
   }
-  for (int i = 0; i < kMDPCount; ++i) {
-    mdps.push_back(std::make_unique<SimpleMockMemoryDumpProvider>(2));
-    RegisterDumpProvider(mdps.back().get(), nullptr, kDefaultOptions,
-                         kBackgroundButNotSummaryWhitelistedMDPName);
-  }
   for (int i = 0; i < kMDPCount; ++i) {
     mdps.push_back(std::make_unique<SimpleMockMemoryDumpProvider>(3));
     RegisterDumpProvider(mdps.back().get(), nullptr, kDefaultOptions,

base/trace_event/memory_dump_provider_info.cc

@@ -16,14 +16,12 @@ MemoryDumpProviderInfo::MemoryDumpProviderInfo(
     const char* name,
     scoped_refptr<SequencedTaskRunner> task_runner,
     const MemoryDumpProvider::Options& options,
-    bool whitelisted_for_background_mode,
-    bool whitelisted_for_summary_mode)
+    bool whitelisted_for_background_mode)
     : dump_provider(dump_provider),
       options(options),
       name(name),
       task_runner(std::move(task_runner)),
       whitelisted_for_background_mode(whitelisted_for_background_mode),
-      whitelisted_for_summary_mode(whitelisted_for_summary_mode),
       consecutive_failures(0),
       disabled(false) {}
 

base/trace_event/memory_dump_provider_info.h

@@ -58,8 +58,7 @@ struct BASE_EXPORT MemoryDumpProviderInfo
                          const char* name,
                          scoped_refptr<SequencedTaskRunner> task_runner,
                          const MemoryDumpProvider::Options& options,
-                         bool whitelisted_for_background_mode,
-                         bool whitelisted_for_summary_mode);
+                         bool whitelisted_for_background_mode);
 
   // It is safe to access the const fields below from any thread as they are
   // never mutated.
@@ -81,9 +80,6 @@ struct BASE_EXPORT MemoryDumpProviderInfo
   // True if the dump provider is whitelisted for background mode.
   const bool whitelisted_for_background_mode;
 
-  // True if the dump provider is whitelisted for summary mode.
-  const bool whitelisted_for_summary_mode;
-
   // These fields below, instead, are not thread safe and can be mutated only:
   // - On the |task_runner|, when not null (i.e. for thread-bound MDPS).
   // - By the MDM's background thread (or in any other way that guarantees

base/trace_event/memory_infra_background_whitelist.cc

@@ -18,48 +18,9 @@ namespace {
 // The names of dump providers whitelisted for background tracing. Dump
 // providers can be added here only if the background mode dump has very
 // little processor and memory overhead.
-const char* const kDumpProviderWhitelist[] = {
-    "android::ResourceManagerImpl",
-    "AutocompleteController",
-    "BlinkGC",
-    "BlinkObjectCounters",
-    "BlobStorageContext",
-    "ClientDiscardableSharedMemoryManager",
-    "DOMStorage",
-    "DownloadService",
-    "DiscardableSharedMemoryManager",
-    "gpu::BufferManager",
-    "gpu::RenderbufferManager",
-    "gpu::TextureManager",
-    "FontCaches",
-    "HistoryReport",
-    "IndexedDBBackingStore",
-    "InMemoryURLIndex",
-    "JavaHeap",
-    "LevelDB",
-    "LeveldbValueStore",
-    "LocalStorage",
-    "Malloc",
-    "MemoryCache",
-    "MojoHandleTable",
-    "MojoLevelDB",
-    "PartitionAlloc",
-    "ProcessMemoryMetrics",
-    "Skia",
-    "SharedMemoryTracker",
-    "Sql",
-    "URLRequestContext",
-    "V8Isolate",
-    "WinHeap",
-    "SyncDirectory",
-    "TabRestoreServiceHelper",
-    nullptr  // End of list marker.
-};
-
-// The names of dump providers whitelisted for summary tracing.
 // TODO(ssid): Some dump providers do not create ownership edges on background
 // dump. So, the effective size will not be correct.
-const char* const kDumpProviderSummaryWhitelist[] = {
+const char* const kDumpProviderWhitelist[] = {
     "android::ResourceManagerImpl",
     "AutocompleteController",
     "BlinkGC",
@@ -298,8 +259,6 @@ const char* const kAllocatorDumpNameWhitelist[] = {
 };
 
 const char* const* g_dump_provider_whitelist = kDumpProviderWhitelist;
-const char* const* g_dump_provider_whitelist_for_summary =
-    kDumpProviderSummaryWhitelist;
 const char* const* g_allocator_dump_name_whitelist =
     kAllocatorDumpNameWhitelist;
 
@@ -317,11 +276,6 @@ bool IsMemoryDumpProviderWhitelisted(const char* mdp_name) {
   return IsMemoryDumpProviderInList(mdp_name, g_dump_provider_whitelist);
 }
 
-bool IsMemoryDumpProviderWhitelistedForSummary(const char* mdp_name) {
-  return IsMemoryDumpProviderInList(mdp_name,
-                                    g_dump_provider_whitelist_for_summary);
-}
-
 bool IsMemoryAllocatorDumpNameWhitelisted(const std::string& name) {
   // Global dumps are explicitly whitelisted for background use.
   if (base::StartsWith(name, "global/", CompareCase::SENSITIVE)) {
@@ -369,10 +323,6 @@ void SetDumpProviderWhitelistForTesting(const char* const* list) {
   g_dump_provider_whitelist = list;
 }
 
-void SetDumpProviderSummaryWhitelistForTesting(const char* const* list) {
-  g_dump_provider_whitelist_for_summary = list;
-}
-
 void SetAllocatorDumpNameWhitelistForTesting(const char* const* list) {
   g_allocator_dump_name_whitelist = list;
 }

base/trace_event/memory_infra_background_whitelist.h

@@ -18,10 +18,6 @@ namespace trace_event {
 // Checks if the given |mdp_name| is in the whitelist.
 bool BASE_EXPORT IsMemoryDumpProviderWhitelisted(const char* mdp_name);
 
-// Checks if the given |mdp_name| is required for summary dumps.
-bool BASE_EXPORT
-IsMemoryDumpProviderWhitelistedForSummary(const char* mdp_name);
-
 // Checks if the given |name| matches any of the whitelisted patterns.
 bool BASE_EXPORT IsMemoryAllocatorDumpNameWhitelisted(const std::string& name);
 
@@ -29,8 +25,6 @@ bool BASE_EXPORT IsMemoryAllocatorDumpNameWhitelisted(const std::string& name);
 // the list must be nullptr.
 void BASE_EXPORT SetDumpProviderWhitelistForTesting(const char* const* list);
 void BASE_EXPORT
-SetDumpProviderSummaryWhitelistForTesting(const char* const* list);
-void BASE_EXPORT
 SetAllocatorDumpNameWhitelistForTesting(const char* const* list);
 
 }  // namespace trace_event

base/trace_event/memory_peak_detector_unittest.cc

@@ -176,10 +176,9 @@ class MemoryPeakDetectorTest : public testing::Test {
     std::unique_ptr<MockMemoryDumpProvider> mdp(new MockMemoryDumpProvider());
     MemoryDumpProvider::Options opt;
     opt.is_fast_polling_supported = true;
-    scoped_refptr<MemoryDumpProviderInfo> mdp_info(
-        new MemoryDumpProviderInfo(mdp.get(), "Mock MDP", nullptr, opt,
-                                   false /* whitelisted_for_background_mode */,
-                                   false /* whitelisted_for_summary_mode */));
+    scoped_refptr<MemoryDumpProviderInfo> mdp_info(new MemoryDumpProviderInfo(
+        mdp.get(), "Mock MDP", nullptr, opt,
+        false /* whitelisted_for_background_mode */));
 
     // The |mdp| instance will be destroyed together with the |mdp_info|.
     mdp_info->owned_dump_provider = std::move(mdp);