Fix crash reporting of child process command line switches.

It's not immediatley clear to me how this broke, since I converted all the
callsites of the old method. However I've verified on each platform that this
now reports command line switches properly.

BUG=77656,298225

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226519 0039d316-1c4b-4281-b951-d872f2087c98

chrome/app/chrome_main_delegate.cc

@@ -27,6 +27,7 @@
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/crash_keys.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/profiling.h"
 #include "chrome/common/url_constants.h"
@@ -744,6 +745,10 @@ void ChromeMainDelegate::PreSandboxStartup() {
 #endif
   }
 #endif
+
+  // After all the platform Breakpads have been initialized, store the command
+  // line for crash reporting.
+  crash_keys::SetSwitchesFromCommandLine(&command_line);
 }
 
 void ChromeMainDelegate::SandboxInitialized(const std::string& process_type) {
@@ -841,6 +846,9 @@ void ChromeMainDelegate::ZygoteForked() {
   // Needs to be called after we have chrome::DIR_USER_DATA.  BrowserMain sets
   // this up for the browser process in a different manner.
   InitCrashReporter();
+
+  // Reset the command line for the newly spawned process.
+  crash_keys::SetSwitchesFromCommandLine(CommandLine::ForCurrentProcess());
 #endif
 }
 

chrome/chrome_tests_unit.gypi

@@ -1725,11 +1725,12 @@
         'browser/webdata/web_apps_table_unittest.cc',
         'common/cancelable_task_tracker_unittest.cc',
         'common/chrome_paths_unittest.cc',
-        'common/cloud_print/cloud_print_helpers_unittest.cc',
         'common/chrome_content_client_unittest.cc',
+        'common/cloud_print/cloud_print_helpers_unittest.cc',
         'common/content_settings_helper_unittest.cc',
         'common/content_settings_pattern_parser_unittest.cc',
         'common/content_settings_pattern_unittest.cc',
+        'common/crash_keys_unittest.cc',
         'common/extensions/api/commands/commands_manifest_unittest.cc',
         'common/extensions/api/extension_action/browser_action_manifest_unittest.cc',
         'common/extensions/api/extension_action/page_action_manifest_unittest.cc',

chrome/common/crash_keys.cc

@@ -280,6 +280,10 @@ void SetSwitchesFromCommandLine(const CommandLine* command_line) {
     if (IsBoringSwitch(switch_str))
       continue;
 
+    // Stop if there are too many switches.
+    if (i > crash_keys::kSwitchesMaxCount)
+      break;
+
     std::string key = base::StringPrintf(kSwitch, key_i++);
     base::debug::SetCrashKeyValue(key, switch_str);
   }

chrome/common/crash_keys_unittest.cc

+// 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 "chrome/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/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class CrashKeysTest : public testing::Test {
+ public:
+  virtual void SetUp() OVERRIDE {
+    self_ = this;
+    base::debug::SetCrashKeyReportingFunctions(
+        &SetCrashKeyValue, &ClearCrashKey);
+    crash_keys::RegisterChromeCrashKeys();
+  }
+
+  virtual 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) {
+    return keys_[key];
+  }
+
+ 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.
+  {
+    CommandLine command_line(CommandLine::NO_PROGRAM);
+    for (int i = 1; i <= 3; ++i)
+      command_line.AppendSwitch(base::StringPrintf("--flag-%d", i));
+    crash_keys::SetSwitchesFromCommandLine(&command_line);
+    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.
+  {
+    CommandLine command_line(CommandLine::NO_PROGRAM);
+    const int kMax = crash_keys::kSwitchesMaxCount + 2;
+    EXPECT_GT(kMax, 15);
+    for (int i = 1; i <= kMax; ++i)
+      command_line.AppendSwitch(base::StringPrintf("--many-%d", i));
+    crash_keys::SetSwitchesFromCommandLine(&command_line);
+    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.
+  {
+    CommandLine command_line(CommandLine::NO_PROGRAM);
+    const char kFormat[] = "--fewer-%d";
+    for (int i = 1; i <= 5; ++i)
+      command_line.AppendSwitch(base::StringPrintf(kFormat, i));
+    crash_keys::SetSwitchesFromCommandLine(&command_line);
+    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 (int i = 6; i < 20; ++i)
+      EXPECT_FALSE(HasCrashKey(base::StringPrintf(kFormat, i)));
+  }
+}