Tweaks on persistent histogram storage's implementation and usage
Bug: 734095
Change-Id: Ia832d7b3b176d90d4c5d618ac169ad0cc5fe8a72
Reviewed-on: https://chromium-review.googlesource.com/996455
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548861}diff --git a/base/metrics/persistent_histogram_storage.cc b/base/metrics/persistent_histogram_storage.cc
index 20922888..0676e47 100644
--- a/base/metrics/persistent_histogram_storage.cc
+++ b/base/metrics/persistent_histogram_storage.cc
@@ -24,8 +24,8 @@
PersistentHistogramStorage::PersistentHistogramStorage(
StringPiece allocator_name,
- StorageDirCreation storage_dir_create_action)
- : storage_dir_create_action_(storage_dir_create_action) {
+ StorageDirManagement storage_dir_management)
+ : storage_dir_management_(storage_dir_management) {
DCHECK(!allocator_name.empty());
DCHECK(IsStringASCII(allocator_name));
@@ -53,8 +53,8 @@
FilePath storage_dir = storage_base_dir_.AppendASCII(allocator->Name());
- switch (storage_dir_create_action_) {
- case StorageDirCreation::kEnable:
+ switch (storage_dir_management_) {
+ case StorageDirManagement::kCreate:
if (!CreateDirectory(storage_dir)) {
LOG(ERROR)
<< "Could not write \"" << allocator->Name()
@@ -63,11 +63,11 @@
return;
}
break;
- case StorageDirCreation::kDisable:
+ case StorageDirManagement::kUseExisting:
if (!DirectoryExists(storage_dir)) {
- // When the consumer of this class disables storage directory creation
- // by the class instance, it should ensure the directory's existence if
- // it's essential.
+ // When the consumer of this class decides to use an existing storage
+ // directory, it should ensure the directory's existence if it's
+ // essential.
LOG(ERROR)
<< "Could not write \"" << allocator->Name()
<< "\" persistent histograms to file as the storage directory "
diff --git a/base/metrics/persistent_histogram_storage.h b/base/metrics/persistent_histogram_storage.h
index e101a2605..397236d 100644
--- a/base/metrics/persistent_histogram_storage.h
+++ b/base/metrics/persistent_histogram_storage.h
@@ -22,7 +22,7 @@
// Persisted histograms will eventually be reported by Chrome.
class BASE_EXPORT PersistentHistogramStorage {
public:
- enum class StorageDirCreation { kEnable, kDisable };
+ enum class StorageDirManagement { kCreate, kUseExisting };
// Creates a process-wide storage location for histograms that will be written
// to a file within a directory provided by |set_storage_base_dir()| on
@@ -30,11 +30,10 @@
// The |allocator_name| is used both as an internal name for the allocator,
// well as the leaf directory name for the file to which the histograms are
// persisted. The string must be ASCII.
- // |storage_dir_create_action| determines that if this instance is
- // responsible for creating the storage directory. If kDisable is supplied,
- // it's the consumer's responsibility to create the storage directory.
+ // |storage_dir_management| specifies if this instance reuses an existing
+ // storage directory, or is responsible for creating one.
PersistentHistogramStorage(StringPiece allocator_name,
- StorageDirCreation storage_dir_create_action);
+ StorageDirManagement storage_dir_management);
~PersistentHistogramStorage();
@@ -53,9 +52,8 @@
// |storage_base_dir_|/|allocator_name| (see the ctor for allocator_name).
FilePath storage_base_dir_;
- // A flag indicating if the class instance is responsible for creating the
- // storage directory.
- const StorageDirCreation storage_dir_create_action_;
+ // The setting of the storage directory management.
+ const StorageDirManagement storage_dir_management_;
// A flag indicating if histogram storage is disabled. It starts with false,
// but can be set to true by the caller who decides to throw away its
diff --git a/base/metrics/persistent_histogram_storage_unittest.cc b/base/metrics/persistent_histogram_storage_unittest.cc
index 4a9b34b..adbcdfc 100644
--- a/base/metrics/persistent_histogram_storage_unittest.cc
+++ b/base/metrics/persistent_histogram_storage_unittest.cc
@@ -55,7 +55,7 @@
auto persistent_histogram_storage =
std::make_unique<PersistentHistogramStorage>(
kTestHistogramAllocatorName,
- PersistentHistogramStorage::StorageDirCreation::kEnable);
+ PersistentHistogramStorage::StorageDirManagement::kCreate);
persistent_histogram_storage->set_storage_base_dir(temp_dir_path());
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 91c93f7..2b355d9 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -1291,11 +1291,11 @@
return installer::CPU_NOT_SUPPORTED;
// Persist histograms so they can be uploaded later. The storage directory is
- // created during installation when the main WorkItemList is evaluated. So
+ // created during installation when the main WorkItemList is evaluated so
// disable storage directory creation in PersistentHistogramStorage.
base::PersistentHistogramStorage persistent_histogram_storage(
installer::kSetupHistogramAllocatorName,
- base::PersistentHistogramStorage::StorageDirCreation::kDisable);
+ base::PersistentHistogramStorage::StorageDirManagement::kUseExisting);
// The exit manager is in charge of calling the dtors of singletons.
base::AtExitManager exit_manager;
@@ -1306,6 +1306,9 @@
switches::kProcessType);
if (process_type == crash_reporter::switches::kCrashpadHandler) {
+ // Histogram storage is enabled at the very top of this wWinMain. Disable it
+ // when this process is decicated to crashpad as there is no directory in
+ // which to write them nor a browser to subsequently upload them.
persistent_histogram_storage.Disable();
return crash_reporter::RunAsCrashpadHandler(
*base::CommandLine::ForCurrentProcess(), base::FilePath(),
@@ -1351,8 +1354,9 @@
const bool is_uninstall = cmd_line.HasSwitch(installer::switches::kUninstall);
- // Disable histogram storage during uninstall since there's neither a
- // directory in which to write them nor a browser to subsequently upload them.
+ // Histogram storage is enabled at the very top of this wWinMain. Disable it
+ // during uninstall since there's neither a directory in which to write them
+ // nor a browser to subsequently upload them.
if (is_uninstall)
persistent_histogram_storage.Disable();
diff --git a/notification_helper/notification_helper.cc b/notification_helper/notification_helper.cc
index 313f00e2..00f2be4c 100644
--- a/notification_helper/notification_helper.cc
+++ b/notification_helper/notification_helper.cc
@@ -29,7 +29,7 @@
// chrome/browser/metrics/chrome_metrics_service_client.cc.
base::PersistentHistogramStorage persistent_histogram_storage(
"NotificationHelperMetrics",
- base::PersistentHistogramStorage::StorageDirCreation::kEnable);
+ base::PersistentHistogramStorage::StorageDirManagement::kCreate);
// Initialize the CommandLine singleton from the environment.
base::CommandLine::Init(0, nullptr);
@@ -39,6 +39,9 @@
// process should exit immediately.
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms683844.aspx
if (!base::CommandLine::ForCurrentProcess()->HasSwitch("embedding")) {
+ // Histogram storage is enabled at the very top of this wWinMain. Disable it
+ // in this case as there is no directory in which to write them nor a
+ // browser to subsequently upload them.
persistent_histogram_storage.Disable();
return 0;
}
