diff --git a/client/crash_report_database_mac.mm b/client/crash_report_database_mac.mm index 7a9154b8..3bba2b63 100644 --- a/client/crash_report_database_mac.mm +++ b/client/crash_report_database_mac.mm @@ -243,7 +243,7 @@ class CrashReportDatabaseMac : public CrashReportDatabase { CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path) : CrashReportDatabase(), base_dir_(path), - settings_(base_dir_.Append(kSettings)), + settings_(), xattr_new_names_(false), initialized_() { } @@ -268,7 +268,7 @@ bool CrashReportDatabaseMac::Initialize(bool may_create) { return false; } - if (!settings_.Initialize()) + if (!settings_.Initialize(base_dir_.Append(kSettings))) return false; // Do an xattr operation as the last step, to ensure the filesystem has diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index 538eff5f..d6699e1b 100644 --- a/client/crash_report_database_win.cc +++ b/client/crash_report_database_win.cc @@ -610,11 +610,7 @@ class CrashReportDatabaseWin : public CrashReportDatabase { }; CrashReportDatabaseWin::CrashReportDatabaseWin(const base::FilePath& path) - : CrashReportDatabase(), - base_dir_(path), - settings_(base_dir_.Append(kSettings)), - initialized_() { -} + : CrashReportDatabase(), base_dir_(path), settings_(), initialized_() {} CrashReportDatabaseWin::~CrashReportDatabaseWin() { } @@ -634,7 +630,7 @@ bool CrashReportDatabaseWin::Initialize(bool may_create) { if (!CreateDirectoryIfNecessary(base_dir_.Append(kReportsDirectory))) return false; - if (!settings_.Initialize()) + if (!settings_.Initialize(base_dir_.Append(kSettings))) return false; INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/client/settings.cc b/client/settings.cc index 15d16f2e..8d4dbec8 100644 --- a/client/settings.cc +++ b/client/settings.cc @@ -59,16 +59,14 @@ struct Settings::Data { UUID client_id; }; -Settings::Settings(const base::FilePath& file_path) - : file_path_(file_path), - initialized_() { -} +Settings::Settings() = default; -Settings::~Settings() { -} +Settings::~Settings() = default; -bool Settings::Initialize() { +bool Settings::Initialize(const base::FilePath& file_path) { + DCHECK(initialized_.is_uninitialized()); initialized_.set_invalid(); + file_path_ = file_path; Data settings; if (!OpenForWritingAndReadSettings(&settings).is_valid()) diff --git a/client/settings.h b/client/settings.h index b64f74fb..488080c1 100644 --- a/client/settings.h +++ b/client/settings.h @@ -44,10 +44,18 @@ struct ScopedLockedFileHandleTraits { //! should be retrieved via CrashReportDatabase::GetSettings(). class Settings { public: - explicit Settings(const base::FilePath& file_path); + Settings(); ~Settings(); - bool Initialize(); + //! \brief Initializes the settings data store. + //! + //! This method must be called only once, and must be successfully called + //! before any other method in this class may be called. + //! + //! \param[in] path The location to store the settings data. + //! \return `true` if the data store was initialized successfully, otherwise + //! `false` with an error logged. + bool Initialize(const base::FilePath& path); //! \brief Retrieves the immutable identifier for this client, which is used //! on a server to locate all crash reports from a specific Crashpad diff --git a/client/settings_test.cc b/client/settings_test.cc index ca961a23..3a5730bd 100644 --- a/client/settings_test.cc +++ b/client/settings_test.cc @@ -26,7 +26,7 @@ namespace { class SettingsTest : public testing::Test { public: - SettingsTest() : settings_(settings_path()) {} + SettingsTest() = default; base::FilePath settings_path() { return temp_dir_.path().Append(FILE_PATH_LITERAL("settings")); @@ -49,7 +49,7 @@ class SettingsTest : public testing::Test { protected: // testing::Test: void SetUp() override { - ASSERT_TRUE(settings()->Initialize()); + ASSERT_TRUE(settings()->Initialize(settings_path())); } private: @@ -64,8 +64,8 @@ TEST_F(SettingsTest, ClientID) { EXPECT_TRUE(settings()->GetClientID(&client_id)); EXPECT_NE(client_id, UUID()); - Settings local_settings(settings_path()); - EXPECT_TRUE(local_settings.Initialize()); + Settings local_settings; + EXPECT_TRUE(local_settings.Initialize(settings_path())); UUID actual; EXPECT_TRUE(local_settings.GetClientID(&actual)); EXPECT_EQ(actual, client_id); @@ -81,8 +81,8 @@ TEST_F(SettingsTest, UploadsEnabled) { EXPECT_TRUE(settings()->GetUploadsEnabled(&enabled)); EXPECT_TRUE(enabled); - Settings local_settings(settings_path()); - EXPECT_TRUE(local_settings.Initialize()); + Settings local_settings; + EXPECT_TRUE(local_settings.Initialize(settings_path())); enabled = false; EXPECT_TRUE(local_settings.GetUploadsEnabled(&enabled)); EXPECT_TRUE(enabled); @@ -107,8 +107,8 @@ TEST_F(SettingsTest, LastUploadAttemptTime) { EXPECT_TRUE(settings()->GetLastUploadAttemptTime(&actual)); EXPECT_EQ(actual, expected); - Settings local_settings(settings_path()); - EXPECT_TRUE(local_settings.Initialize()); + Settings local_settings; + EXPECT_TRUE(local_settings.Initialize(settings_path())); actual = -1; EXPECT_TRUE(local_settings.GetLastUploadAttemptTime(&actual)); EXPECT_EQ(actual, expected); @@ -120,8 +120,8 @@ TEST_F(SettingsTest, LastUploadAttemptTime) { TEST_F(SettingsTest, BadFileOnInitialize) { InitializeBadFile(); - Settings settings(settings_path()); - EXPECT_TRUE(settings.Initialize()); + Settings settings; + EXPECT_TRUE(settings.Initialize(settings_path())); } TEST_F(SettingsTest, BadFileOnGet) { @@ -131,8 +131,8 @@ TEST_F(SettingsTest, BadFileOnGet) { EXPECT_TRUE(settings()->GetClientID(&client_id)); EXPECT_NE(client_id, UUID()); - Settings local_settings(settings_path()); - EXPECT_TRUE(local_settings.Initialize()); + Settings local_settings; + EXPECT_TRUE(local_settings.Initialize(settings_path())); UUID actual; EXPECT_TRUE(local_settings.GetClientID(&actual)); EXPECT_EQ(actual, client_id); @@ -161,8 +161,8 @@ TEST_F(SettingsTest, UnlinkFile) { << ErrnoMessage("unlink"); #endif - Settings local_settings(settings_path()); - EXPECT_TRUE(local_settings.Initialize()); + Settings local_settings; + EXPECT_TRUE(local_settings.Initialize(settings_path())); UUID new_client_id; EXPECT_TRUE(local_settings.GetClientID(&new_client_id)); EXPECT_NE(new_client_id, client_id);