Skip to content

Commit

Permalink
[Extensions] Create new on-install incremented histograms that check
Browse files Browse the repository at this point in the history
that the profile can use non-component extensions before emitting.

This change is limited to installed_loader.cc (RecordExtensionsMetrics()
and LoadAllExtensions()), but there are other stragglers in other files
that will be addressed in a follow-up CL.

This is a mix of histograms in various states, below is the state and
the intended outcome:

* >1 month expired histograms: delete the previous histogram (because
  it is no longer being recorded) and replace with the incremented
  version.
* <1 month expired histograms: extend the previous histograms
  `expires_after` date (so we can compare data) and create the
  incremented version.
* >1 month valid histograms: expire the previous histograms in a couple
  months (so they will be cleaned up automatically) and create the
  incremented version.

The unittest tests a subset of the incremented histograms, as the
effort/complexity in generating a test extension that exercises all the
code lines might not have value and would make the test brittle.

Bug: 1383740,1284659
Change-Id: I205f0e5a21fbe0b71abcd049a59c67b8ca776158
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4122980
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100691}
  • Loading branch information
Justin Lulejian authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 9b50879 commit bfdacc6
Show file tree
Hide file tree
Showing 7 changed files with 1,448 additions and 137 deletions.
19 changes: 17 additions & 2 deletions chrome/browser/extensions/extension_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,8 @@ void ExtensionService::GrantPermissions(const Extension* extension) {
// static
void ExtensionService::RecordPermissionMessagesHistogram(
const Extension* extension,
const char* histogram_basename) {
const char* histogram_basename,
bool log_user_profile_histograms) {
PermissionIDSet permissions =
PermissionMessageProvider::Get()->GetAllPermissionIDs(
extension->permissions_data()->active_permissions(),
Expand All @@ -1090,6 +1091,19 @@ void ExtensionService::RecordPermissionMessagesHistogram(
base::StringPrintf("Extensions.Permissions_%s3", histogram_basename);
for (const PermissionID& id : permissions)
base::UmaHistogramEnumeration(permissions_histogram_name, id.id());

if (log_user_profile_histograms) {
base::UmaHistogramBoolean(
base::StringPrintf("Extensions.HasPermissions_%s4", histogram_basename),
!permissions.empty());

std::string permissions_histogram_name_incremented =
base::StringPrintf("Extensions.Permissions_%s4", histogram_basename);
for (const PermissionID& id : permissions) {
base::UmaHistogramEnumeration(permissions_histogram_name_incremented,
id.id());
}
}
}

// TODO(michaelpg): Group with other ExtensionRegistrar::Delegate overrides
Expand Down Expand Up @@ -1687,7 +1701,8 @@ void ExtensionService::OnExtensionInstalled(
100);
UMA_HISTOGRAM_ENUMERATION("Extensions.InstallSource",
extension->location());
RecordPermissionMessagesHistogram(extension, "Install");
// TODO(crbug.com/1383740): Address Install metrics below in a follow-up CL.
RecordPermissionMessagesHistogram(extension, "Install", false);
}

const Extension::State initial_state =
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/extensions/extension_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,10 @@ class ExtensionService : public ExtensionServiceInterface,
// permission in |e|.
// NOTE: If this is ever called with high frequency, the implementation may
// need to be made more efficient.
static void RecordPermissionMessagesHistogram(const Extension* extension,
const char* histogram);
static void RecordPermissionMessagesHistogram(
const Extension* extension,
const char* histogram,
bool log_user_profile_histograms);

// Unloads the given extension and marks the extension as terminated. This
// doesn't notify the user that the extension was terminated, if such a
Expand Down

0 comments on commit bfdacc6

Please sign in to comment.