Skip to content

Commit f376884

Browse files
afakhryCommit bot
afakhry
authored and
Commit bot
committedSep 13, 2016
Add the most recent crash report IDs to feedback reports
Adds the IDs of crashes that occurred within the last hour before sending the feedback report, making sure that these IDs don't end up in the system_logs.txt file for privacy reasons. BUG=613606 Review-Url: https://codereview.chromium.org/2242833003 Cr-Commit-Position: refs/heads/master@{#418351}
1 parent 2ae993f commit f376884

File tree

7 files changed

+172
-21
lines changed

7 files changed

+172
-21
lines changed
 

‎chrome/browser/BUILD.gn

+2
Original file line numberDiff line numberDiff line change
@@ -2587,6 +2587,8 @@ split_static_library("browser") {
25872587
"feedback/system_logs/about_system_logs_fetcher.h",
25882588
"feedback/system_logs/log_sources/chrome_internal_log_source.cc",
25892589
"feedback/system_logs/log_sources/chrome_internal_log_source.h",
2590+
"feedback/system_logs/log_sources/crash_ids_source.cc",
2591+
"feedback/system_logs/log_sources/crash_ids_source.h",
25902592
"feedback/system_logs/log_sources/memory_details_log_source.cc",
25912593
"feedback/system_logs/log_sources/memory_details_log_source.h",
25922594
"feedback/system_logs/scrubbed_system_logs_fetcher.cc",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright 2016 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "chrome/browser/feedback/system_logs/log_sources/crash_ids_source.h"
6+
7+
#include <string>
8+
9+
#include "base/bind.h"
10+
#include "base/time/time.h"
11+
#include "chrome/browser/crash_upload_list/crash_upload_list.h"
12+
#include "components/feedback/feedback_report.h"
13+
14+
namespace system_logs {
15+
16+
namespace {
17+
18+
// The maximum number of crashes we retrieve from the crash list.
19+
constexpr size_t kMaxCrashesCountToRetrieve = 10;
20+
21+
// The length of the crash ID string.
22+
constexpr size_t kCrashIdStringSize = 16;
23+
24+
// We are only interested in crashes that took place within the last hour.
25+
constexpr base::TimeDelta kOneHourTimeDelta = base::TimeDelta::FromHours(1);
26+
27+
} // namespace
28+
29+
CrashIdsSource::CrashIdsSource()
30+
: SystemLogsSource("CrashId"),
31+
crash_upload_list_(CreateCrashUploadList(this)),
32+
pending_crash_list_loading_(false) {}
33+
34+
CrashIdsSource::~CrashIdsSource() {}
35+
36+
void CrashIdsSource::Fetch(const SysLogsSourceCallback& callback) {
37+
// Unretained since we own these callbacks.
38+
pending_requests_.emplace_back(base::Bind(
39+
&CrashIdsSource::RespondWithCrashIds, base::Unretained(this), callback));
40+
41+
if (pending_crash_list_loading_)
42+
return;
43+
44+
pending_crash_list_loading_ = true;
45+
crash_upload_list_->LoadUploadListAsynchronously();
46+
}
47+
48+
void CrashIdsSource::OnUploadListAvailable() {
49+
pending_crash_list_loading_ = false;
50+
51+
// Only get the IDs of crashes that occurred within the last hour (if any).
52+
std::vector<UploadList::UploadInfo> crashes;
53+
crash_upload_list_->GetUploads(kMaxCrashesCountToRetrieve, &crashes);
54+
const base::Time now = base::Time::Now();
55+
crash_ids_list_.clear();
56+
crash_ids_list_.reserve(kMaxCrashesCountToRetrieve *
57+
(kCrashIdStringSize + 2));
58+
59+
// The feedback server expects the crash IDs to be a comma-separated list.
60+
for (const auto& crash_info : crashes) {
61+
if (now - crash_info.capture_time < kOneHourTimeDelta) {
62+
const std::string& crash_id = crash_info.upload_id;
63+
crash_ids_list_.append(crash_ids_list_.empty() ? crash_id
64+
: ", " + crash_id);
65+
}
66+
}
67+
68+
for (const auto& request : pending_requests_)
69+
request.Run();
70+
71+
pending_requests_.clear();
72+
}
73+
74+
void CrashIdsSource::RespondWithCrashIds(
75+
const SysLogsSourceCallback& callback) const {
76+
std::unique_ptr<SystemLogsResponse> response(new SystemLogsResponse());
77+
(*response)[feedback::FeedbackReport::kCrashReportIdsKey] = crash_ids_list_;
78+
79+
// We must respond anyways.
80+
callback.Run(response.get());
81+
}
82+
83+
} // namespace system_logs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2016 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef CHROME_BROWSER_FEEDBACK_SYSTEM_LOGS_LOG_SOURCES_CRASH_IDS_SOURCE_H_
6+
#define CHROME_BROWSER_FEEDBACK_SYSTEM_LOGS_LOG_SOURCES_CRASH_IDS_SOURCE_H_
7+
8+
#include <vector>
9+
10+
#include "base/callback_forward.h"
11+
#include "chrome/browser/feedback/system_logs/system_logs_fetcher_base.h"
12+
#include "components/upload_list/crash_upload_list.h"
13+
#include "components/upload_list/upload_list.h"
14+
15+
namespace system_logs {
16+
17+
// Extract the most recent crash IDs (if any) and adds them to the system logs.
18+
class CrashIdsSource : public SystemLogsSource, public UploadList::Delegate {
19+
public:
20+
CrashIdsSource();
21+
~CrashIdsSource() override;
22+
23+
// SystemLogsSource:
24+
void Fetch(const SysLogsSourceCallback& callback) override;
25+
26+
// UploadList::Delegate:
27+
void OnUploadListAvailable() override;
28+
29+
private:
30+
void RespondWithCrashIds(const SysLogsSourceCallback& callback) const;
31+
32+
scoped_refptr<CrashUploadList> crash_upload_list_;
33+
34+
// A comma-separated list of crash IDs as expected by the server.
35+
std::string crash_ids_list_;
36+
37+
// Contains any pending fetch requests waiting for the crash upload list to
38+
// finish loading.
39+
std::vector<base::Closure> pending_requests_;
40+
41+
// True if the crash list is currently being loaded.
42+
bool pending_crash_list_loading_;
43+
44+
DISALLOW_COPY_AND_ASSIGN(CrashIdsSource);
45+
};
46+
47+
} // namespace system_logs
48+
49+
#endif // CHROME_BROWSER_FEEDBACK_SYSTEM_LOGS_LOG_SOURCES_CRASH_IDS_SOURCE_H_

‎chrome/browser/feedback/system_logs/scrubbed_system_logs_fetcher.cc

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "base/bind_helpers.h"
99
#include "build/build_config.h"
1010
#include "chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.h"
11+
#include "chrome/browser/feedback/system_logs/log_sources/crash_ids_source.h"
1112
#include "chrome/browser/feedback/system_logs/log_sources/memory_details_log_source.h"
1213
#include "content/public/browser/browser_thread.h"
1314

@@ -26,6 +27,7 @@ namespace system_logs {
2627

2728
ScrubbedSystemLogsFetcher::ScrubbedSystemLogsFetcher() {
2829
data_sources_.push_back(new ChromeInternalLogSource());
30+
data_sources_.push_back(new CrashIdsSource());
2931
data_sources_.push_back(new MemoryDetailsLogSource());
3032

3133
#if defined(OS_CHROMEOS)

‎components/feedback/feedback_common.cc

+17-10
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "base/memory/ptr_util.h"
1010
#include "base/strings/string_util.h"
11+
#include "components/feedback/feedback_report.h"
1112
#include "components/feedback/feedback_util.h"
1213
#include "components/feedback/proto/common.pb.h"
1314
#include "components/feedback/proto/dom.pb.h"
@@ -22,24 +23,24 @@ constexpr int kChromeOSProductId = 208;
2223
constexpr int kChromeBrowserProductId = 237;
2324
#endif
2425

25-
const char kMultilineIndicatorString[] = "<multiline>\n";
26-
const char kMultilineStartString[] = "---------- START ----------\n";
27-
const char kMultilineEndString[] = "---------- END ----------\n\n";
26+
constexpr char kMultilineIndicatorString[] = "<multiline>\n";
27+
constexpr char kMultilineStartString[] = "---------- START ----------\n";
28+
constexpr char kMultilineEndString[] = "---------- END ----------\n\n";
2829

2930
// The below thresholds were chosen arbitrarily to conveniently show small data
3031
// as part of the report itself without having to look into the system_logs.zip
3132
// file.
32-
const size_t kFeedbackMaxLength = 1024;
33-
const size_t kFeedbackMaxLineCount = 10;
33+
constexpr size_t kFeedbackMaxLength = 1024;
34+
constexpr size_t kFeedbackMaxLineCount = 10;
3435

35-
const base::FilePath::CharType kLogsFilename[] =
36+
constexpr base::FilePath::CharType kLogsFilename[] =
3637
FILE_PATH_LITERAL("system_logs.txt");
37-
const char kLogsAttachmentName[] = "system_logs.zip";
38+
constexpr char kLogsAttachmentName[] = "system_logs.zip";
3839

39-
const char kZipExt[] = ".zip";
40+
constexpr char kZipExt[] = ".zip";
4041

41-
const char kPngMimeType[] = "image/png";
42-
const char kArbitraryMimeType[] = "application/octet-stream";
42+
constexpr char kPngMimeType[] = "image/png";
43+
constexpr char kArbitraryMimeType[] = "application/octet-stream";
4344

4445
// Determine if the given feedback value is small enough to not need to
4546
// be compressed.
@@ -66,6 +67,11 @@ std::unique_ptr<std::string> LogsToString(
6667
base::TrimString(key, "\n ", &key);
6768
base::TrimString(value, "\n ", &value);
6869

70+
// We must avoid adding the crash IDs to the system_logs.txt file for
71+
// privacy reasons. They should just be part of the product specific data.
72+
if (key == feedback::FeedbackReport::kCrashReportIdsKey)
73+
continue;
74+
6975
if (value.find("\n") != std::string::npos) {
7076
syslogs_string->append(key + "=" + kMultilineIndicatorString +
7177
kMultilineStartString + value + "\n" +
@@ -234,6 +240,7 @@ void FeedbackCommon::CompressLogs() {
234240
std::move(logs));
235241
}
236242
}
243+
237244
void FeedbackCommon::AddFilesAndLogsToReport(
238245
userfeedback::ExtensionSubmit* feedback_data) const {
239246
for (size_t i = 0; i < attachments(); ++i) {

‎components/feedback/feedback_report.cc

+10-6
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,8 @@ FeedbackReport::FeedbackReport(
5454
&WriteReportOnBlockingPool, reports_path_, file_, data_));
5555
}
5656

57-
FeedbackReport::~FeedbackReport() {}
58-
59-
void FeedbackReport::DeleteReportOnDisk() {
60-
reports_task_runner_->PostTask(FROM_HERE, base::Bind(
61-
base::IgnoreResult(&base::DeleteFile), file_, false));
62-
}
57+
// static
58+
const char FeedbackReport::kCrashReportIdsKey[] = "crash_report_ids";
6359

6460
// static
6561
void FeedbackReport::LoadReportsAndQueue(
@@ -81,4 +77,12 @@ void FeedbackReport::LoadReportsAndQueue(
8177
}
8278
}
8379

80+
void FeedbackReport::DeleteReportOnDisk() {
81+
reports_task_runner_->PostTask(
82+
FROM_HERE,
83+
base::Bind(base::IgnoreResult(&base::DeleteFile), file_, false));
84+
}
85+
86+
FeedbackReport::~FeedbackReport() {}
87+
8488
} // namespace feedback

‎components/feedback/feedback_report.h

+9-5
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,22 @@ class FeedbackReport : public base::RefCounted<FeedbackReport> {
3131
const std::string& data,
3232
scoped_refptr<base::SequencedTaskRunner> task_runner);
3333

34+
// The ID of the product specific data for the crash report IDs as stored by
35+
// the feedback server.
36+
static const char kCrashReportIdsKey[];
37+
38+
// Loads the reports still on disk and queues then using the given callback.
39+
// This call blocks on the file reads.
40+
static void LoadReportsAndQueue(const base::FilePath& user_dir,
41+
QueueCallback callback);
42+
3443
// Stops the disk write of the report and deletes the report file if already
3544
// written.
3645
void DeleteReportOnDisk();
3746

3847
const base::Time& upload_at() const { return upload_at_; }
3948
const std::string& data() const { return data_; }
4049

41-
// Loads the reports still on disk and queues then using the given callback.
42-
// This call blocks on the file reads.
43-
static void LoadReportsAndQueue(const base::FilePath& user_dir,
44-
QueueCallback callback);
45-
4650
private:
4751
friend class base::RefCounted<FeedbackReport>;
4852
virtual ~FeedbackReport();

0 commit comments

Comments
 (0)
Please sign in to comment.