[Startup Tracing] Enable startup tracing from config file

base/trace_event/startup_tracing.cc

+// Copyright (c) 2015 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 "base/trace_event/startup_tracing.h"
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "base/trace_event/trace_event_impl.h"
+
+namespace base {
+namespace trace_event {
+
+namespace {
+
+// Trace config file path:
+// - Android: /data/local/chrome-trace-config.json

[dsinclair, 2015/06/11 14:37:08]

Why not /data/local/chrome-telemetry/chrome-tracing-config.json seems a bit more
consistent?

[Zhen Wang, 2015/06/12 01:26:05]

Done.

+// - POSIX other than Android: $HOME/.chrome-telemetry/chrome-trace-config.json
+// - Win: %USERPROFILE%/.chrome-telemetry/chrome-trace-config.json
+#if defined(OS_ANDROID)
+const FilePath::CharType kAndroidTraceConfigDir[] =
+    FILE_PATH_LITERAL("/data/local");
+#elif defined(OS_POSIX) || defined(OS_WIN)
+const FilePath::CharType kChromeTelemetryDir[] =
+    FILE_PATH_LITERAL(".chrome-telemetry");
+#endif
+
+#if defined(OS_POSIX) || defined(OS_WIN)

[dsinclair, 2015/06/11 14:37:09]

Is this ifdef needed? This is always the name of the json file.

[Zhen Wang, 2015/06/12 01:26:05]

This is for the cases where it is neither POSIX nor WIN. For example, IOS.

[dsinclair, 2015/06/12 14:04:14]

I think we can still have those constant defined on IOS, and I'd assume if we
get startup tracing on IOS then we could use them same directory names? Is this
just to save space in the final binary?

[Zhen Wang, 2015/06/12 15:41:34]

OK. defined is now removed.

+const FilePath::CharType kTraceConfigFileName[] =
+    FILE_PATH_LITERAL("chrome-trace-config.json");
+#endif
+
+FilePath GetTraceConfigFilePath() {
+#if defined(OS_ANDROID)
+  FilePath path(kAndroidTraceConfigDir);
+  path = path.Append(kTraceConfigFileName);
+#elif defined(OS_POSIX) || defined(OS_WIN)
+  FilePath path;
+  PathService::Get(DIR_HOME, &path);
+  path = path.Append(kChromeTelemetryDir);
+  path = path.Append(kTraceConfigFileName);
+#else
+  FilePath path;

[dsinclair, 2015/06/11 14:37:09]

Should this log a warning that the config file path doesn't exist? 

[Zhen Wang, 2015/06/12 01:26:05]

This is for the cases where it is neither POSIX nor WIN. For example, IOS. And
we currently do not support it.

For POSIX and WIN, the config file path always exists. The file itself may not
exist.

+#endif
+  return path;
+}
+
+} // namespace
+
+void EnableStartupTracingFromConfigFile() {

[dsinclair, 2015/06/11 14:37:09]

This seems slightly misnamed?

EnableStartupTracingIfConfigFileExists()? or something? Just, this doesn't
always enable, it depends on the config file being there.

[Zhen Wang, 2015/06/12 01:26:05]

Makes sense. Renamed.

+  base::FilePath trace_config_file_path = GetTraceConfigFilePath();
+  if (!trace_config_file_path.empty() && PathExists(trace_config_file_path)) {

[dsinclair, 2015/06/11 14:37:09]

if (trace_config_file_path.empty() || !PathExists(trace_config_file_path))
 return;

[Zhen Wang, 2015/06/12 01:26:05]

Done.

+    std::string trace_config_str;
+    base::ReadFileToString(trace_config_file_path, &trace_config_str);

[dsinclair, 2015/06/11 14:37:09]

Can this fail?

[Zhen Wang, 2015/06/12 01:26:05]

Thanks for catching this. Now checking the return value.

+    TraceConfig trace_config(trace_config_str);

[dsinclair, 2015/06/11 14:37:09]

Was TraceConfig converted over to SafeJSONReader?

[Zhen Wang, 2015/06/12 01:26:05]

No. It still uses normal json reader. So the decision is to put the config file
to /data/local/ on Android and rooted device is required. See
https://docs.google.com/document/d/1yRCXhrQ-0rsfUgNHt9T4YdnmJYrXKN6aK56Ozk3kP...

+    TraceLog::GetInstance()->SetEnabled(trace_config, TraceLog::RECORDING_MODE);
+  }
+}
+
+}  // namespace trace_event
+}  // namespace base