Add UploadReport to manage database resources during upload

This change adds CrashReportDatabase::UploadReport which owns the
report's file handle during upload. An upload is recorded as a success
by calling RecordUploadComplete(). If RecordUploadComplete() is not
called, the operation is recorded as a failure when the UploadReport is
destroyed.

Bug: crashpad:206
Change-Id: I8385d08d52185ad30b06a3ed054de9812ae006a2
Reviewed-on: https://chromium-review.googlesource.com/917983
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This commit is contained in:
Joshua Peraza 2018-02-15 08:17:12 -08:00 committed by Commit Bot
parent 7d5487fc44
commit c406797ce6
8 changed files with 168 additions and 153 deletions

View File

@ -55,4 +55,28 @@ bool CrashReportDatabase::NewReport::Initialize(
return true; return true;
} }
CrashReportDatabase::UploadReport::UploadReport()
: Report(), reader_(std::make_unique<FileReader>()), database_(nullptr) {}
CrashReportDatabase::UploadReport::~UploadReport() {
if (database_) {
database_->RecordUploadAttempt(this, false, std::string());
}
}
bool CrashReportDatabase::UploadReport::Initialize(const base::FilePath path,
CrashReportDatabase* db) {
database_ = db;
return reader_->Open(path);
}
CrashReportDatabase::OperationStatus CrashReportDatabase::RecordUploadComplete(
std::unique_ptr<const UploadReport> report_in,
const std::string& id) {
UploadReport* report = const_cast<UploadReport*>(report_in.get());
report->database_ = nullptr;
return RecordUploadAttempt(report, true, id);
}
} // namespace crashpad } // namespace crashpad

View File

@ -24,6 +24,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "util/file/file_io.h" #include "util/file/file_io.h"
#include "util/file/file_reader.h"
#include "util/file/file_writer.h" #include "util/file/file_writer.h"
#include "util/file/scoped_remove_file.h" #include "util/file/scoped_remove_file.h"
#include "util/misc/metrics.h" #include "util/misc/metrics.h"
@ -49,7 +50,7 @@ class Settings;
//! processed, or it was has been brought back from 'Completed' state by //! processed, or it was has been brought back from 'Completed' state by
//! user request. //! user request.
//! 3. Completed: The report has been locally processed, either by uploading //! 3. Completed: The report has been locally processed, either by uploading
//! it to a collection server and calling RecordUploadAttempt(), or by //! it to a collection server and calling RecordUploadComplete(), or by
//! calling SkipReportUpload(). //! calling SkipReportUpload().
class CrashReportDatabase { class CrashReportDatabase {
public: public:
@ -100,7 +101,7 @@ class CrashReportDatabase {
//! \brief A crash report that is in the process of being written. //! \brief A crash report that is in the process of being written.
//! //!
//! An instance of this struct should be created via PrepareNewCrashReport(). //! An instance of this class should be created via PrepareNewCrashReport().
class NewReport { class NewReport {
public: public:
NewReport(); NewReport();
@ -128,6 +129,30 @@ class CrashReportDatabase {
DISALLOW_COPY_AND_ASSIGN(NewReport); DISALLOW_COPY_AND_ASSIGN(NewReport);
}; };
//! \brief A crash report that is in the process of being uploaded.
//!
//! An instance of this class should be created via GetReportForUploading().
class UploadReport : public Report {
public:
UploadReport();
virtual ~UploadReport();
// An open FileReader with which to read the report.
FileReader* Reader() const { return reader_.get(); }
private:
friend class CrashReportDatabase;
friend class CrashReportDatabaseMac;
friend class CrashReportDatabaseWin;
bool Initialize(const base::FilePath path, CrashReportDatabase* database);
std::unique_ptr<FileReader> reader_;
CrashReportDatabase* database_;
DISALLOW_COPY_AND_ASSIGN(UploadReport);
};
//! \brief The result code for operations performed on a database. //! \brief The result code for operations performed on a database.
enum OperationStatus { enum OperationStatus {
//! \brief No error occurred. //! \brief No error occurred.
@ -260,42 +285,38 @@ class CrashReportDatabase {
//! \return The operation status code. //! \return The operation status code.
virtual OperationStatus GetCompletedReports(std::vector<Report>* reports) = 0; virtual OperationStatus GetCompletedReports(std::vector<Report>* reports) = 0;
//! \brief Obtains a report object for uploading to a collection server. //! \brief Obtains and locks a report object for uploading to a collection
//! server.
//! //!
//! The file at Report::file_path should be uploaded by the caller, and then //! Callers should upload the crash report using the FileReader provided.
//! the returned Report object must be disposed of via a call to //! Callers should then call RecordUploadComplete() to record a successful
//! RecordUploadAttempt(). //! upload. If RecordUploadComplete() is not called, the upload attempt will
//! //! be recorded as unsuccessful and the report lock released when \a report is
//! A subsequent call to this method with the same \a uuid is illegal until //! destroyed.
//! RecordUploadAttempt() has been called.
//! //!
//! \param[in] uuid The unique identifier for the crash report record. //! \param[in] uuid The unique identifier for the crash report record.
//! \param[out] report A crash report record for the report to be uploaded. //! \param[out] report A crash report record for the report to be uploaded.
//! The caller does not own this object. Only valid if this returns //! Only valid if this returns #kNoError.
//! #kNoError.
//! //!
//! \return The operation status code. //! \return The operation status code.
virtual OperationStatus GetReportForUploading(const UUID& uuid, virtual OperationStatus GetReportForUploading(
const Report** report) = 0; const UUID& uuid,
std::unique_ptr<const UploadReport>* report) = 0;
//! \brief Adjusts a crash report records metadata to account for an upload //! \brief Records a successful upload for a report and updates the last
//! attempt, and updates the last upload attempt time as returned by //! upload attempt time as returned by
//! Settings::GetLastUploadAttemptTime(). //! Settings::GetLastUploadAttemptTime().
//! //!
//! After calling this method, the database is permitted to move and rename //! \param[in] report A UploadReport object obtained from
//! the file at Report::file_path. //! GetReportForUploading(). The UploadReport object will be invalidated
//! //! and the report unlocked as part of this call.
//! \param[in] report The report object obtained from //! \param[in] id The possibly empty identifier assigned to this crash report
//! GetReportForUploading(). This object is invalidated after this call. //! by the collection server.
//! \param[in] successful Whether the upload attempt was successful.
//! \param[in] id The identifier assigned to this crash report by the
//! collection server. Must be empty if \a successful is `false`; may be
//! empty if it is `true`.
//! //!
//! \return The operation status code. //! \return The operation status code.
virtual OperationStatus RecordUploadAttempt(const Report* report, OperationStatus RecordUploadComplete(
bool successful, std::unique_ptr<const UploadReport> report,
const std::string& id) = 0; const std::string& id);
//! \brief Moves a report from the pending state to the completed state, but //! \brief Moves a report from the pending state to the completed state, but
//! without the report being uploaded. //! without the report being uploaded.
@ -331,6 +352,22 @@ class CrashReportDatabase {
CrashReportDatabase() {} CrashReportDatabase() {}
private: private:
//! \brief Adjusts a crash report records metadata to account for an upload
//! attempt, and updates the last upload attempt time as returned by
//! Settings::GetLastUploadAttemptTime().
//!
//! \param[in] report The report object obtained from
//! GetReportForUploading().
//! \param[in] successful Whether the upload attempt was successful.
//! \param[in] id The identifier assigned to this crash report by the
//! collection server. Must be empty if \a successful is `false`; may be
//! empty if it is `true`.
//!
//! \return The operation status code.
virtual OperationStatus RecordUploadAttempt(UploadReport* report,
bool successful,
const std::string& id) = 0;
DISALLOW_COPY_AND_ASSIGN(CrashReportDatabase); DISALLOW_COPY_AND_ASSIGN(CrashReportDatabase);
}; };

View File

@ -139,17 +139,20 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override; OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override;
OperationStatus GetPendingReports(std::vector<Report>* reports) override; OperationStatus GetPendingReports(std::vector<Report>* reports) override;
OperationStatus GetCompletedReports(std::vector<Report>* reports) override; OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
OperationStatus GetReportForUploading(const UUID& uuid, OperationStatus GetReportForUploading(
const Report** report) override; const UUID& uuid,
OperationStatus RecordUploadAttempt(const Report* report, std::unique_ptr<const UploadReport>* report) override;
bool successful,
const std::string& id) override;
OperationStatus SkipReportUpload(const UUID& uuid, OperationStatus SkipReportUpload(const UUID& uuid,
Metrics::CrashSkippedReason reason) override; Metrics::CrashSkippedReason reason) override;
OperationStatus DeleteReport(const UUID& uuid) override; OperationStatus DeleteReport(const UUID& uuid) override;
OperationStatus RequestUpload(const UUID& uuid) override; OperationStatus RequestUpload(const UUID& uuid) override;
private: private:
// CrashReportDatabase:
OperationStatus RecordUploadAttempt(UploadReport* report,
bool successful,
const std::string& id) override;
//! \brief Report states for use with LocateCrashReport(). //! \brief Report states for use with LocateCrashReport().
//! //!
//! ReportState may be considered to be a bitfield. //! ReportState may be considered to be a bitfield.
@ -163,10 +166,10 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
//! \brief A private extension of the Report class that maintains bookkeeping //! \brief A private extension of the Report class that maintains bookkeeping
//! information of the database. //! information of the database.
struct UploadReport : public Report { struct UploadReportMac : public UploadReport {
//! \brief Stores the flock of the file for the duration of //! \brief Stores the flock of the file for the duration of
//! GetReportForUploading() and RecordUploadAttempt(). //! GetReportForUploading() and RecordUploadAttempt().
int lock_fd; base::ScopedFD lock_fd;
}; };
//! \brief Locates a crash report in the database by UUID. //! \brief Locates a crash report in the database by UUID.
@ -406,31 +409,36 @@ CrashReportDatabaseMac::GetCompletedReports(
} }
CrashReportDatabase::OperationStatus CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::GetReportForUploading(const UUID& uuid, CrashReportDatabaseMac::GetReportForUploading(
const Report** report) { const UUID& uuid,
std::unique_ptr<const UploadReport>* report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_); INITIALIZATION_STATE_DCHECK_VALID(initialized_);
base::FilePath report_path = LocateCrashReport(uuid, kReportStatePending); auto upload_report = std::make_unique<UploadReportMac>();
if (report_path.empty())
upload_report->file_path = LocateCrashReport(uuid, kReportStatePending);
if (upload_report->file_path.empty())
return kReportNotFound; return kReportNotFound;
std::unique_ptr<UploadReport> upload_report(new UploadReport()); base::ScopedFD lock(ObtainReportLock(upload_report->file_path));
upload_report->file_path = report_path;
base::ScopedFD lock(ObtainReportLock(report_path));
if (!lock.is_valid()) if (!lock.is_valid())
return kBusyError; return kBusyError;
if (!ReadReportMetadataLocked(report_path, upload_report.get())) if (!ReadReportMetadataLocked(upload_report->file_path, upload_report.get()))
return kDatabaseError; return kDatabaseError;
upload_report->lock_fd = lock.release(); if (!upload_report->reader_->Open(upload_report->file_path)) {
*report = upload_report.release(); return kFileSystemError;
}
upload_report->database_ = this;
upload_report->lock_fd.reset(lock.release());
report->reset(upload_report.release());
return kNoError; return kNoError;
} }
CrashReportDatabase::OperationStatus CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::RecordUploadAttempt(const Report* report, CrashReportDatabaseMac::RecordUploadAttempt(UploadReport* report,
bool successful, bool successful,
const std::string& id) { const std::string& id) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_); INITIALIZATION_STATE_DCHECK_VALID(initialized_);
@ -445,13 +453,6 @@ CrashReportDatabaseMac::RecordUploadAttempt(const Report* report,
if (report_path.empty()) if (report_path.empty())
return kReportNotFound; return kReportNotFound;
std::unique_ptr<const UploadReport> upload_report(
static_cast<const UploadReport*>(report));
base::ScopedFD lock(upload_report->lock_fd);
if (!lock.is_valid())
return kBusyError;
if (successful) { if (successful) {
CrashReportDatabase::OperationStatus os = CrashReportDatabase::OperationStatus os =
MarkReportCompletedLocked(report_path, &report_path); MarkReportCompletedLocked(report_path, &report_path);

View File

@ -69,15 +69,19 @@ class CrashReportDatabaseTest : public testing::Test {
time_t times[2]; time_t times[2];
ASSERT_TRUE(settings->GetLastUploadAttemptTime(&times[0])); ASSERT_TRUE(settings->GetLastUploadAttemptTime(&times[0]));
const CrashReportDatabase::Report* report = nullptr; std::unique_ptr<const CrashReportDatabase::UploadReport> report;
ASSERT_EQ(db_->GetReportForUploading(uuid, &report), ASSERT_EQ(db_->GetReportForUploading(uuid, &report),
CrashReportDatabase::kNoError); CrashReportDatabase::kNoError);
EXPECT_NE(report->uuid, UUID()); EXPECT_NE(report->uuid, UUID());
EXPECT_FALSE(report->file_path.empty()); EXPECT_FALSE(report->file_path.empty());
EXPECT_TRUE(FileExists(report->file_path)) << report->file_path.value(); EXPECT_TRUE(FileExists(report->file_path)) << report->file_path.value();
EXPECT_GT(report->creation_time, 0); EXPECT_GT(report->creation_time, 0);
EXPECT_EQ(db_->RecordUploadAttempt(report, successful, id), if (successful) {
CrashReportDatabase::kNoError); EXPECT_EQ(db_->RecordUploadComplete(std::move(report), id),
CrashReportDatabase::kNoError);
} else {
report.reset();
}
ASSERT_TRUE(settings->GetLastUploadAttemptTime(&times[1])); ASSERT_TRUE(settings->GetLastUploadAttemptTime(&times[1]));
EXPECT_NE(times[1], 0); EXPECT_NE(times[1], 0);
@ -452,16 +456,16 @@ TEST_F(CrashReportDatabaseTest, DuelingUploads) {
CrashReportDatabase::Report report; CrashReportDatabase::Report report;
CreateCrashReport(&report); CreateCrashReport(&report);
const CrashReportDatabase::Report* upload_report; std::unique_ptr<const CrashReportDatabase::UploadReport> upload_report;
EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report), EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report),
CrashReportDatabase::kNoError); CrashReportDatabase::kNoError);
const CrashReportDatabase::Report* upload_report_2 = nullptr; std::unique_ptr<const CrashReportDatabase::UploadReport> upload_report_2;
EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report_2), EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report_2),
CrashReportDatabase::kBusyError); CrashReportDatabase::kBusyError);
EXPECT_FALSE(upload_report_2); EXPECT_FALSE(upload_report_2);
EXPECT_EQ(db()->RecordUploadAttempt(upload_report, true, std::string()), EXPECT_EQ(db()->RecordUploadComplete(std::move(upload_report), std::string()),
CrashReportDatabase::kNoError); CrashReportDatabase::kNoError);
} }
@ -469,16 +473,16 @@ TEST_F(CrashReportDatabaseTest, UploadAlreadyUploaded) {
CrashReportDatabase::Report report; CrashReportDatabase::Report report;
CreateCrashReport(&report); CreateCrashReport(&report);
const CrashReportDatabase::Report* upload_report; std::unique_ptr<const CrashReportDatabase::UploadReport> upload_report;
EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report), EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report),
CrashReportDatabase::kNoError); CrashReportDatabase::kNoError);
EXPECT_EQ(db()->RecordUploadAttempt(upload_report, true, std::string()), EXPECT_EQ(db()->RecordUploadComplete(std::move(upload_report), std::string()),
CrashReportDatabase::kNoError); CrashReportDatabase::kNoError);
const CrashReportDatabase::Report* upload_report_2 = nullptr; std::unique_ptr<const CrashReportDatabase::UploadReport> upload_report_2;
EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report_2), EXPECT_EQ(db()->GetReportForUploading(report.uuid, &upload_report_2),
CrashReportDatabase::kReportNotFound); CrashReportDatabase::kReportNotFound);
EXPECT_FALSE(upload_report_2); EXPECT_FALSE(upload_report_2.get());
} }
TEST_F(CrashReportDatabaseTest, MoveDatabase) { TEST_F(CrashReportDatabaseTest, MoveDatabase) {

View File

@ -29,6 +29,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "client/settings.h" #include "client/settings.h"
#include "util/misc/implicit_cast.h"
#include "util/misc/initialization_state_dcheck.h" #include "util/misc/initialization_state_dcheck.h"
#include "util/misc/metrics.h" #include "util/misc/metrics.h"
@ -591,17 +592,20 @@ class CrashReportDatabaseWin : public CrashReportDatabase {
OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override; OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override;
OperationStatus GetPendingReports(std::vector<Report>* reports) override; OperationStatus GetPendingReports(std::vector<Report>* reports) override;
OperationStatus GetCompletedReports(std::vector<Report>* reports) override; OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
OperationStatus GetReportForUploading(const UUID& uuid, OperationStatus GetReportForUploading(
const Report** report) override; const UUID& uuid,
OperationStatus RecordUploadAttempt(const Report* report, std::unique_ptr<const UploadReport>* report) override;
bool successful,
const std::string& id) override;
OperationStatus SkipReportUpload(const UUID& uuid, OperationStatus SkipReportUpload(const UUID& uuid,
Metrics::CrashSkippedReason reason) override; Metrics::CrashSkippedReason reason) override;
OperationStatus DeleteReport(const UUID& uuid) override; OperationStatus DeleteReport(const UUID& uuid) override;
OperationStatus RequestUpload(const UUID& uuid) override; OperationStatus RequestUpload(const UUID& uuid) override;
private: private:
// CrashReportDatabase:
OperationStatus RecordUploadAttempt(UploadReport* report,
bool successful,
const std::string& id) override;
std::unique_ptr<Metadata> AcquireMetadata(); std::unique_ptr<Metadata> AcquireMetadata();
base::FilePath base_dir_; base::FilePath base_dir_;
@ -716,44 +720,38 @@ OperationStatus CrashReportDatabaseWin::GetCompletedReports(
OperationStatus CrashReportDatabaseWin::GetReportForUploading( OperationStatus CrashReportDatabaseWin::GetReportForUploading(
const UUID& uuid, const UUID& uuid,
const Report** report) { std::unique_ptr<const UploadReport>* report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_); INITIALIZATION_STATE_DCHECK_VALID(initialized_);
std::unique_ptr<Metadata> metadata(AcquireMetadata()); std::unique_ptr<Metadata> metadata(AcquireMetadata());
if (!metadata) if (!metadata)
return kDatabaseError; return kDatabaseError;
// TODO(scottmg): After returning this report to the client, there is no way
// to reap this report if the uploader fails to call RecordUploadAttempt() or
// SkipReportUpload() (if it crashed or was otherwise buggy). To resolve this,
// one possibility would be to change the interface to be FileHandle based, so
// that instead of giving the file_path back to the client and changing state
// to kUploading, we return an exclusive access handle, and use that as the
// signal that the upload is pending, rather than an update to state in the
// metadata. Alternatively, there could be a "garbage collection" at startup
// where any reports that are orphaned in the kUploading state are either
// reset to kPending to retry, or discarded.
ReportDisk* report_disk; ReportDisk* report_disk;
OperationStatus os = metadata->FindSingleReportAndMarkDirty( OperationStatus os = metadata->FindSingleReportAndMarkDirty(
uuid, ReportState::kPending, &report_disk); uuid, ReportState::kPending, &report_disk);
if (os == kNoError) { if (os == kNoError) {
report_disk->state = ReportState::kUploading; report_disk->state = ReportState::kUploading;
// Create a copy for passing back to client. This will be freed in auto upload_report = std::make_unique<UploadReport>();
// RecordUploadAttempt. *implicit_cast<Report*>(upload_report.get()) = *report_disk;
*report = new Report(*report_disk);
if (!upload_report->Initialize(upload_report->file_path, this)) {
return kFileSystemError;
}
report->reset(upload_report.release());
} }
return os; return os;
} }
OperationStatus CrashReportDatabaseWin::RecordUploadAttempt( OperationStatus CrashReportDatabaseWin::RecordUploadAttempt(
const Report* report, UploadReport* report,
bool successful, bool successful,
const std::string& id) { const std::string& id) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_); INITIALIZATION_STATE_DCHECK_VALID(initialized_);
Metrics::CrashUploadAttempted(successful); Metrics::CrashUploadAttempted(successful);
// Take ownership, allocated in GetReportForUploading.
std::unique_ptr<const Report> upload_report(report);
std::unique_ptr<Metadata> metadata(AcquireMetadata()); std::unique_ptr<Metadata> metadata(AcquireMetadata());
if (!metadata) if (!metadata)
return kDatabaseError; return kDatabaseError;

View File

@ -42,9 +42,10 @@ class MockDatabase : public CrashReportDatabase {
MOCK_METHOD1(GetPendingReports, OperationStatus(std::vector<Report>*)); MOCK_METHOD1(GetPendingReports, OperationStatus(std::vector<Report>*));
MOCK_METHOD1(GetCompletedReports, OperationStatus(std::vector<Report>*)); MOCK_METHOD1(GetCompletedReports, OperationStatus(std::vector<Report>*));
MOCK_METHOD2(GetReportForUploading, MOCK_METHOD2(GetReportForUploading,
OperationStatus(const UUID&, const Report**)); OperationStatus(const UUID&,
std::unique_ptr<const UploadReport>*));
MOCK_METHOD3(RecordUploadAttempt, MOCK_METHOD3(RecordUploadAttempt,
OperationStatus(const Report*, bool, const std::string&)); OperationStatus(UploadReport*, bool, const std::string&));
MOCK_METHOD2(SkipReportUpload, MOCK_METHOD2(SkipReportUpload,
OperationStatus(const UUID&, Metrics::CrashSkippedReason)); OperationStatus(const UUID&, Metrics::CrashSkippedReason));
MOCK_METHOD1(DeleteReport, OperationStatus(const UUID&)); MOCK_METHOD1(DeleteReport, OperationStatus(const UUID&));

View File

@ -45,44 +45,6 @@
namespace crashpad { namespace crashpad {
namespace {
// Calls CrashReportDatabase::RecordUploadAttempt() with |successful| set to
// false upon destruction unless disarmed by calling Fire() or Disarm(). Fire()
// triggers an immediate call. Armed upon construction.
class CallRecordUploadAttempt {
public:
CallRecordUploadAttempt(CrashReportDatabase* database,
const CrashReportDatabase::Report* report)
: database_(database),
report_(report) {
}
~CallRecordUploadAttempt() {
Fire();
}
void Fire() {
if (report_) {
database_->RecordUploadAttempt(report_, false, std::string());
}
Disarm();
}
void Disarm() {
report_ = nullptr;
}
private:
CrashReportDatabase* database_; // weak
const CrashReportDatabase::Report* report_; // weak
DISALLOW_COPY_AND_ASSIGN(CallRecordUploadAttempt);
};
} // namespace
CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database, CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database,
const std::string& url, const std::string& url,
const Options& options) const Options& options)
@ -229,7 +191,7 @@ void CrashReportUploadThread::ProcessPendingReport(
} }
} }
const CrashReportDatabase::Report* upload_report; std::unique_ptr<const CrashReportDatabase::UploadReport> upload_report;
CrashReportDatabase::OperationStatus status = CrashReportDatabase::OperationStatus status =
database_->GetReportForUploading(report.uuid, &upload_report); database_->GetReportForUploading(report.uuid, &upload_report);
switch (status) { switch (status) {
@ -256,18 +218,16 @@ void CrashReportUploadThread::ProcessPendingReport(
return; return;
} }
CallRecordUploadAttempt call_record_upload_attempt(database_, upload_report);
std::string response_body; std::string response_body;
UploadResult upload_result = UploadReport(upload_report, &response_body); UploadResult upload_result =
UploadReport(upload_report.get(), &response_body);
switch (upload_result) { switch (upload_result) {
case UploadResult::kSuccess: case UploadResult::kSuccess:
call_record_upload_attempt.Disarm(); database_->RecordUploadComplete(std::move(upload_report), response_body);
database_->RecordUploadAttempt(upload_report, true, response_body);
break; break;
case UploadResult::kPermanentFailure: case UploadResult::kPermanentFailure:
case UploadResult::kRetry: case UploadResult::kRetry:
call_record_upload_attempt.Fire(); upload_report.reset();
// TODO(mark): Deal with retries properly: dont call SkipReportUplaod() // TODO(mark): Deal with retries properly: dont call SkipReportUplaod()
// if the result was kRetry and the report hasnt already been retried // if the result was kRetry and the report hasnt already been retried
@ -279,17 +239,12 @@ void CrashReportUploadThread::ProcessPendingReport(
} }
CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport( CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport(
const CrashReportDatabase::Report* report, const CrashReportDatabase::UploadReport* report,
std::string* response_body) { std::string* response_body) {
std::map<std::string, std::string> parameters; std::map<std::string, std::string> parameters;
FileReader minidump_file_reader; FileReader* reader = report->Reader();
if (!minidump_file_reader.Open(report->file_path)) { FileOffset start_offset = reader->SeekGet();
// If the minidump file cant be opened, all hope is lost.
return UploadResult::kPermanentFailure;
}
FileOffset start_offset = minidump_file_reader.SeekGet();
if (start_offset < 0) { if (start_offset < 0) {
return UploadResult::kPermanentFailure; return UploadResult::kPermanentFailure;
} }
@ -299,12 +254,12 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport(
// parameters, but as long as theres a dump file, the server can decide what // parameters, but as long as theres a dump file, the server can decide what
// to do with it. // to do with it.
ProcessSnapshotMinidump minidump_process_snapshot; ProcessSnapshotMinidump minidump_process_snapshot;
if (minidump_process_snapshot.Initialize(&minidump_file_reader)) { if (minidump_process_snapshot.Initialize(reader)) {
parameters = parameters =
BreakpadHTTPFormParametersFromMinidump(&minidump_process_snapshot); BreakpadHTTPFormParametersFromMinidump(&minidump_process_snapshot);
} }
if (!minidump_file_reader.SeekSet(start_offset)) { if (!reader->SeekSet(start_offset)) {
return UploadResult::kPermanentFailure; return UploadResult::kPermanentFailure;
} }
@ -322,15 +277,10 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport(
} }
} }
http_multipart_builder.SetFileAttachment( http_multipart_builder.SetFileAttachment(kMinidumpKey,
kMinidumpKey, report->uuid.ToString() + ".dmp",
#if defined(OS_WIN) reader,
base::UTF16ToUTF8(report->file_path.BaseName().value()), "application/octet-stream");
#else
report->file_path.BaseName().value(),
#endif
&minidump_file_reader,
"application/octet-stream");
std::unique_ptr<HTTPTransport> http_transport(HTTPTransport::Create()); std::unique_ptr<HTTPTransport> http_transport(HTTPTransport::Create());
HTTPHeaders content_headers; HTTPHeaders content_headers;

View File

@ -148,14 +148,14 @@ class CrashReportUploadThread : public WorkerThread::Delegate {
//! \param[in] report The report to upload. The caller is responsible for //! \param[in] report The report to upload. The caller is responsible for
//! calling CrashReportDatabase::GetReportForUploading() before calling //! calling CrashReportDatabase::GetReportForUploading() before calling
//! this method, and for calling //! this method, and for calling
//! CrashReportDatabase::RecordUploadAttempt() after calling this method. //! CrashReportDatabase::RecordUploadComplete() after calling this method.
//! \param[out] response_body If the upload attempt is successful, this will //! \param[out] response_body If the upload attempt is successful, this will
//! be set to the response body sent by the server. Breakpad-type servers //! be set to the response body sent by the server. Breakpad-type servers
//! provide the crash ID assigned by the server in the response body. //! provide the crash ID assigned by the server in the response body.
//! //!
//! \return A member of UploadResult indicating the result of the upload //! \return A member of UploadResult indicating the result of the upload
//! attempt. //! attempt.
UploadResult UploadReport(const CrashReportDatabase::Report* report, UploadResult UploadReport(const CrashReportDatabase::UploadReport* report,
std::string* response_body); std::string* response_body);
// WorkerThread::Delegate: // WorkerThread::Delegate: