Set active extension IDs for crash reports using the crash key logging system.

BUG=77656

Review URL: https://chromiumcodereview.appspot.com/23471007

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

base/debug/crash_logging.h

@@ -56,7 +56,7 @@ class BASE_EXPORT ScopedCrashKey {
 // Before setting values for a key, all the keys must be registered.
 struct BASE_EXPORT CrashKey {
   // The name of the crash key, used in the above functions.
-  const char* const key_name;
+  const char* key_name;
 
   // The maximum length for a value. If the value is longer than this, it will
   // be truncated. If the value is larger than the |chunk_max_length| passed to

chrome/app/breakpad_linux.cc

@@ -1167,16 +1167,6 @@ void HandleCrashDump(const BreakpadInfo& info) {
   //   3 \r\n
   //   BOUNDARY \r\n
   //
-  //   zero or one:
-  //   Content-Disposition: form-data; name="num-extensions" \r\n \r\n
-  //   5 \r\n
-  //   BOUNDARY \r\n
-  //
-  //   zero to 10:
-  //   Content-Disposition: form-data; name="extension-1" \r\n \r\n
-  //   abcdefghijklmnopqrstuvwxyzabcdef \r\n
-  //   BOUNDARY \r\n
-  //
   //   zero to 4:
   //   Content-Disposition: form-data; name="prn-info-1" \r\n \r\n
   //   abcdefghijklmnopqrstuvwxyzabcdef \r\n
@@ -1311,26 +1301,6 @@ void HandleCrashDump(const BreakpadInfo& info) {
     writer.Flush();
   }
 
-  if (*child_process_logging::g_num_extensions) {
-    writer.AddPairString("num-extensions",
-                         child_process_logging::g_num_extensions);
-    writer.AddBoundary();
-    writer.Flush();
-  }
-
-  unsigned extension_ids_len =
-      my_strlen(child_process_logging::g_extension_ids);
-  if (extension_ids_len) {
-    static const char extension_msg[] = "extension-";
-    static const unsigned kMaxExtensionsLen =
-        kMaxReportedActiveExtensions * child_process_logging::kExtensionLen;
-    writer.AddPairDataInChunks(extension_msg, sizeof(extension_msg) - 1,
-        child_process_logging::g_extension_ids,
-        std::min(extension_ids_len, kMaxExtensionsLen),
-        child_process_logging::kExtensionLen,
-        false /* Don't strip whitespace. */);
-  }
-
   unsigned printer_info_len =
       my_strlen(child_process_logging::g_printer_info);
   if (printer_info_len) {

chrome/app/breakpad_win.cc

@@ -102,8 +102,6 @@ typedef NTSTATUS (WINAPI* NtTerminateProcessPtr)(HANDLE ProcessHandle,
                                                  NTSTATUS ExitStatus);
 char* g_real_terminate_process_stub = NULL;
 
-static size_t g_num_of_extensions_offset = 0;
-static size_t g_extension_ids_offset = 0;
 static size_t g_client_id_offset = 0;
 static size_t g_printer_info_offset = 0;
 static size_t g_num_of_views_offset = 0;
@@ -439,17 +437,6 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& exe_path,
     g_custom_entries->push_back(google_breakpad::CustomInfoEntry(
         L"special", UTF16ToWide(special_build).c_str()));
 
-  g_num_of_extensions_offset = g_custom_entries->size();
-  g_custom_entries->push_back(
-      google_breakpad::CustomInfoEntry(L"num-extensions", L"N/A"));
-
-  g_extension_ids_offset = g_custom_entries->size();
-  // one-based index for the name suffix.
-  for (int i = 1; i <= kMaxReportedActiveExtensions; ++i) {
-    g_custom_entries->push_back(google_breakpad::CustomInfoEntry(
-        base::StringPrintf(L"extension-%i", i).c_str(), L""));
-  }
-
   // Add empty values for the prn_info-*. We'll put the actual values when we
   // collect them at this location.
   g_printer_info_offset = g_custom_entries->size();
@@ -653,24 +640,6 @@ extern "C" void __declspec(dllexport) __cdecl SetClientId(
                 google_breakpad::CustomInfoEntry::kValueMaxLength);
 }
 
-extern "C" void __declspec(dllexport) __cdecl SetNumberOfExtensions(
-    int number_of_extensions) {
-  SetIntegerValue(g_num_of_extensions_offset, number_of_extensions);
-}
-
-extern "C" void __declspec(dllexport) __cdecl SetExtensionID(
-    int index, const wchar_t* id) {
-  DCHECK(id);
-  DCHECK(index < kMaxReportedActiveExtensions);
-
-  if (!g_custom_entries)
-    return;
-
-  base::wcslcpy((*g_custom_entries)[g_extension_ids_offset + index].value,
-                id,
-                google_breakpad::CustomInfoEntry::kValueMaxLength);
-}
-
 extern "C" void __declspec(dllexport) __cdecl SetPrinterInfo(
     const wchar_t* printer_info) {
   if (!g_custom_entries)

chrome/browser/extensions/extension_service.cc

@@ -67,9 +67,9 @@
 #include "chrome/browser/ui/webui/favicon_source.h"
 #include "chrome/browser/ui/webui/ntp/thumbnail_source.h"
 #include "chrome/browser/ui/webui/theme_source.h"
-#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/crash_keys.h"
 #include "chrome/common/extensions/background_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -2315,7 +2315,7 @@ void ExtensionService::UpdateActiveExtensionsInCrashReporter() {
       extension_ids.insert(extension->id());
   }
 
-  child_process_logging::SetActiveExtensions(extension_ids);
+  crash_keys::SetActiveExtensions(extension_ids);
 }
 
 ExtensionService::ImportStatus ExtensionService::SatisfyImports(

chrome/common/child_process_logging.h

@@ -15,11 +15,6 @@
 
 class CommandLine;
 
-// The maximum number of active extensions we will report.
-// Also used in chrome/app, but we define it here to avoid a common->app
-// dependency.
-static const size_t kMaxReportedActiveExtensions = 10;
-
 // The maximum number of variation chunks we will report.
 // Also used in chrome/app, but we define it here to avoid a common->app
 // dependency.
@@ -47,8 +42,6 @@ namespace child_process_logging {
 // compromised context without going through the standard library.
 extern char g_channel[];
 extern char g_client_id[];
-extern char g_extension_ids[];
-extern char g_num_extensions[];
 extern char g_num_switches[];
 extern char g_num_variations[];
 extern char g_num_views[];
@@ -56,9 +49,6 @@ extern char g_printer_info[];
 extern char g_switches[];
 extern char g_variation_chunks[];
 
-// Assume IDs are 32 bytes long.
-static const size_t kExtensionLen = 32;
-
 // Assume command line switches are less than 64 chars.
 static const size_t kSwitchLen = 64;
 
@@ -73,14 +63,6 @@ void SetClientId(const std::string& client_id);
 // id in |client_id| if it's known, an empty string otherwise.
 std::string GetClientId();
 
-// Sets the list of "active" extensions in this process. We overload "active" to
-// mean different things depending on the process type:
-// - browser: all enabled extensions
-// - renderer: the unique set of extension ids from all content scripts
-// - extension: the id of each extension running in this process (there can be
-//   multiple because of process collapsing).
-void SetActiveExtensions(const std::set<std::string>& extension_ids);
-
 // Sets a number of views/tabs opened in this process.
 void SetNumberOfViews(int number_of_views);
 

chrome/common/child_process_logging_mac.mm

@@ -25,8 +25,6 @@ using base::debug::ClearCrashKey;
 
 const char* kGuidParamName = "guid";
 const char* kNumberOfViews = "num-views";
-const char* kNumExtensionsName = "num-extensions";
-const char* kExtensionNameFormat = "extension-%zu";
 const char* kPrinterInfoNameFormat = "prn-info-%zu";
 
 // Account for the terminating null character.
@@ -53,25 +51,6 @@ std::string GetClientId() {
   return std::string(g_client_id);
 }
 
-void SetActiveExtensions(const std::set<std::string>& extension_ids) {
-  // Log the count separately to track heavy users.
-  const int count = static_cast<int>(extension_ids.size());
-  SetCrashKeyValue(kNumExtensionsName, base::IntToString(count));
-
-  // Record up to |kMaxReportedActiveExtensions| extensions, clearing
-  // keys if there aren't that many.
-  std::set<std::string>::const_iterator iter = extension_ids.begin();
-  for (size_t i = 0; i < kMaxReportedActiveExtensions; ++i) {
-    std::string key = base::StringPrintf(kExtensionNameFormat, i);
-    if (iter != extension_ids.end()) {
-      SetCrashKeyValue(key, *iter);
-      ++iter;
-    } else {
-      ClearCrashKey(key);
-    }
-  }
-}
-
 void SetPrinterInfo(const char* printer_info) {
   std::vector<std::string> info;
   base::SplitString(printer_info, ';', &info);

chrome/common/child_process_logging_posix.cc

@@ -29,15 +29,10 @@ char g_channel[kChannelSize] = "";
 char g_printer_info[kPrinterInfoStrLen * kMaxReportedPrinterRecords + 1] = "";
 
 static const size_t kNumSize = 32;
-char g_num_extensions[kNumSize] = "";
 char g_num_switches[kNumSize] = "";
 char g_num_variations[kNumSize] = "";
 char g_num_views[kNumSize] = "";
 
-static const size_t kMaxExtensionSize =
-    kExtensionLen * kMaxReportedActiveExtensions + 1;
-char g_extension_ids[kMaxExtensionSize] = "";
-
 // Assume command line switches are less than 64 chars.
 static const size_t kMaxSwitchesSize = kSwitchLen * kMaxSwitches + 1;
 char g_switches[kMaxSwitchesSize] = "";
@@ -62,21 +57,6 @@ std::string GetClientId() {
   return std::string(g_client_id);
 }
 
-void SetActiveExtensions(const std::set<std::string>& extension_ids) {
-  snprintf(g_num_extensions, arraysize(g_num_extensions), "%" PRIuS,
-           extension_ids.size());
-
-  std::string extension_str;
-  std::set<std::string>::const_iterator iter = extension_ids.begin();
-  for (size_t i = 0;
-       i < kMaxReportedActiveExtensions && iter != extension_ids.end();
-       ++i, ++iter) {
-    extension_str += *iter;
-  }
-  base::strlcpy(g_extension_ids, extension_str.c_str(),
-                arraysize(g_extension_ids));
-}
-
 void SetPrinterInfo(const char* printer_info) {
   std::string printer_info_str;
   std::vector<std::string> info;

chrome/common/child_process_logging_win.cc

@@ -22,14 +22,6 @@ namespace {
 // exported in breakpad_win.cc: void __declspec(dllexport) __cdecl SetClientId.
 typedef void (__cdecl *MainSetClientId)(const wchar_t*);
 
-// exported in breakpad_win.cc:
-//   void __declspec(dllexport) __cdecl SetNumberOfExtensions.
-typedef void (__cdecl *MainSetNumberOfExtensions)(int);
-
-// exported in breakpad_win.cc:
-// void __declspec(dllexport) __cdecl SetExtensionID.
-typedef void (__cdecl *MainSetExtensionID)(size_t, const wchar_t*);
-
 // exported in breakpad_win.cc:
 //     void __declspec(dllexport) __cdecl SetPrinterInfo.
 typedef void (__cdecl *MainSetPrinterInfo)(const wchar_t*);
@@ -102,44 +94,6 @@ std::string GetClientId() {
     return std::string();
 }
 
-void SetActiveExtensions(const std::set<std::string>& extension_ids) {
-  static MainSetNumberOfExtensions set_number_of_extensions = NULL;
-  // note: benign race condition on set_number_of_extensions.
-  if (!set_number_of_extensions) {
-    HMODULE exe_module = GetModuleHandle(chrome::kBrowserProcessExecutableName);
-    if (!exe_module)
-      return;
-    set_number_of_extensions = reinterpret_cast<MainSetNumberOfExtensions>(
-        GetProcAddress(exe_module, "SetNumberOfExtensions"));
-    if (!set_number_of_extensions)
-      return;
-  }
-
-  static MainSetExtensionID set_extension_id = NULL;
-  // note: benign race condition on set_extension_id.
-  if (!set_extension_id) {
-    HMODULE exe_module = GetModuleHandle(chrome::kBrowserProcessExecutableName);
-    if (!exe_module)
-      return;
-    set_extension_id = reinterpret_cast<MainSetExtensionID>(
-        GetProcAddress(exe_module, "SetExtensionID"));
-    if (!set_extension_id)
-      return;
-  }
-
-  (set_number_of_extensions)(static_cast<int>(extension_ids.size()));
-
-  std::set<std::string>::const_iterator iter = extension_ids.begin();
-  for (size_t i = 0; i < kMaxReportedActiveExtensions; ++i) {
-    if (iter != extension_ids.end()) {
-      (set_extension_id)(i, ASCIIToWide(iter->c_str()).c_str());
-      ++iter;
-    } else {
-      (set_extension_id)(i, L"");
-    }
-  }
-}
-
 void SetPrinterInfo(const char* printer_info) {
   static MainSetPrinterInfo set_printer_info = NULL;
   // note: benign race condition on set_printer_info.

chrome/common/crash_keys.cc

@@ -4,6 +4,11 @@
 
 #include "chrome/common/crash_keys.h"
 
+#include "base/format_macros.h"
+#include "base/logging.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+
 #if defined(OS_MACOSX)
 #include "breakpad/src/common/simple_string_dictionary.h"
 #elif defined(OS_WIN)
@@ -43,9 +48,51 @@ COMPILE_ASSERT(kMediumSize <= kSingleChunkLength,
                mac_has_medium_size_crash_key_chunks);
 #endif
 
+const char kActiveURL[] = "url-chunk";
+
+const char kExtensionID[] = "extension-%" PRIuS;
+const char kNumExtensionsCount[] = "num-extensions";
+
+#if !defined(OS_ANDROID)
+const char kGPUVendorID[] = "gpu-venid";
+const char kGPUDeviceID[] = "gpu-devid";
+#endif
+const char kGPUDriverVersion[] = "gpu-driver";
+const char kGPUPixelShaderVersion[] = "gpu-psver";
+const char kGPUVertexShaderVersion[] = "gpu-vsver";
+#if defined(OS_LINUX)
+const char kGPUVendor[] = "gpu-gl-vendor";
+const char kGPURenderer[] = "gpu-gl-renderer";
+#elif defined(OS_MACOSX)
+const char kGPUGLVersion[] = "gpu-glver";
+#endif
+
+#if defined(OS_MACOSX)
+namespace mac {
+
+const char kFirstNSException[] = "firstexception";
+const char kFirstNSExceptionTrace[] = "firstexception_bt";
+
+const char kLastNSException[] = "lastexception";
+const char kLastNSExceptionTrace[] = "lastexception_bt";
+
+const char kNSException[] = "nsexception";
+const char kNSExceptionTrace[] = "nsexception_bt";
+
+const char kSendAction[] = "sendaction";
+
+const char kZombie[] = "zombie";
+const char kZombieTrace[] = "zombie_dealloc_bt";
+
+}  // namespace mac
+#endif
+
 size_t RegisterChromeCrashKeys() {
-  base::debug::CrashKey keys[] = {
+  // The following keys may be chunked by the underlying crash logging system,
+  // but ultimately constitute a single key-value pair.
+  base::debug::CrashKey fixed_keys[] = {
     { kActiveURL, kLargeSize },
+    { kNumExtensionsCount, kSmallSize },
 #if !defined(OS_ANDROID)
     { kGPUVendorID, kSmallSize },
     { kGPUDeviceID, kSmallSize },
@@ -82,43 +129,46 @@ size_t RegisterChromeCrashKeys() {
 #endif
   };
 
-  return base::debug::InitCrashKeys(keys, arraysize(keys), kSingleChunkLength);
+  // This dynamic set of keys is used for sets of key value pairs when gathering
+  // a collection of data, like command line switches or extension IDs.
+  std::vector<base::debug::CrashKey> keys(
+      fixed_keys, fixed_keys + arraysize(fixed_keys));
+
+  // Register the extension IDs.
+  {
+    // The fixed_keys names are string constants. Use static storage for
+    // formatted key names as well, since they will persist for the duration of
+    // the program.
+    static char formatted_keys[kExtensionIDMaxCount][sizeof(kExtensionID) + 1] =
+        {{ 0 }};
+    const size_t formatted_key_len = sizeof(formatted_keys[0]);
+    for (size_t i = 0; i < kExtensionIDMaxCount; ++i) {
+      int n = base::snprintf(
+          formatted_keys[i], formatted_key_len, kExtensionID, i);
+      DCHECK_GT(n, 0);
+      base::debug::CrashKey crash_key = { formatted_keys[i], kSmallSize };
+      keys.push_back(crash_key);
+    }
+  }
+
+  return base::debug::InitCrashKeys(&keys.at(0), keys.size(),
+                                    kSingleChunkLength);
 }
 
-const char kActiveURL[] = "url-chunk";
-
-#if !defined(OS_ANDROID)
-const char kGPUVendorID[] = "gpu-venid";
-const char kGPUDeviceID[] = "gpu-devid";
-#endif
-const char kGPUDriverVersion[] = "gpu-driver";
-const char kGPUPixelShaderVersion[] = "gpu-psver";
-const char kGPUVertexShaderVersion[] = "gpu-vsver";
-#if defined(OS_LINUX)
-const char kGPUVendor[] = "gpu-gl-vendor";
-const char kGPURenderer[] = "gpu-gl-renderer";
-#elif defined(OS_MACOSX)
-const char kGPUGLVersion[] = "gpu-glver";
-#endif
-
-#if defined(OS_MACOSX)
-namespace mac {
-
-const char kFirstNSException[] = "firstexception";
-const char kFirstNSExceptionTrace[] = "firstexception_bt";
-
-const char kLastNSException[] = "lastexception";
-const char kLastNSExceptionTrace[] = "lastexception_bt";
-
-const char kNSException[] = "nsexception";
-const char kNSExceptionTrace[] = "nsexception_bt";
-
-const char kSendAction[] = "sendaction";
-
-const char kZombie[] = "zombie";
-const char kZombieTrace[] = "zombie_dealloc_bt";
-
-}  // namespace mac
-#endif
+void SetActiveExtensions(const std::set<std::string>& extensions) {
+  base::debug::SetCrashKeyValue(kNumExtensionsCount,
+      base::StringPrintf("%" PRIuS, extensions.size()));
+
+  std::set<std::string>::const_iterator it = extensions.begin();
+  for (size_t i = 0; i < kExtensionIDMaxCount; ++i) {
+    std::string key = base::StringPrintf(kExtensionID, i);
+    if (it == extensions.end()) {
+      base::debug::ClearCrashKey(key);
+    } else {
+      base::debug::SetCrashKeyValue(key, *it);
+      ++it;
+    }
+  }
+}
 
 }  // namespace crash_keys

chrome/common/crash_keys.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_COMMON_CRASH_KEYS_H_
 #define CHROME_COMMON_CRASH_KEYS_H_
 
+#include <set>
+#include <string>
+
 #include "base/debug/crash_logging.h"
 
 namespace crash_keys {
@@ -13,11 +16,28 @@ namespace crash_keys {
 // reporting server. Returns the size of the union of all keys.
 size_t RegisterChromeCrashKeys();
 
+// Sets the list of "active" extensions in this process. We overload "active" to
+// mean different things depending on the process type:
+// - browser: all enabled extensions
+// - renderer: the unique set of extension ids from all content scripts
+// - extension: the id of each extension running in this process (there can be
+//   multiple because of process collapsing).
+void SetActiveExtensions(const std::set<std::string>& extensions);
+
 // Crash Key Name Constants ////////////////////////////////////////////////////
 
 // The URL of the active tab.
 extern const char kActiveURL[];
 
+// Installed extensions. |kExtensionID| should be formatted with an integer,
+// in the range [0, kExtensionIDMaxCount).
+const size_t kExtensionIDMaxCount = 10;
+extern const char kExtensionID[];
+// The total number of installed extensions, recorded in case it exceeds
+// kExtensionIDMaxCount. Also used in chrome/app, but defined here to avoid
+// a common->app dependency.
+extern const char kNumExtensionsCount[];
+
 // GPU information.
 #if !defined(OS_ANDROID)
 extern const char kGPUVendorID[];

chrome/renderer/extensions/dispatcher.cc

@@ -14,9 +14,9 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/crash_keys.h"
 #include "chrome/common/extensions/api/extension_api.h"
 #include "chrome/common/extensions/api/runtime.h"
 #include "chrome/common/extensions/background_info.h"
@@ -1394,7 +1394,7 @@ void Dispatcher::UpdateActiveExtensions() {
 
   std::set<std::string> active_extensions = active_extension_ids_;
   user_script_slave_->GetActiveExtensions(&active_extensions);
-  child_process_logging::SetActiveExtensions(active_extensions);
+  crash_keys::SetActiveExtensions(active_extensions);
 }
 
 void Dispatcher::OnUsingWebRequestAPI(