Commit ca39e031 authored by Findit's avatar Findit

Revert "Report installation stages in increasing order"

This reverts commit 7ff62f12.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 809777 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzdmZjYyZjEyMGUyNTUzMmRmYWU2NjAzNTE0NTJiN2JkODM3MzM3NjQM

Sample Failed Build: https://ci.chromium.org/b/8868354221177409968

Sample Failed Step: compile

Original change's description:
> Report installation stages in increasing order
> 
> This CL reports the installation stages for force installed extensions
> in an increasing order. This is done in order to investigate
> the extensions stuck in CREATED stage.
> 
> Bug: 989526
> Change-Id: I82e07f30c6768c7bde30a76b61b9987173f76fbd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421375
> Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
> Reviewed-by: Oleg Davydov <burunduk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#809777}


Change-Id: I692551b411e6f50b1a75e54c9d941920e0a370c2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 989526
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424816
Cr-Commit-Position: refs/heads/master@{#809791}
parent 36caab03
......@@ -592,50 +592,6 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReady) {
histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
}
// Verifies that the installation stage is not overwritten by a previous stage.
TEST_F(ForceInstalledMetricsTest,
ExtensionsPreviousInstallationStageReportedAgain) {
SetupForceList();
auto extension =
ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, extension.get());
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::CREATED);
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::PENDING);
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::CREATED);
EXPECT_TRUE(fake_timer_->IsRunning());
fake_timer_->Fire();
histogram_tester_.ExpectUniqueSample(
kFailureReasonsCWS, InstallStageTracker::FailureReason::IN_PROGRESS, 1);
histogram_tester_.ExpectBucketCount(kInstallationStages,
InstallStageTracker::Stage::PENDING, 1);
}
// Verifies that the installation stage is overwritten if DOWNLOADING stage is
// reported again after INSTALLING stage.
TEST_F(ForceInstalledMetricsTest, ExtensionsDownloadingStageReportedAgain) {
SetupForceList();
auto extension =
ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
tracker_->OnExtensionLoaded(profile_, extension.get());
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::DOWNLOADING);
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::INSTALLING);
install_stage_tracker_->ReportInstallationStage(
kExtensionId2, InstallStageTracker::Stage::DOWNLOADING);
install_stage_tracker_->ReportDownloadingStage(
kExtensionId2, ExtensionDownloaderDelegate::Stage::PENDING);
EXPECT_TRUE(fake_timer_->IsRunning());
fake_timer_->Fire();
histogram_tester_.ExpectUniqueSample(
kFailureReasonsCWS, InstallStageTracker::FailureReason::IN_PROGRESS, 1);
histogram_tester_.ExpectBucketCount(
kInstallationStages, InstallStageTracker::Stage::DOWNLOADING, 1);
}
TEST_F(ForceInstalledMetricsTest, ExtensionsStuck) {
SetupForceList();
install_stage_tracker()->ReportInstallationStage(
......
......@@ -15,24 +15,6 @@
namespace extensions {
namespace {
// Returns true if the |current_stage| should be overridden by the
// |new_stage|.
bool ShouldOverrideCurrentStage(
base::Optional<InstallStageTracker::Stage> current_stage,
InstallStageTracker::Stage new_stage) {
if (!current_stage)
return true;
// If CRX was from the cache and was damaged as a file, we would try to
// download the CRX after reporting the INSTALLING stage.
if (current_stage == InstallStageTracker::Stage::INSTALLING &&
new_stage == InstallStageTracker::Stage::DOWNLOADING)
return true;
return new_stage > current_stage;
}
} // namespace
#if defined(OS_CHROMEOS)
InstallStageTracker::UserInfo::UserInfo(const UserInfo&) = default;
InstallStageTracker::UserInfo::UserInfo(user_manager::UserType user_type,
......@@ -180,8 +162,6 @@ void InstallStageTracker::ReportInstallCreationStage(
void InstallStageTracker::ReportInstallationStage(const ExtensionId& id,
Stage stage) {
InstallationData& data = installation_data_map_[id];
if (!ShouldOverrideCurrentStage(data.install_stage, stage))
return;
data.install_stage = stage;
for (auto& observer : observers_) {
observer.OnExtensionInstallationStageChanged(id, stage);
......
......@@ -38,13 +38,10 @@ class InstallStageTracker : public KeyedService {
public:
// Stage of extension installing process. Typically forced extensions from
// policies should go through all stages in this order, other extensions skip
// CREATED stage. The stages are recorded in the increasing order of their
// values, therefore always verify that values are in increasing order and
// items are in order in which they appear. Exceptions are handled in
// ShouldOverrideCurrentStage method. Note: enum used for UMA. Do NOT reorder
// or remove entries. Don't forget to update enums.xml (name:
// ExtensionInstallationStage) when adding new entries. Don't forget to update
// device_management_backend.proto (name:
// CREATED stage.
// Note: enum used for UMA. Do NOT reorder or remove entries. Don't forget to
// update enums.xml (name: ExtensionInstallationStage) when adding new
// entries. Don't forget to update device_management_backend.proto (name:
// ExtensionInstallReportLogEvent::InstallationStage) when adding new entries.
// Don't forget to update ConvertInstallationStageToProto method in
// ExtensionInstallEventLogCollector.
......
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