Change memory dumps to not use periodic dumps by default.

content/browser/tracing/tracing_ui.cc

 // Copyright (c) 2012 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 "content/browser/tracing/tracing_ui.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <set>
@@ skipping 71 matching lines @@
   bool options_ok = true;
   std::string category_filter_string;
   options_ok &= options->GetString("categoryFilter", &category_filter_string);
 
   std::string record_mode;
   options_ok &= options->GetString("tracingRecordMode", &record_mode);
 
   *trace_config = base::trace_event::TraceConfig(category_filter_string,
                                                  record_mode);
 
+  // When enabling memory-infra via tracing, use periodic dumping to trigger a
+  // detailed dump every 5s.
+  if (category_filter_string.find(
+          base::trace_event::MemoryDumpManager::kTraceCategory) !=

[Primiano Tucci (use gerrit), 2017/05/26 19:12:33]

Iirc there is a iscategoryenabled in traceconfig that makes this a bit cleaner. 
The real bug here is that I think somebody created a memory-infra.V8 which adds
some extra expensive stuff. This string matching will get also that. Is category
enabled() will not. 

[erikchen, 2017/05/26 20:31:13]

Done.

+      std::string::npos) {
+    base::trace_event::TraceConfig::MemoryDumpConfig::Trigger trigger = {
+        5000,  // min_time_between_dumps_ms
+        base::trace_event::MemoryDumpLevelOfDetail::DETAILED,
+        base::trace_event::MemoryDumpType::PERIODIC_INTERVAL};
+    base::trace_event::TraceConfig::MemoryDumpConfig memory_dump_config;
+    memory_dump_config.triggers.push_back(trigger);
+    memory_dump_config.allowed_dump_modes.insert(
+        base::trace_event::MemoryDumpLevelOfDetail::DETAILED);

[Primiano Tucci (use gerrit), 2017/05/26 19:12:33]

Check with ssid, I don't think you need this. This should be only for
restricting, this one here should be the default behavior. 

[erikchen, 2017/05/26 20:31:13]

This is necessary, because a freshly constructed MemoryDumpConfig doesn't pick
up the default behavior. *shrug*. You're welcome to fix this. :)

[ssid, 2017/05/26 21:56:07]

Yeah we only thought about the trace config string case where if it is not
specified then all modes are assumed. While constructing the object itself, we
need to add modes manually. meh this is unfortunate. add a todo for me.

[erikchen, 2017/05/27 00:51:03]

Done.

+    trace_config->ResetMemoryDumpConfig(memory_dump_config);
+  }
+
   bool enable_systrace;
   options_ok &= options->GetBoolean("useSystemTracing", &enable_systrace);
   if (enable_systrace)
     trace_config->EnableSystrace();
 
   if (!options_ok) {
     LOG(ERROR) << "Malformed options";
     return false;
   }
   return true;
@@ skipping 205 matching lines @@
     web_ui()->CallJavascriptFunctionUnsafe("onUploadComplete",
                                            base::Value(feedback));
   } else {
     web_ui()->CallJavascriptFunctionUnsafe("onUploadError",
                                            base::Value(feedback));
   }
   trace_uploader_.reset();
 }
 
 }  // namespace content