3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-01-14 09:17:57 +08:00
Code Issues Projects Releases Wiki Activity

Lazy load settings in CrashReportDatabase.

Browse Source 
Before this patch, settings.dat is read from and written to during
database initialization. This happens within Crashpad for iOS, and
within Chrome during startup here:
https://source.chromium.org/chromium/chromium/src/+/main:components/crash/core/app/crashpad.cc;l=209
These are blocking calls on the main thread.

CrashReportDatabaseMac::Initialize will still fail if the various
directory create/ensure calls fail.

Change-Id: Ic665884d1f41caa853aba9b29b6fb2c14b2cda15
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3674639
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This commit is contained in:
Justin Cohen 2022-06-08 19:45:54 -04:00 committed by Crashpad LUCI CQ
parent 339b125241
commit 816c5572b8
4 changed files with 72 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
client/crash_report_database_generic.cc
View File

@ -257,8 +257,16 @@ class CrashReportDatabaseGeneric : public CrashReportDatabase {
// Writes the metadata for report to the filesystem at path.
static bool WriteMetadata(const base::FilePath& path, const Report& report);
Settings& SettingsInternal() {
if (!settings_init_)
settings_.Initialize(base_dir_.Append(kSettings));
settings_init_ = true;
return settings_;
}
base::FilePath base_dir_;
Settings settings_;
bool settings_init_ = false;
InitializationStateDcheck initialized_;
};
@ -289,10 +297,6 @@ bool CrashReportDatabaseGeneric::Initialize(const base::FilePath& path,
return false;
}
if (!settings_.Initialize(base_dir_.Append(kSettings))) {
return false;
}
INITIALIZATION_STATE_SET_VALID(initialized_);
return true;
}
@ -317,7 +321,7 @@ base::FilePath CrashReportDatabaseGeneric::DatabasePath() {
Settings* CrashReportDatabaseGeneric::GetSettings() {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
return &settings_;
return &SettingsInternal();
}
OperationStatus CrashReportDatabaseGeneric::PrepareNewCrashReport(
@ -588,7 +592,7 @@ OperationStatus CrashReportDatabaseGeneric::RecordUploadAttempt(
return kDatabaseError;
}
if (!settings_.SetLastUploadAttemptTime(now)) {
if (!SettingsInternal().SetLastUploadAttemptTime(now)) {
return kDatabaseError;
}

20
client/crash_report_database_mac.mm
View File

@ -262,20 +262,27 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
// Cleans any attachments that have no associated report in any state.
void CleanOrphanedAttachments();
Settings& SettingsInternal() {
if (!settings_init_)
settings_.Initialize(base_dir_.Append(kSettings));
settings_init_ = true;
return settings_;
}
base::FilePath base_dir_;
Settings settings_;
bool settings_init_;
bool xattr_new_names_;
InitializationStateDcheck initialized_;
};
CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path)
: CrashReportDatabase(),
base_dir_(path),
settings_(),
settings_init_(false),
xattr_new_names_(false),
initialized_() {
}
initialized_() {}
CrashReportDatabaseMac::~CrashReportDatabaseMac() {}
@ -301,9 +308,6 @@ bool CrashReportDatabaseMac::Initialize(bool may_create) {
return false;
}
if (!settings_.Initialize(base_dir_.Append(kSettings)))
return false;
// Do an xattr operation as the last step, to ensure the filesystem has
// support for them. This xattr also serves as a marker for whether the
// database uses old or new xattr names.
@ -334,7 +338,7 @@ base::FilePath CrashReportDatabaseMac::DatabasePath() {
Settings* CrashReportDatabaseMac::GetSettings() {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
return &settings_;
return &SettingsInternal();
}
CrashReportDatabase::OperationStatus
@ -527,7 +531,7 @@ CrashReportDatabaseMac::RecordUploadAttempt(UploadReport* report,
return kDatabaseError;
}
if (!settings_.SetLastUploadAttemptTime(now)) {
if (!SettingsInternal().SetLastUploadAttemptTime(now)) {
return kDatabaseError;
}

35
client/crash_report_database_test.cc
View File

@ -185,6 +185,41 @@ TEST_F(CrashReportDatabaseTest, Initialize) {
EXPECT_FALSE(db);
}
TEST_F(CrashReportDatabaseTest, Settings) {
// Initialize three databases and ensure settings.dat isn't created yet.
ASSERT_TRUE(db());
base::FilePath settings_path =
path().Append(FILE_PATH_LITERAL("settings.dat"));
EXPECT_FALSE(FileExists(settings_path));
std::unique_ptr<CrashReportDatabase> db2 =
CrashReportDatabase::Initialize(path());
ASSERT_TRUE(db2);
EXPECT_FALSE(FileExists(settings_path));
std::unique_ptr<CrashReportDatabase> db3 =
CrashReportDatabase::Initialize(path());
ASSERT_TRUE(db3);
EXPECT_FALSE(FileExists(settings_path));
// Ensure settings.dat exists after getter.
Settings* settings = db3->GetSettings();
ASSERT_TRUE(settings);
EXPECT_TRUE(FileExists(settings_path));
time_t last_upload_attempt_time = 42;
ASSERT_TRUE(settings->SetLastUploadAttemptTime(last_upload_attempt_time));
// Ensure the first two databases read the same value.
ASSERT_TRUE(
db2->GetSettings()->GetLastUploadAttemptTime(&last_upload_attempt_time));
EXPECT_EQ(last_upload_attempt_time, 42);
ASSERT_TRUE(
db()->GetSettings()->GetLastUploadAttemptTime(&last_upload_attempt_time));
EXPECT_EQ(last_upload_attempt_time, 42);
}
TEST_F(CrashReportDatabaseTest, NewCrashReport) {
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
EXPECT_EQ(db()->PrepareNewCrashReport(&new_report),

21
client/crash_report_database_win.cc
View File

@ -662,13 +662,25 @@ class CrashReportDatabaseWin : public CrashReportDatabase {
std::unique_ptr<Metadata> AcquireMetadata();
Settings& SettingsInternal() {
if (!settings_init_)
settings_.Initialize(base_dir_.Append(kSettings));
settings_init_ = true;
return settings_;
}
base::FilePath base_dir_;
Settings settings_;
bool settings_init_;
InitializationStateDcheck initialized_;
};
CrashReportDatabaseWin::CrashReportDatabaseWin(const base::FilePath& path)
: CrashReportDatabase(), base_dir_(path), settings_(), initialized_() {}
: CrashReportDatabase(),
base_dir_(path),
settings_(),
settings_init_(false),
initialized_() {}
CrashReportDatabaseWin::~CrashReportDatabaseWin() {
}
@ -691,9 +703,6 @@ bool CrashReportDatabaseWin::Initialize(bool may_create) {
if (!CreateDirectoryIfNecessary(AttachmentsRootPath()))
return false;
if (!settings_.Initialize(base_dir_.Append(kSettings)))
return false;
INITIALIZATION_STATE_SET_VALID(initialized_);
return true;
}
@ -704,7 +713,7 @@ base::FilePath CrashReportDatabaseWin::DatabasePath() {
Settings* CrashReportDatabaseWin::GetSettings() {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
return &settings_;
return &SettingsInternal();
}
OperationStatus CrashReportDatabaseWin::PrepareNewCrashReport(
@ -848,7 +857,7 @@ OperationStatus CrashReportDatabaseWin::RecordUploadAttempt(
report->upload_explicitly_requested;
}
if (!settings_.SetLastUploadAttemptTime(now))
if (!SettingsInternal().SetLastUploadAttemptTime(now))
return kDatabaseError;
return kNoError;