Skip to content

Commit

Permalink
Set the experiment/variation info in crash reports using the crash ke…
Browse files Browse the repository at this point in the history
…y logging system.

BUG=77656
R=asvitkine@chromium.org, jochen@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@225186 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rsesek@chromium.org committed Sep 25, 2013
1 parent d0e8a90 commit 42d46a8
Show file tree
Hide file tree
Showing 17 changed files with 44 additions and 434 deletions.
58 changes: 0 additions & 58 deletions chrome/app/breakpad_field_trial_win.cc

This file was deleted.

22 changes: 0 additions & 22 deletions chrome/app/breakpad_field_trial_win.h

This file was deleted.

20 changes: 0 additions & 20 deletions chrome/app/breakpad_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1264,26 +1264,6 @@ void HandleCrashDump(const BreakpadInfo& info) {
writer.Flush();
}

if (*child_process_logging::g_num_variations) {
writer.AddPairString("num-experiments",
child_process_logging::g_num_variations);
writer.AddBoundary();
writer.Flush();
}

unsigned variation_chunks_len =
my_strlen(child_process_logging::g_variation_chunks);
if (variation_chunks_len) {
static const char variation_msg[] = "experiment-chunk-";
static const unsigned kMaxVariationsLen =
kMaxReportedVariationChunks * kMaxVariationChunkSize;
writer.AddPairDataInChunks(variation_msg, sizeof(variation_msg) - 1,
child_process_logging::g_variation_chunks,
std::min(variation_chunks_len, kMaxVariationsLen),
kMaxVariationChunkSize,
true /* Strip whitespace since variation chunks are padded. */);
}

if (info.oom_size) {
char oom_size_str[kUint64StringSize];
const unsigned oom_size_len = my_uint64_len(info.oom_size);
Expand Down
83 changes: 0 additions & 83 deletions chrome/app/breakpad_unittest_win.cc

This file was deleted.

28 changes: 0 additions & 28 deletions chrome/app/breakpad_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "base/win/registry.h"
#include "base/win/win_util.h"
#include "breakpad/src/client/windows/handler/exception_handler.h"
#include "chrome/app/breakpad_field_trial_win.h"
#include "chrome/app/hard_error_handler_win.h"
#include "chrome/common/child_process_logging.h"
#include "chrome/common/chrome_constants.h"
Expand All @@ -54,16 +53,12 @@ namespace breakpad_win {
// is way too too fragile. See
// https://code.google.com/p/chromium/issues/detail?id=137062.
std::vector<google_breakpad::CustomInfoEntry>* g_custom_entries = NULL;
size_t g_num_of_experiments_offset = 0;
size_t g_experiment_chunks_offset = 0;
bool g_deferred_crash_uploads = false;

} // namespace breakpad_win

using breakpad_win::g_custom_entries;
using breakpad_win::g_deferred_crash_uploads;
using breakpad_win::g_experiment_chunks_offset;
using breakpad_win::g_num_of_experiments_offset;

namespace {

Expand Down Expand Up @@ -426,21 +421,6 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& exe_path,
if (use_crash_service)
SetBreakpadDumpPath();

g_num_of_experiments_offset = g_custom_entries->size();
g_custom_entries->push_back(
google_breakpad::CustomInfoEntry(L"num-experiments", L"N/A"));

g_experiment_chunks_offset = g_custom_entries->size();
// We depend on this in UpdateExperiments...
DCHECK_NE(0UL, g_experiment_chunks_offset);
// And the test code depends on this.
DCHECK_EQ(g_num_of_experiments_offset + 1, g_experiment_chunks_offset);
// one-based index for the name suffix.
for (int i = 1; i <= kMaxReportedVariationChunks; ++i) {
g_custom_entries->push_back(google_breakpad::CustomInfoEntry(
base::StringPrintf(L"experiment-chunk-%i", i).c_str(), L""));
}

// Create space for dynamic ad-hoc keys. The names and values are set using
// the API defined in base/debug/crash_logging.h.
g_dynamic_keys_offset = g_custom_entries->size();
Expand Down Expand Up @@ -925,11 +905,3 @@ void InitCrashReporter() {
void InitDefaultCrashCallback(LPTOP_LEVEL_EXCEPTION_FILTER filter) {
previous_filter = SetUnhandledExceptionFilter(filter);
}

void StringVectorToCStringVector(const std::vector<std::wstring>& wstrings,
std::vector<const wchar_t*>* cstrings) {
cstrings->clear();
cstrings->reserve(wstrings.size());
for (size_t i = 0; i < wstrings.size(); ++i)
cstrings->push_back(wstrings[i].c_str());
}
5 changes: 1 addition & 4 deletions chrome/browser/ui/webui/version_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ void VersionHandler::HandleRequestVersionInfo(const ListValue* args) {
}
#else
// In release mode, display the hashes only.
std::vector<string16> active_groups;
chrome_variations::GetFieldTrialActiveGroupIdsAsStrings(&active_groups);
for (size_t i = 0; i < active_groups.size(); ++i)
variations.push_back(UTF16ToASCII(active_groups[i]));
chrome_variations::GetFieldTrialActiveGroupIdsAsStrings(&variations);
#endif

ListValue variations_list;
Expand Down
2 changes: 0 additions & 2 deletions chrome/chrome_exe.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
'enable_wexit_time_destructors': 1,
},
'sources': [
'app/breakpad_field_trial_win.cc',
'app/breakpad_field_trial_win.h',
'app/breakpad_win.cc',
'app/breakpad_win.h',
'app/chrome_exe_main_aura.cc',
Expand Down
2 changes: 0 additions & 2 deletions chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -2715,9 +2715,7 @@
'..',
],
'sources': [
'app/breakpad_field_trial_win.cc',
'app/breakpad_win.cc',
'app/breakpad_unittest_win.cc',
'app/delay_load_hook_win.cc',
'app/delay_load_hook_win.h',
'app/delay_load_hook_unittest_win.cc',
Expand Down
24 changes: 1 addition & 23 deletions chrome/common/child_process_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,17 @@
#ifndef CHROME_COMMON_CHILD_PROCESS_LOGGING_H_
#define CHROME_COMMON_CHILD_PROCESS_LOGGING_H_

#include <set>
#include <string>
#include <vector>

#include "base/basictypes.h"
#include "base/debug/crash_logging.h"
#include "base/strings/string16.h"

// 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.
static const size_t kMaxReportedVariationChunks = 15;

// The maximum size of a variation chunk. This size was picked to be
// consistent between platforms and the value was chosen from the Windows
// limit of google_breakpad::CustomInfoEntry::kValueMaxLength.
static const size_t kMaxVariationChunkSize = 64;

namespace child_process_logging {

#if defined(OS_POSIX) && !defined(OS_MACOSX)
// These are declared here so the crash reporter can access them directly in
// compromised context without going through the standard library.
extern char g_client_id[];
extern char g_num_variations[];
extern char g_variation_chunks[];

// Assume command line switches are less than 64 chars.
static const size_t kSwitchLen = 64;
Expand All @@ -43,18 +28,11 @@ void SetClientId(const std::string& client_id);
// id in |client_id| if it's known, an empty string otherwise.
std::string GetClientId();

// Initialize the list of experiment info to send along with crash reports.
void SetExperimentList(const std::vector<string16>& state);

} // namespace child_process_logging

#if defined(OS_WIN)
namespace child_process_logging {

// Sets up the base/debug/crash_logging.h mechanism.
void Init();
#endif // defined(OS_WIN)

} // namespace child_process_logging
#endif // defined(OS_WIN)

#endif // CHROME_COMMON_CHILD_PROCESS_LOGGING_H_
29 changes: 0 additions & 29 deletions chrome/common/child_process_logging_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
#import <Foundation/Foundation.h>

#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/common/metrics/variations/variations_util.h"
#include "chrome/installer/util/google_update_settings.h"

namespace child_process_logging {
Expand Down Expand Up @@ -45,31 +43,4 @@ void SetClientId(const std::string& client_id) {
return std::string(g_client_id);
}

void SetExperimentList(const std::vector<string16>& experiments) {
// These should match the corresponding strings in breakpad_win.cc.
const char* kNumExperimentsKey = "num-experiments";
const char* kExperimentChunkFormat = "experiment-chunk-%zu"; // 1-based

std::vector<string16> chunks;
chrome_variations::GenerateVariationChunks(experiments, &chunks);

// Store up to |kMaxReportedVariationChunks| chunks.
for (size_t i = 0; i < kMaxReportedVariationChunks; ++i) {
std::string key = base::StringPrintf(kExperimentChunkFormat, i + 1);
if (i < chunks.size()) {
std::string value = UTF16ToUTF8(chunks[i]);
SetCrashKeyValue(key, value);
} else {
ClearCrashKey(key);
}
}

// Make note of the total number of experiments, which may be greater than
// what was able to fit in |kMaxReportedVariationChunks|. This is useful when
// correlating stability with the number of experiments running
// simultaneously.
SetCrashKeyValue(kNumExperimentsKey,
base::StringPrintf("%zu", experiments.size()));
}

} // namespace child_process_logging
Loading

0 comments on commit 42d46a8

Please sign in to comment.