Commit 80cae97e authored by Saurabh Nijhara's avatar Saurabh Nijhara Committed by Commit Bot

Report extension installation times using UMA statistics

We want to improve the installation time for force installed
extensions. For this, we need to first gather the statistics regarding
which installation stages are time consuming.

This CL adds the UMA statistics for the following installation stages:
1) Unpacking - Time taken to complete the unpacking of the extension
archive.
2) Checking - Time taken to perform the final checks to confirm that the
extension can be installed.
3) Finalizing - Time taken to install the extension.

Following histograms are added:
Extensions.ForceInstalledTime.UnpackingStartTo.CheckingStart
Extensions.ForceInstalledTime.CheckingStartTo.FinalizingStart
Extensions.ForceInstalledTime.FinalizingStartTo.CRXInstallComplete

Bug: 1102369
Change-Id: I46ed872a2954edba51ebdaeab8ab0ca9e1f4703b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351996Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarOleg Davydov <burunduk@chromium.org>
Reviewed-by: default avatarSwapnil Gupta <swapnilgupta@google.com>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Commit-Queue: Saurabh Nijhara <snijhara@google.com>
Cr-Commit-Position: refs/heads/master@{#799034}
parent b57201dd
...@@ -524,7 +524,7 @@ void CrxInstaller::OnUnpackSuccess( ...@@ -524,7 +524,7 @@ void CrxInstaller::OnUnpackSuccess(
extension_ = extension; extension_ = extension;
temp_dir_ = temp_dir; temp_dir_ = temp_dir;
ruleset_checksums_ = std::move(ruleset_checksums); ruleset_checksums_ = std::move(ruleset_checksums);
ReportInstallationStage(InstallationStage::kFinalizing); ReportInstallationStage(InstallationStage::kCheckingExpectations);
if (!install_icon.empty()) if (!install_icon.empty())
install_icon_ = std::make_unique<SkBitmap>(install_icon); install_icon_ = std::make_unique<SkBitmap>(install_icon);
...@@ -722,7 +722,7 @@ void CrxInstaller::OnInstallChecksComplete(const PreloadCheck::Errors& errors) { ...@@ -722,7 +722,7 @@ void CrxInstaller::OnInstallChecksComplete(const PreloadCheck::Errors& errors) {
void CrxInstaller::ConfirmInstall() { void CrxInstaller::ConfirmInstall() {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
ReportInstallationStage(InstallationStage::kComplete); ReportInstallationStage(InstallationStage::kFinalizing);
ExtensionService* service = service_weak_.get(); ExtensionService* service = service_weak_.get();
if (!service || service->browser_terminating()) if (!service || service->browser_terminating())
return; return;
...@@ -1047,6 +1047,7 @@ void CrxInstaller::NotifyCrxInstallBegin() { ...@@ -1047,6 +1047,7 @@ void CrxInstaller::NotifyCrxInstallBegin() {
void CrxInstaller::NotifyCrxInstallComplete( void CrxInstaller::NotifyCrxInstallComplete(
const base::Optional<CrxInstallError>& error) { const base::Optional<CrxInstallError>& error) {
ReportInstallationStage(InstallationStage::kComplete);
const std::string extension_id = const std::string extension_id =
expected_id_.empty() && extension() ? extension()->id() : expected_id_; expected_id_.empty() && extension() ? extension()->id() : expected_id_;
InstallStageTracker* install_stage_tracker = InstallStageTracker* install_stage_tracker =
......
...@@ -205,6 +205,30 @@ void ForceInstalledMetrics::ReportMetrics() { ...@@ -205,6 +205,30 @@ void ForceInstalledMetrics::ReportMetrics() {
installation.unpacking_started_time.value() - installation.unpacking_started_time.value() -
installation.copying_started_time.value()); installation.copying_started_time.value());
} }
if (installation.checking_expectations_started_time) {
DCHECK(installation.unpacking_started_time);
base::UmaHistogramLongTimes(
"Extensions.ForceInstalledTime.UnpackingStartTo."
"CheckingExpectationsStart",
installation.checking_expectations_started_time.value() -
installation.unpacking_started_time.value());
}
if (installation.finalizing_started_time) {
DCHECK(installation.checking_expectations_started_time);
base::UmaHistogramLongTimes(
"Extensions.ForceInstalledTime.CheckingExpectationsStartTo."
"FinalizingStart",
installation.finalizing_started_time.value() -
installation.checking_expectations_started_time.value());
}
if (installation.installation_complete_time) {
DCHECK(installation.finalizing_started_time);
base::UmaHistogramLongTimes(
"Extensions.ForceInstalledTime.FinalizingStartTo."
"CRXInstallComplete",
installation.installation_complete_time.value() -
installation.finalizing_started_time.value());
}
} }
} }
if (missing_forced_extensions.empty()) { if (missing_forced_extensions.empty()) {
......
...@@ -115,6 +115,12 @@ constexpr char kVerificationTimeStats[] = ...@@ -115,6 +115,12 @@ constexpr char kVerificationTimeStats[] =
"Extensions.ForceInstalledTime.VerificationStartTo.CopyingStart"; "Extensions.ForceInstalledTime.VerificationStartTo.CopyingStart";
constexpr char kCopyingTimeStats[] = constexpr char kCopyingTimeStats[] =
"Extensions.ForceInstalledTime.CopyingStartTo.UnpackingStart"; "Extensions.ForceInstalledTime.CopyingStartTo.UnpackingStart";
constexpr char kUnpackingTimeStats[] =
"Extensions.ForceInstalledTime.UnpackingStartTo.CheckingExpectationsStart";
constexpr char kCheckingExpectationsTimeStats[] =
"Extensions.ForceInstalledTime.CheckingExpectationsStartTo.FinalizingStart";
constexpr char kFinalizingTimeStats[] =
"Extensions.ForceInstalledTime.FinalizingStartTo.CRXInstallComplete";
} // namespace } // namespace
...@@ -199,12 +205,9 @@ class ForceInstalledMetricsTest : public testing::Test, ...@@ -199,12 +205,9 @@ class ForceInstalledMetricsTest : public testing::Test,
// Report downloading manifest stage for both the extensions. // Report downloading manifest stage for both the extensions.
void ReportDownloadingManifestStage() { void ReportDownloadingManifestStage() {
auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
install_stage_tracker_->ReportDownloadingStage( install_stage_tracker_->ReportDownloadingStage(
kExtensionId1, kExtensionId1,
ExtensionDownloaderDelegate::Stage::DOWNLOADING_MANIFEST); ExtensionDownloaderDelegate::Stage::DOWNLOADING_MANIFEST);
tracker_->OnExtensionLoaded(profile_, ext1.get());
auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
install_stage_tracker_->ReportDownloadingStage( install_stage_tracker_->ReportDownloadingStage(
kExtensionId2, kExtensionId2,
ExtensionDownloaderDelegate::Stage::DOWNLOADING_MANIFEST); ExtensionDownloaderDelegate::Stage::DOWNLOADING_MANIFEST);
...@@ -340,6 +343,8 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsManifestDownloadTime) { ...@@ -340,6 +343,8 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsManifestDownloadTime) {
ReportDownloadingManifestStage(); ReportDownloadingManifestStage();
install_stage_tracker_->ReportDownloadingStage( install_stage_tracker_->ReportDownloadingStage(
kExtensionId1, ExtensionDownloaderDelegate::Stage::MANIFEST_LOADED); kExtensionId1, ExtensionDownloaderDelegate::Stage::MANIFEST_LOADED);
auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, ext1.get());
install_stage_tracker_->ReportFailure( install_stage_tracker_->ReportFailure(
kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID); kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID);
// ForceInstalledMetrics shuts down timer because all extension are either // ForceInstalledMetrics shuts down timer because all extension are either
...@@ -354,6 +359,8 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsCrxDownloadTime) { ...@@ -354,6 +359,8 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsCrxDownloadTime) {
SetupForceList(); SetupForceList();
ReportDownloadingManifestStage(); ReportDownloadingManifestStage();
ReportInstallationStarted(); ReportInstallationStarted();
auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, ext1.get());
install_stage_tracker_->ReportFailure( install_stage_tracker_->ReportFailure(
kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID); kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID);
// ForceInstalledMetrics shuts down timer because all extension are either // ForceInstalledMetrics shuts down timer because all extension are either
...@@ -372,6 +379,8 @@ TEST_F(ForceInstalledMetricsTest, ...@@ -372,6 +379,8 @@ TEST_F(ForceInstalledMetricsTest,
kExtensionId1, ExtensionDownloaderDelegate::Stage::FINISHED); kExtensionId1, ExtensionDownloaderDelegate::Stage::FINISHED);
install_stage_tracker_->ReportInstallationStage( install_stage_tracker_->ReportInstallationStage(
kExtensionId1, InstallStageTracker::Stage::INSTALLING); kExtensionId1, InstallStageTracker::Stage::INSTALLING);
auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, ext1.get());
install_stage_tracker_->ReportFailure( install_stage_tracker_->ReportFailure(
kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID); kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID);
// ForceInstalledMetrics shuts down timer because all extension are either // ForceInstalledMetrics shuts down timer because all extension are either
...@@ -393,6 +402,14 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReportInstallationStageTimes) { ...@@ -393,6 +402,14 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReportInstallationStageTimes) {
kExtensionId1, InstallationStage::kCopying); kExtensionId1, InstallationStage::kCopying);
install_stage_tracker_->ReportCRXInstallationStage( install_stage_tracker_->ReportCRXInstallationStage(
kExtensionId1, InstallationStage::kUnpacking); kExtensionId1, InstallationStage::kUnpacking);
install_stage_tracker_->ReportCRXInstallationStage(
kExtensionId1, InstallationStage::kCheckingExpectations);
install_stage_tracker_->ReportCRXInstallationStage(
kExtensionId1, InstallationStage::kFinalizing);
install_stage_tracker_->ReportCRXInstallationStage(
kExtensionId1, InstallationStage::kComplete);
auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, ext1.get());
install_stage_tracker_->ReportFailure( install_stage_tracker_->ReportFailure(
kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID); kExtensionId2, InstallStageTracker::FailureReason::MANIFEST_INVALID);
// ForceInstalledMetrics shuts down timer because all extension are either // ForceInstalledMetrics shuts down timer because all extension are either
...@@ -400,6 +417,9 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReportInstallationStageTimes) { ...@@ -400,6 +417,9 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReportInstallationStageTimes) {
EXPECT_FALSE(fake_timer_->IsRunning()); EXPECT_FALSE(fake_timer_->IsRunning());
histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1); histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1);
histogram_tester_.ExpectTotalCount(kCopyingTimeStats, 1); histogram_tester_.ExpectTotalCount(kCopyingTimeStats, 1);
histogram_tester_.ExpectTotalCount(kUnpackingTimeStats, 1);
histogram_tester_.ExpectTotalCount(kCheckingExpectationsTimeStats, 1);
histogram_tester_.ExpectTotalCount(kFinalizingTimeStats, 1);
} }
// Reporting disable reason for the force installed extensions which are // Reporting disable reason for the force installed extensions which are
......
...@@ -197,6 +197,12 @@ void InstallStageTracker::ReportCRXInstallationStage(const ExtensionId& id, ...@@ -197,6 +197,12 @@ void InstallStageTracker::ReportCRXInstallationStage(const ExtensionId& id,
data.copying_started_time = current_time; data.copying_started_time = current_time;
else if (stage == InstallationStage::kUnpacking) else if (stage == InstallationStage::kUnpacking)
data.unpacking_started_time = current_time; data.unpacking_started_time = current_time;
else if (stage == InstallationStage::kCheckingExpectations)
data.checking_expectations_started_time = current_time;
else if (stage == InstallationStage::kFinalizing)
data.finalizing_started_time = current_time;
else if (stage == InstallationStage::kComplete)
data.installation_complete_time = current_time;
for (auto& observer : observers_) { for (auto& observer : observers_) {
observer.OnExtensionDataChangedForTesting(id, browser_context_, data); observer.OnExtensionDataChangedForTesting(id, browser_context_, data);
......
...@@ -331,6 +331,14 @@ class InstallStageTracker : public KeyedService { ...@@ -331,6 +331,14 @@ class InstallStageTracker : public KeyedService {
base::Optional<base::TimeTicks> copying_started_time; base::Optional<base::TimeTicks> copying_started_time;
// Time at which unpacking of the extension archive is started. // Time at which unpacking of the extension archive is started.
base::Optional<base::TimeTicks> unpacking_started_time; base::Optional<base::TimeTicks> unpacking_started_time;
// Time at which the extension archive has been successfully unpacked and
// the expectation checks before extension installation are started.
base::Optional<base::TimeTicks> checking_expectations_started_time;
// Time at which the extension has passed the expectation checks and the
// installation is started.
base::Optional<base::TimeTicks> finalizing_started_time;
// Time at which the installation process is complete.
base::Optional<base::TimeTicks> installation_complete_time;
}; };
class Observer : public base::CheckedObserver { class Observer : public base::CheckedObserver {
......
...@@ -15,9 +15,12 @@ enum class InstallationStage { ...@@ -15,9 +15,12 @@ enum class InstallationStage {
kCopying = 1, kCopying = 1,
// Extension archive is about to be unpacked. // Extension archive is about to be unpacked.
kUnpacking = 2, kUnpacking = 2,
// Final checks before the installation is finished. // Performing the expectation checks before the installation can be started.
kFinalizing = 3, kCheckingExpectations = 3,
kComplete = 4, // Installation of unpacked extension is started.
kFinalizing = 4,
// Extension installation process is complete.
kComplete = 5,
}; };
} // namespace extensions } // namespace extensions
......
...@@ -203771,13 +203771,21 @@ regressions. --> ...@@ -203771,13 +203771,21 @@ regressions. -->
</histogram_suffixes> </histogram_suffixes>
<histogram_suffixes name="ExtensionInstallStages" separator="."> <histogram_suffixes name="ExtensionInstallStages" separator=".">
<suffix name="CheckingExpectationsStartTo.FinalizingStart"
label="Time taken to perform the expectations checks to confirm that
the extension can be installed."/>
<suffix name="CopyingStartTo.UnpackingStart" <suffix name="CopyingStartTo.UnpackingStart"
label="Time taken to complete copying of extension archive into the label="Time taken to complete copying of extension archive into the
working directory"/> working directory"/>
<suffix name="DownloadingStartTo.ManifestDownloadComplete" <suffix name="DownloadingStartTo.ManifestDownloadComplete"
label="Time taken to complete download of update manifest"/> label="Time taken to complete download of update manifest"/>
<suffix name="FinalizingStartTo.CRXInstallComplete"
label="Time taken to complete the installation of the unpacked
extension."/>
<suffix name="ManifestDownloadCompleteTo.CRXDownloadComplete" <suffix name="ManifestDownloadCompleteTo.CRXDownloadComplete"
label="Time taken to complete download of CRX"/> label="Time taken to complete download of CRX"/>
<suffix name="UnpackingStartTo.CheckingExpectationsStart"
label="Time taken to complete the unpacking of the extension archive."/>
<suffix name="VerificationStartTo.CopyingStart" <suffix name="VerificationStartTo.CopyingStart"
label="Time taken to complete signature verification of CRX"/> label="Time taken to complete signature verification of CRX"/>
<affected-histogram name="Extensions.ForceInstalledTime"/> <affected-histogram name="Extensions.ForceInstalledTime"/>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment