Allow consumers of components/crash to use crash_keys_win and base::d…

…ebug::InitCrashKeys without depending on breakpad.

InitCrashKeys (in base/) requires a chunk_max_length value that depends
on the platform-specific crash reporter being used. This change
introduces a crash_keys::kChunkMaxLength constant (published in
core/common/crash_keys.h) that has the proper platform-specific
value. The correctness of this value is enforced via static_asserts in
the per-platform breakpad_foo implementation files. This change allows
consumers to call InitCrashKeys without needing platform-specific hacks
to compute values from various breakpad headers.

This change also gently massages CrashKeysWin
(content/app/crash_keys_win.h) so that consumers don't need to include
breakpad headers.

Along with these two changes are varioud DEPS, BUILD.gn, and .gyp*
tweaks to keep everything building smoothly.

BUG=548216
TBR=thakis@chromium.org,droger@chromium.org,gunsch@chromium.org

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

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

chrome/chrome_common.gypi

@@ -433,9 +433,6 @@
             ['include', '^common/translate'],
             ['include', '^common/zip'],
           ],
-          'include_dirs': [
-            '<(DEPTH)/breakpad/src',
-          ],
         }],
         ['disable_nacl==0', {
           'dependencies': [
@@ -499,7 +496,6 @@
         }],
         ['OS=="win"', {
           'include_dirs': [
-            '<(DEPTH)/breakpad/src',
             '<(DEPTH)/third_party/wtl/include',
           ],
           'dependencies': [
@@ -516,9 +512,6 @@
             '../third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:google_toolbox_for_mac',
             '../third_party/mach_override/mach_override.gyp:mach_override',
           ],
-          'include_dirs': [
-            '<(DEPTH)/breakpad/src',
-          ],
           'sources!': [
             'common/channel_info_posix.cc',
           ],

chrome/common/BUILD.gn

@@ -155,7 +155,6 @@ static_library("common") {
   if (is_win || is_mac) {
     sources +=
         rebase_path(gypi_values.chrome_common_win_mac_sources, ".", "//chrome")
-    public_deps += [ "//breakpad:client" ]
   }
   if (is_win || is_mac || is_chromeos) {
     sources += rebase_path(gypi_values.chrome_common_networking_private_sources,

chrome/common/DEPS

@@ -1,5 +1,4 @@
 include_rules = [
-  "+breakpad",  # For Breakpad constants.
   "+chrome/grit",  # For generated headers
   "+chromeos",  # For chromeos_switches.h
   "+components/autofill/content/common",

chrome/common/crash_keys.cc

@@ -16,40 +16,14 @@
 #include "content/public/common/content_switches.h"
 #include "ipc/ipc_switches.h"
 
-// Breakpad dependencies exist only on Windows and Mac desktop. Exclude them
-// from "gn check" to avoid failures on Linux (it doesn't understand ifdefs).
-#if defined(OS_MACOSX)
-#include "breakpad/src/common/simple_string_dictionary.h"  // nogncheck
-#elif defined(OS_WIN)
-#include "breakpad/src/client/windows/common/ipc_protocol.h"  // nogncheck
-#elif defined(OS_CHROMEOS)
+#if defined(OS_CHROMEOS)
 #include "chrome/common/chrome_switches.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "ui/gl/gl_switches.h"
 #endif
 
 namespace crash_keys {
 
-// The maximum lengths specified by breakpad include the trailing NULL, so
-// the actual length of the string is one less.
-#if defined(OS_MACOSX)
-static const size_t kSingleChunkLength =
-    google_breakpad::SimpleStringDictionary::value_size - 1;
-#elif defined(OS_WIN)
-static const size_t kSingleChunkLength =
-    google_breakpad::CustomInfoEntry::kValueMaxLength - 1;
-#else
-static const size_t kSingleChunkLength = 63;
-#endif
-
-// Guarantees for crash key sizes.
-static_assert(kSmallSize <= kSingleChunkLength,
-              "crash key chunk size too small");
-#if defined(OS_MACOSX)
-static_assert(kMediumSize <= kSingleChunkLength,
-              "mac has medium size crash key chunks");
-#endif
-
 const char kActiveURL[] = "url-chunk";
 
 const char kFontKeyName[] = "font_key_name";
@@ -223,8 +197,7 @@ size_t RegisterChromeCrashKeys() {
     }
   }
 
-  return base::debug::InitCrashKeys(&keys.at(0), keys.size(),
-                                    kSingleChunkLength);
+  return base::debug::InitCrashKeys(&keys.at(0), keys.size(), kChunkMaxLength);
 }
 
 static bool IsBoringSwitch(const std::string& flag) {

chromecast/crash/cast_crash_keys.cc

@@ -21,9 +21,9 @@ const size_t kLargeSize = kSmallSize * 16;
 
 // The maximum lengths specified by breakpad include the trailing NULL, so
 // the actual length of the string is one less.
-static const size_t kSingleChunkLength = 63;
+static const size_t kChunkMaxLength = 63;
 
-}
+}  // namespace
 
 namespace chromecast {
 namespace crash_keys {
@@ -46,8 +46,8 @@ size_t RegisterCastCrashKeys() {
   };
 
   return base::debug::InitCrashKeys(fixed_keys, arraysize(fixed_keys),
-                                    kSingleChunkLength);
+                                    kChunkMaxLength);
 }
 
-}  // namespace chromecast
 }  // namespace crash_keys
+}  // namespace chromecast

components/crash.gypi

@@ -252,6 +252,7 @@
               ],
               'include_dirs': [
                 '..',
+                '../breakpad/src',
               ],
             }],
           ],

components/crash/content/app/BUILD.gn

@@ -30,8 +30,10 @@ source_set("app") {
 
   defines = [ "CRASH_IMPLEMENTATION" ]
 
-  deps = [
+  public_deps = [
     ":app_non_mac",
+  ]
+  deps = [
     "//base",
   ]
 
@@ -134,6 +136,7 @@ source_set("app_breakpad_mac_to_be_deleted") {
       "//base",
       "//base:base_static",
       "//breakpad",
+      "//breakpad:client",
     ]
   }
 }

components/crash/content/app/breakpad_linux.cc

@@ -43,6 +43,7 @@
 #include "build/build_config.h"
 #include "components/crash/content/app/breakpad_linux_impl.h"
 #include "components/crash/content/app/crash_reporter_client.h"
+#include "components/crash/core/common/crash_keys.h"
 #include "content/public/common/content_descriptors.h"
 
 #if defined(OS_ANDROID)
@@ -1763,6 +1764,11 @@ void HandleCrashDump(const BreakpadInfo& info) {
 }
 
 void InitCrashReporter(const std::string& process_type) {
+  // The maximum lengths specified by breakpad include the trailing NULL, so the
+  // actual length of the chunk is one less.
+  static_assert(crash_keys::kChunkMaxLength == 63, "kChunkMaxLength mismatch");
+  static_assert(crash_keys::kSmallSize <= crash_keys::kChunkMaxLength,
+                "crash key chunk size too small");
 #if defined(OS_ANDROID)
   // This will guarantee that the BuildInfo has been initialized and subsequent
   // calls will not require memory allocation.

components/crash/content/app/breakpad_mac.mm

@@ -25,7 +25,9 @@
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
 #import "breakpad/src/client/mac/Framework/Breakpad.h"
+#include "breakpad/src/common/simple_string_dictionary.h"
 #include "components/crash/content/app/crash_reporter_client.h"
+#include "components/crash/core/common/crash_keys.h"
 
 using crash_reporter::GetCrashReporterClient;
 
@@ -155,6 +157,14 @@ bool IsCrashReporterEnabled() {
 
 // Only called for a branded build of Chrome.app.
 void InitCrashReporter(const std::string& process_type) {
+  // The maximum lengths specified by breakpad include the trailing NULL, so the
+  // actual length of the chunk is one less.
+  static_assert(google_breakpad::SimpleStringDictionary::value_size - 1 ==
+                crash_keys::kChunkMaxLength, "kChunkMaxLength mismatch");
+  static_assert(crash_keys::kSmallSize <= crash_keys::kChunkMaxLength,
+                "crash key chunk size too small for small values");
+  static_assert(crash_keys::kMediumSize <= crash_keys::kChunkMaxLength,
+                "crash key chunk size too small for medium values");
   DCHECK(!gBreakpadRef);
   base::mac::ScopedNSAutoreleasePool autorelease_pool;
 

components/crash/content/app/breakpad_win.cc

@@ -32,10 +32,12 @@
 #include "base/win/pe_image.h"
 #include "base/win/registry.h"
 #include "base/win/win_util.h"
+#include "breakpad/src/client/windows/common/ipc_protocol.h"
 #include "breakpad/src/client/windows/handler/exception_handler.h"
 #include "components/crash/content/app/crash_keys_win.h"
 #include "components/crash/content/app/crash_reporter_client.h"
 #include "components/crash/content/app/hard_error_handler_win.h"
+#include "components/crash/core/common/crash_keys.h"
 #include "content/public/common/result_codes.h"
 #include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sidestep/preamble_patcher.h"
@@ -527,6 +529,12 @@ void InitDefaultCrashCallback(LPTOP_LEVEL_EXCEPTION_FILTER filter) {
 }
 
 void InitCrashReporter(const std::string& process_type_switch) {
+  // The maximum lengths specified by breakpad include the trailing NULL, so the
+  // actual length of the chunk is one less.
+  static_assert(google_breakpad::CustomInfoEntry::kValueMaxLength - 1 ==
+                crash_keys::kChunkMaxLength, "kChunkMaxLength mismatch");
+  static_assert(crash_keys::kSmallSize <= crash_keys::kChunkMaxLength,
+                "crash key chunk size too small");
   const base::CommandLine& command = *base::CommandLine::ForCurrentProcess();
   if (command.HasSwitch(switches::kDisableBreakpad))
     return;

components/crash/content/app/crash_keys_win.cc

@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "breakpad/src/client/windows/common/ipc_protocol.h"
 #include "components/crash/content/app/crash_reporter_client.h"
 
 namespace breakpad {
@@ -27,7 +28,9 @@ const size_t kMaxDynamicEntries = 256;
 
 CrashKeysWin* CrashKeysWin::keeper_;
 
-CrashKeysWin::CrashKeysWin() : dynamic_keys_offset_(0) {
+CrashKeysWin::CrashKeysWin()
+    : custom_entries_(new std::vector<google_breakpad::CustomInfoEntry>),
+      dynamic_keys_offset_(0) {
   DCHECK_EQ(static_cast<CrashKeysWin*>(NULL), keeper_);
   keeper_ = this;
 }
@@ -56,7 +59,7 @@ void CrashKeysWin::SetPluginPath(const std::wstring& path) {
   for (chunk_start = 0; chunk_start < path.size(); chunk_index++) {
     size_t chunk_length = std::min(kChunkSize, path.size() - chunk_start);
 
-    custom_entries_.push_back(google_breakpad::CustomInfoEntry(
+    custom_entries_->push_back(google_breakpad::CustomInfoEntry(
         base::StringPrintf(L"plugin-path-chunk-%i", chunk_index + 1).c_str(),
         path.substr(chunk_start, chunk_length).c_str()));
 
@@ -68,7 +71,7 @@ void CrashKeysWin::SetPluginPath(const std::wstring& path) {
 void CrashKeysWin::SetBreakpadDumpPath(CrashReporterClient* crash_client) {
   base::FilePath crash_dumps_dir_path;
   if (crash_client->GetAlternativeCrashDumpLocation(&crash_dumps_dir_path)) {
-    custom_entries_.push_back(google_breakpad::CustomInfoEntry(
+    custom_entries_->push_back(google_breakpad::CustomInfoEntry(
         L"breakpad-dump-location", crash_dumps_dir_path.value().c_str()));
   }
 }
@@ -94,24 +97,24 @@ CrashKeysWin::GetCustomInfo(const std::wstring& exe_path,
 
   // We only expect this method to be called once per process.
   // Common enties
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"ver", version.c_str()));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"prod", product.c_str()));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"plat", L"Win32"));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"ptype", type.c_str()));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(
       L"pid", base::IntToString16(::GetCurrentProcessId()).c_str()));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"channel", channel_name.c_str()));
-  custom_entries_.push_back(
+  custom_entries_->push_back(
       google_breakpad::CustomInfoEntry(L"profile-type", profile_type.c_str()));
 
   if (!special_build.empty()) {
-    custom_entries_.push_back(
+    custom_entries_->push_back(
         google_breakpad::CustomInfoEntry(L"special", special_build.c_str()));
   }
 
@@ -133,18 +136,18 @@ CrashKeysWin::GetCustomInfo(const std::wstring& exe_path,
 
   // Create space for dynamic ad-hoc keys. The names and values are set using
   // the API defined in base/debug/crash_logging.h.
-  dynamic_keys_offset_ = custom_entries_.size();
+  dynamic_keys_offset_ = custom_entries_->size();
   for (size_t i = 0; i < kMaxDynamicEntries; ++i) {
     // The names will be mutated as they are set. Un-numbered since these are
     // merely placeholders. The name cannot be empty because Breakpad's
     // HTTPUpload will interpret that as an invalid parameter.
-    custom_entries_.push_back(
+    custom_entries_->push_back(
         google_breakpad::CustomInfoEntry(L"unspecified-crash-key", L""));
   }
 
   static google_breakpad::CustomClientInfo custom_client_info;
-  custom_client_info.entries = &custom_entries_.front();
-  custom_client_info.count = custom_entries_.size();
+  custom_client_info.entries = &custom_entries_->front();
+  custom_client_info.count = custom_entries_->size();
 
   return &custom_client_info;
 }
@@ -169,7 +172,7 @@ void CrashKeysWin::SetCrashKeyValue(
   if (it == dynamic_entries_.end()) {
     if (dynamic_entries_.size() >= kMaxDynamicEntries)
       return;
-    entry = &custom_entries_[dynamic_keys_offset_++];
+    entry = &(*custom_entries_)[dynamic_keys_offset_++];
     dynamic_entries_.insert(std::make_pair(safe_key, entry));
   } else {
     entry = it->second;

components/crash/content/app/crash_keys_win.h

@@ -10,10 +10,8 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
-#include "breakpad/src/client/windows/common/ipc_protocol.h"
-#include "breakpad/src/client/windows/handler/exception_handler.h"
-
 
 namespace base {
 class CommandLine;
@@ -23,6 +21,11 @@ namespace crash_reporter {
 class CrashReporterClient;
 }
 
+namespace google_breakpad {
+struct CustomClientInfo;
+struct CustomInfoEntry;
+}
+
 namespace breakpad {
 
 // Manages the breakpad key/value pair stash, there may only be one instance
@@ -54,7 +57,7 @@ class CrashKeysWin {
 
   const std::vector<google_breakpad::CustomInfoEntry>& custom_info_entries()
       const {
-    return custom_entries_;
+    return *custom_entries_;
   }
 
   static CrashKeysWin* keeper() { return keeper_; }
@@ -65,7 +68,7 @@ class CrashKeysWin {
   void SetBreakpadDumpPath(crash_reporter::CrashReporterClient* crash_client);
 
   // Must not be resized after GetCustomInfo is invoked.
-  std::vector<google_breakpad::CustomInfoEntry> custom_entries_;
+  scoped_ptr<std::vector<google_breakpad::CustomInfoEntry>> custom_entries_;
 
   typedef std::map<std::wstring, google_breakpad::CustomInfoEntry*>
       DynamicEntriesMap;

components/crash/content/app/crash_keys_win_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/strings/stringprintf.h"
+#include "breakpad/src/client/windows/common/ipc_protocol.h"
 #include "components/crash/content/app/crash_reporter_client.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"

components/crash/core/common/crash_keys.h

@@ -23,6 +23,16 @@ void SetVariationsList(const std::vector<std::string>& variations);
 
 // Crash Key Constants /////////////////////////////////////////////////////////
 
+// kChunkMaxLength is the platform-specific maximum size that a value in a
+// single chunk can be; see base::debug::InitCrashKeys. The maximum lengths
+// specified by breakpad include the trailing NULL, so the actual length of the
+// chunk is one less.
+#if defined(OS_MACOSX)
+const size_t kChunkMaxLength = 255;
+#else  // OS_MACOSX
+const size_t kChunkMaxLength = 63;
+#endif  // !OS_MACOSX
+
 // A small crash key, guaranteed to never be split into multiple pieces.
 const size_t kSmallSize = 63;