Move crash keys for command-line switches to components/crash so they can be set

from the installer as well as chrome, and set them on installer startup.

BUG=558552

Review URL: https://codereview.chromium.org/1481933002

Cr-Commit-Position: refs/heads/master@{#362437}

5 files changed, 249 insertions, 8 deletions

diff --git a/components/components_tests.gyp b/components/components_tests.gyp
index 861bd3a..1683204 100644
--- a/components/components_tests.gyp
+++ b/components/components_tests.gyp
@@ -125,6 +125,7 @@
     ],
     'crash_unittest_sources': [
       'crash/content/app/crash_keys_win_unittest.cc',
+      'crash/core/common/crash_keys_unittest.cc',
       'crash/core/common/objc_zombie_unittest.mm',
     ],
     'crx_file_unittest_sources': [
diff --git a/components/crash/core/common/BUILD.gn b/components/crash/core/common/BUILD.gn
index e602f4e..dc7216d 100644
--- a/components/crash/core/common/BUILD.gn
+++ b/components/crash/core/common/BUILD.gn
@@ -22,14 +22,17 @@ source_set("common") {
 
 source_set("unit_tests") {
   testonly = true
+  sources = [
+    "crash_keys_unittest.cc",
+  ]
+
+  deps = [
+    ":common",
+    "//base",
+    "//testing/gtest",
+  ]
+
   if (is_mac || is_ios) {
-    sources = [
-      "objc_zombie_unittest.mm",
-    ]
-    deps = [
-      ":common",
-      "//base",
-      "//testing/gtest",
-    ]
+    sources += [ "objc_zombie_unittest.mm" ]
   }
 }
diff --git a/components/crash/core/common/crash_keys.cc b/components/crash/core/common/crash_keys.cc
index 3c3f2fb..f34409e 100644
--- a/components/crash/core/common/crash_keys.cc
+++ b/components/crash/core/common/crash_keys.cc
@@ -4,11 +4,14 @@
 
 #include "components/crash/core/common/crash_keys.h"
 
+#include "base/command_line.h"
 #include "base/debug/crash_logging.h"
 #include "base/format_macros.h"
+#include "base/logging.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 
 namespace crash_keys {
 
@@ -33,6 +36,9 @@ const char kChannel[] = "channel";
 const char kNumVariations[] = "num-experiments";
 const char kVariations[] = "variations";
 
+const char kSwitchFormat[] = "switch-%" PRIuS;
+const char kNumSwitches[] = "num-switches";
+
 const char kBug464926CrashKey[] = "bug-464926-info";
 
 #if defined(OS_MACOSX)
@@ -100,4 +106,64 @@ void SetVariationsList(const std::vector<std::string>& variations) {
   base::debug::SetCrashKeyValue(kVariations, variations_string);
 }
 
+void GetCrashKeysForCommandLineSwitches(
+    std::vector<base::debug::CrashKey>* keys) {
+  DCHECK(keys);
+  base::debug::CrashKey crash_key = { kNumSwitches, kSmallSize };
+  keys->push_back(crash_key);
+
+  // Use static storage for formatted key names, since they will persist for
+  // the duration of the program.
+  static char formatted_keys[kSwitchesMaxCount][sizeof(kSwitchFormat) + 1] =
+      {{ 0 }};
+  const size_t formatted_key_len = sizeof(formatted_keys[0]);
+
+  // sizeof(kSwitchFormat) is platform-dependent, so make sure formatted_keys
+  // actually have space for a 2-digit switch number plus null-terminator.
+  static_assert(formatted_key_len >= 10,
+                "insufficient space for \"switch-NN\"");
+
+  for (size_t i = 0; i < kSwitchesMaxCount; ++i) {
+    // Name the keys using 1-based indexing.
+    int n = base::snprintf(formatted_keys[i], formatted_key_len, kSwitchFormat,
+                           i + 1);
+    DCHECK_GT(n, 0);
+    base::debug::CrashKey crash_key = { formatted_keys[i], kSmallSize };
+    keys->push_back(crash_key);
+  }
+}
+
+void SetSwitchesFromCommandLine(const base::CommandLine& command_line,
+                                SwitchFilterFunction skip_filter) {
+  const base::CommandLine::StringVector& argv = command_line.argv();
+
+  // Set the number of switches in case size > kNumSwitches.
+  base::debug::SetCrashKeyValue(kNumSwitches,
+      base::StringPrintf("%" PRIuS, argv.size() - 1));
+
+  size_t key_i = 1;  // Key names are 1-indexed.
+
+  // Go through the argv, skipping the exec path. Stop if there are too many
+  // switches to hold in crash keys.
+  for (size_t i = 1; i < argv.size() && key_i <= crash_keys::kSwitchesMaxCount;
+       ++i) {
+#if defined(OS_WIN)
+    std::string switch_str = base::WideToUTF8(argv[i]);
+#else
+    std::string switch_str = argv[i];
+#endif
+
+    // Skip uninteresting switches.
+    if (skip_filter && (*skip_filter)(switch_str))
+      continue;
+
+    std::string key = base::StringPrintf(kSwitchFormat, key_i++);
+    base::debug::SetCrashKeyValue(key, switch_str);
+  }
+
+  // Clear any remaining switches.
+  for (; key_i <= kSwitchesMaxCount; ++key_i)
+    base::debug::ClearCrashKey(base::StringPrintf(kSwitchFormat, key_i));
+}
+
 }  // namespace crash_keys
diff --git a/components/crash/core/common/crash_keys.h b/components/crash/core/common/crash_keys.h
index 6ddb356..2265505 100644
--- a/components/crash/core/common/crash_keys.h
+++ b/components/crash/core/common/crash_keys.h
@@ -8,8 +8,13 @@
 #include <string>
 #include <vector>
 
+#include "base/debug/crash_logging.h"
 #include "build/build_config.h"
 
+namespace base {
+class CommandLine;
+}  // namespace base
+
 namespace crash_keys {
 
 // Sets the ID (which may either be a full GUID or a GUID that was already
@@ -21,6 +26,20 @@ void ClearMetricsClientId();
 // Sets the list of active experiment/variations info.
 void SetVariationsList(const std::vector<std::string>& variations);
 
+// Adds a common set of crash keys for holding command-line switches to |keys|.
+void GetCrashKeysForCommandLineSwitches(
+    std::vector<base::debug::CrashKey>* keys);
+
+// A function returning true if |flag| is a switch that should be filtered out
+// of crash keys.
+using SwitchFilterFunction = bool (*)(const std::string& flag);
+
+// Sets the kNumSwitches key and a set of keys named using kSwitchFormat based
+// on the given |command_line|. If |skip_filter| is not null, ignore any switch
+// for which it returns true.
+void SetSwitchesFromCommandLine(const base::CommandLine& command_line,
+                                SwitchFilterFunction skip_filter);
+
 // Crash Key Constants /////////////////////////////////////////////////////////
 
 // kChunkMaxLength is the platform-specific maximum size that a value in a
@@ -71,6 +90,18 @@ extern const char kNumVariations[];
 // typically set by SetExperimentList.
 extern const char kVariations[];
 
+// The maximum number of command line switches to process. |kSwitchFormat|
+// should be formatted with an integer in the range [1, kSwitchesMaxCount].
+const size_t kSwitchesMaxCount = 15;
+
+// A printf-style format string naming the set of crash keys corresponding to
+// at most |kSwitchesMaxCount| command line switches.
+extern const char kSwitchFormat[];
+
+// The total number of switches, used to report the total in case more than
+// |kSwitchesMaxCount| are present.
+extern const char kNumSwitches[];
+
 // Used to help investigate bug 464926.
 extern const char kBug464926CrashKey[];
 
diff --git a/components/crash/core/common/crash_keys_unittest.cc b/components/crash/core/common/crash_keys_unittest.cc
new file mode 100644
index 0000000..ef8a802
--- /dev/null
+++ b/components/crash/core/common/crash_keys_unittest.cc
@@ -0,0 +1,140 @@
+// Copyright 2013 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 "components/crash/core/common/crash_keys.h"
+
+#include <map>
+#include <string>
+
+#include "base/command_line.h"
+#include "base/compiler_specific.h"
+#include "base/debug/crash_logging.h"
+#include "base/format_macros.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class CrashKeysTest : public testing::Test {
+ public:
+  void SetUp() override {
+    self_ = this;
+    base::debug::SetCrashKeyReportingFunctions(
+        &SetCrashKeyValue, &ClearCrashKey);
+
+    std::vector<base::debug::CrashKey> keys;
+    crash_keys::GetCrashKeysForCommandLineSwitches(&keys);
+    base::debug::InitCrashKeys(keys.data(), keys.size(),
+                               crash_keys::kChunkMaxLength);
+    ASSERT_FALSE(keys.empty());
+  }
+
+  void TearDown() override {
+    base::debug::ResetCrashLoggingForTesting();
+    self_ = NULL;
+  }
+
+  bool HasCrashKey(const std::string& key) {
+    return keys_.find(key) != keys_.end();
+  }
+
+  std::string GetKeyValue(const std::string& key) {
+    std::map<std::string, std::string>::const_iterator it = keys_.find(key);
+    if (it == keys_.end())
+      return std::string();
+    return it->second;
+  }
+
+ private:
+  static void SetCrashKeyValue(const base::StringPiece& key,
+                               const base::StringPiece& value) {
+    self_->keys_[key.as_string()] = value.as_string();
+  }
+
+  static void ClearCrashKey(const base::StringPiece& key) {
+    self_->keys_.erase(key.as_string());
+  }
+
+  static CrashKeysTest* self_;
+
+  std::map<std::string, std::string> keys_;
+};
+
+CrashKeysTest* CrashKeysTest::self_ = NULL;
+
+TEST_F(CrashKeysTest, Switches) {
+  // Set three switches.
+  {
+    base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+    for (size_t i = 1; i <= 3; ++i)
+      command_line.AppendSwitch(base::StringPrintf("--flag-%" PRIuS, i));
+    crash_keys::SetSwitchesFromCommandLine(command_line, nullptr);
+    EXPECT_EQ("--flag-1", GetKeyValue("switch-1"));
+    EXPECT_EQ("--flag-2", GetKeyValue("switch-2"));
+    EXPECT_EQ("--flag-3", GetKeyValue("switch-3"));
+    EXPECT_FALSE(HasCrashKey("switch-4"));
+  }
+
+  // Set more than the max switches.
+  {
+    base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+    const size_t kMax = crash_keys::kSwitchesMaxCount + 2;
+    EXPECT_GT(kMax, static_cast<size_t>(15));
+    for (size_t i = 1; i <= kMax; ++i)
+      command_line.AppendSwitch(base::StringPrintf("--many-%" PRIuS, i));
+    crash_keys::SetSwitchesFromCommandLine(command_line, nullptr);
+    EXPECT_EQ("--many-1", GetKeyValue("switch-1"));
+    EXPECT_EQ("--many-9", GetKeyValue("switch-9"));
+    EXPECT_EQ("--many-15", GetKeyValue("switch-15"));
+    EXPECT_FALSE(HasCrashKey("switch-16"));
+    EXPECT_FALSE(HasCrashKey("switch-17"));
+  }
+
+  // Set fewer to ensure that old ones are erased.
+  {
+    base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+    for (size_t i = 1; i <= 5; ++i)
+      command_line.AppendSwitch(base::StringPrintf("--fewer-%" PRIuS, i));
+    crash_keys::SetSwitchesFromCommandLine(command_line, nullptr);
+    EXPECT_EQ("--fewer-1", GetKeyValue("switch-1"));
+    EXPECT_EQ("--fewer-2", GetKeyValue("switch-2"));
+    EXPECT_EQ("--fewer-3", GetKeyValue("switch-3"));
+    EXPECT_EQ("--fewer-4", GetKeyValue("switch-4"));
+    EXPECT_EQ("--fewer-5", GetKeyValue("switch-5"));
+    for (size_t i = 6; i < 20; ++i)
+      EXPECT_FALSE(HasCrashKey(base::StringPrintf(crash_keys::kSwitchFormat,
+                                                  i)));
+  }
+}
+
+namespace {
+
+bool IsBoringFlag(const std::string& flag) {
+  return flag.compare("--boring") == 0;
+}
+
+}  // namespace
+
+TEST_F(CrashKeysTest, FilterFlags) {
+  using crash_keys::kSwitchesMaxCount;
+
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch("--not-boring-1");
+  command_line.AppendSwitch("--boring");
+
+  // Include the max number of non-boring switches, to make sure that only the
+  // switches actually included in the crash keys are counted.
+  for (size_t i = 2; i <= kSwitchesMaxCount; ++i)
+    command_line.AppendSwitch(base::StringPrintf("--not-boring-%" PRIuS, i));
+
+  crash_keys::SetSwitchesFromCommandLine(command_line, &IsBoringFlag);
+
+  // If the boring keys are filtered out, every single key should now be
+  // not-boring.
+  for (size_t i = 1; i <= kSwitchesMaxCount; ++i) {
+    std::string switch_name = base::StringPrintf(crash_keys::kSwitchFormat, i);
+    std::string switch_value = base::StringPrintf("--not-boring-%" PRIuS, i);
+    EXPECT_EQ(switch_value, GetKeyValue(switch_name)) << "switch_name is " <<
+        switch_name;
+  }
+}