Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5565)

Unified Diff: chrome/installer/setup/persistent_histogram_storage.cc

Issue 2335313002: Always persist histograms from setup.exe. (Closed)
Patch Set: CR grt/bcwhite #4-5 Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/installer/setup/persistent_histogram_storage.h ('k') | chrome/installer/setup/setup_main.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/installer/setup/persistent_histogram_storage.cc
diff --git a/chrome/installer/setup/installer_metrics.cc b/chrome/installer/setup/persistent_histogram_storage.cc
similarity index 36%
rename from chrome/installer/setup/installer_metrics.cc
rename to chrome/installer/setup/persistent_histogram_storage.cc
index 7cc3108a4cc3627daa1f4433ea15f3818a1d5cf6..a02dff1b48996c7166c13bd61f03ca3d8514cadb 100644
--- a/chrome/installer/setup/installer_metrics.cc
+++ b/chrome/installer/setup/persistent_histogram_storage.cc
@@ -2,15 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/installer/setup/installer_metrics.h"
+#include "chrome/installer/setup/persistent_histogram_storage.h"
-#include <windows.h> // NOLINT
-#include <atlsecurity.h>
-
-#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/files/important_file_writer.h"
+#include "base/logging.h"
#include "base/metrics/histogram_base.h"
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/metrics/persistent_memory_allocator.h"
@@ -21,123 +18,41 @@
namespace installer {
-namespace {
-
-// This is duplicated from components/metrics/file_metrics_provider.h which is
-// not accessible from setup code.
-const base::FilePath::CharType MetricsFileExtension[] =
- FILE_PATH_LITERAL(".pma");
-
-// Add to the ACL of an object on disk. This follows the method from MSDN:
-// https://msdn.microsoft.com/en-us/library/windows/desktop/aa379283.aspx
-// This is done using explicit flags rather than the "security string" format
-// because strings do not necessarily read what is written which makes it
-// difficult to de-dup. Working with the binary format is always exact and
-// the system libraries will properly ignore duplicate ACL entries.
-bool AddAclToPath(const base::FilePath& path,
- const CSid& trustee,
- ACCESS_MASK access_mask,
- BYTE ace_flags) {
- DCHECK(!path.empty());
- DCHECK(trustee);
-
- // Get the existing DACL.
- ATL::CDacl dacl;
- if (!ATL::AtlGetDacl(path.value().c_str(), SE_FILE_OBJECT, &dacl)) {
- DPLOG(ERROR) << "Failed getting DACL for path \"" << path.value() << "\"";
- return false;
- }
-
- // Check if the requested access already exists and return if so.
- for (UINT i = 0; i < dacl.GetAceCount(); ++i) {
- ATL::CSid sid;
- ACCESS_MASK mask = 0;
- BYTE type = 0;
- BYTE flags = 0;
- dacl.GetAclEntry(i, &sid, &mask, &type, &flags);
- if (sid == trustee && type == ACCESS_ALLOWED_ACE_TYPE &&
- (flags & ace_flags) == ace_flags &&
- (mask & access_mask) == access_mask) {
- return true;
- }
- }
-
- // Add the new access to the DACL.
- if (!dacl.AddAllowedAce(trustee, access_mask, ace_flags)) {
- DPLOG(ERROR) << "Failed adding ACE to DACL";
- return false;
- }
-
- // Attach the updated ACL as the object's DACL.
- if (!ATL::AtlSetDacl(path.value().c_str(), SE_FILE_OBJECT, dacl)) {
- DPLOG(ERROR) << "Failed setting DACL for path \"" << path.value() << "\"";
- return false;
- }
-
- return true;
-}
-
-} // namespace
-
-base::FilePath GetPersistentHistogramStorageDir(
- const base::FilePath& target_path) {
- return target_path.AppendASCII(kSetupHistogramAllocatorName);
-}
-
-void BeginPersistentHistogramStorage() {
+PersistentHistogramStorage::PersistentHistogramStorage() {
base::GlobalHistogramAllocator::CreateWithLocalMemory(
1 << 20, // 1 MiB
0, // No identifier.
- installer::kSetupHistogramAllocatorName);
+ kSetupHistogramAllocatorName);
base::GlobalHistogramAllocator::Get()->CreateTrackingHistograms(
kSetupHistogramAllocatorName);
- // This can't be enabled until after the allocator is configured because
- // there is no other reporting out of setup other than persistent memory.
+ // This can't be enabled until after the allocator is configured because there
+ // is no other reporting out of setup other than persistent memory.
base::HistogramBase::EnableActivityReportHistogram("setup");
}
-void EndPersistentHistogramStorage(const base::FilePath& target_path,
- bool system_install) {
+PersistentHistogramStorage::~PersistentHistogramStorage() {
base::PersistentHistogramAllocator* allocator =
base::GlobalHistogramAllocator::Get();
allocator->UpdateTrackingHistograms();
- // Allocator dumps are saved to a directory that was created earlier. Stop
- // now if that directory has been removed because an uninstall has happened.
- base::FilePath dir_path = GetPersistentHistogramStorageDir(target_path);
- if (!base::DirectoryExists(dir_path))
+ // Stop if |storage_dir_| isn't set or does not exist. That can happen if the
+ // product hasn't been installed yet or if it has been uninstalled.
+ if (storage_dir_.empty() || !base::DirectoryExists(storage_dir_))
return;
- // Set permissions on the directory that allows other processes, including
- // non-privileged ones, to read and delete the files stored there. This
- // allows the browser process to remove the metrics files once it's done
- // reading them. This is only done for system-level installs; user-level
- // installs already provide delete-file access to the browser process.
- if (system_install) {
- if (!AddAclToPath(dir_path, ATL::Sids::AuthenticatedUser(),
- FILE_GENERIC_READ | FILE_DELETE_CHILD,
- CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE)) {
- PLOG(ERROR) << "Could not set \"delete\" permission for metrics directory"
- << " \"" << dir_path.value() << "\"";
- }
- }
-
- // Remove any existing metrics file at the old (non-subdir) pathname.
- base::DeleteFile(dir_path.AddExtension(MetricsFileExtension), false);
-
// Save data using the current time as the filename. The actual filename
// doesn't matter (so long as it ends with the correct extension) but this
// works as well as anything.
base::Time::Exploded exploded;
base::Time::Now().LocalExplode(&exploded);
- base::FilePath file_path =
- dir_path
+ const base::FilePath file_path =
+ storage_dir_
.AppendASCII(base::StringPrintf("%04d%02d%02d%02d%02d%02d",
exploded.year, exploded.month,
exploded.day_of_month, exploded.hour,
exploded.minute, exploded.second))
- .AddExtension(MetricsFileExtension);
+ .AddExtension(base::PersistentMemoryAllocator::kFileExtension);
base::StringPiece contents(static_cast<const char*>(allocator->data()),
allocator->used());
@@ -145,4 +60,10 @@ void EndPersistentHistogramStorage(const base::FilePath& target_path,
VLOG(1) << "Persistent histograms saved in file: " << file_path.value();
}
+// static
+base::FilePath PersistentHistogramStorage::GetReportedStorageDir(
+ const base::FilePath& target_path) {
+ return target_path.AppendASCII(kSetupHistogramAllocatorName);
+}
+
} // namespace installer
« no previous file with comments | « chrome/installer/setup/persistent_histogram_storage.h ('k') | chrome/installer/setup/setup_main.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698