Revert "Take user data snapshots on canary builds for each minor upgrade."

This reverts commit 0c6bc2e3.

Reason for revert: reverting to avoid perf analysis surprises -- to be relanded with a trace event.

Original change's description:
> Take user data snapshots on canary builds for each minor upgrade.
> 
> Bug: 958893
> Change-Id: I987a99d3db2b659babe46d2f15cfac6627bc573c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134396
> Commit-Queue: Yann Dago <ydago@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#757835}

TBR=grt@chromium.org,ydago@chromium.org

Change-Id: Ie5bc93a13e952ae13ab90249adb93623eb3261c3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 958893
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144173Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758018}

chrome/browser/downgrade/downgrade_manager.cc

@@ -32,7 +32,6 @@
 #include "chrome/browser/downgrade/snapshot_manager.h"
 #include "chrome/browser/downgrade/user_data_downgrade.h"
 #include "chrome/browser/policy/browser_dm_token_storage.h"
-#include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
@@ -223,36 +222,30 @@ bool DowngradeManager::PrepareUserDataDirectoryForCurrentVersion(
     return type_ == Type::kAdministrativeWipe;
   }
 
-  if (current_version == *last_version)
-    return false;  // Nothing to do if the version has not changed.
-
-  if (current_version < *last_version) {
-    type_ = GetDowngradeTypeWithSnapshot(user_data_dir, current_version,
-                                         *last_version);
-    if (type_ != Type::kNone)
-      base::UmaHistogramEnumeration("Downgrade.Type", type_);
+  auto current_milestone = current_version.components()[0];
+  auto last_milestone = last_version->components()[0];
 
-    return type_ == Type::kAdministrativeWipe ||
-           type_ == Type::kSnapshotRestore;
+  // Take a snapshot on the first launch after a major version jump.
+  if (current_milestone > last_milestone) {
+    const int max_number_of_snapshots =
+        g_browser_process->local_state()->GetInteger(
+            prefs::kUserDataSnapshotRentionLimit);
+    SnapshotManager snapshot_manager(user_data_dir);
+    if (max_number_of_snapshots > 0)
+      snapshot_manager.TakeSnapshot(*last_version);
+    snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots);
+    return false;
   }
 
-  auto current_milestone = current_version.components()[0];
-  int max_number_of_snapshots = g_browser_process->local_state()->GetInteger(
-      prefs::kUserDataSnapshotRentionLimit);
-  base::Optional<uint32_t> purge_milestone;
-  if (current_milestone == last_version->components()[0]) {
-    // Mid-milestone snapshots are only taken on canary installs.
-    if (chrome::GetChannel() != version_info::Channel::CANARY)
-      return false;
-    // Keep one snapshot in this milestone unless snapshots are disabled.
-    max_number_of_snapshots = std::min(max_number_of_snapshots, 1);
-    purge_milestone = current_milestone;
-  }
-  SnapshotManager snapshot_manager(user_data_dir);
-  snapshot_manager.TakeSnapshot(*last_version);
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots,
-                                               purge_milestone);
-  return false;
+  if (current_version >= *last_version)
+    return false;  // Same version or mid-milestone upgrade.
+
+  type_ = GetDowngradeTypeWithSnapshot(user_data_dir, current_version,
+                                       *last_version);
+  if (type_ != Type::kNone)
+    base::UmaHistogramEnumeration("Downgrade.Type", type_);
+
+  return type_ == Type::kAdministrativeWipe || type_ == Type::kSnapshotRestore;
 }
 
 void DowngradeManager::UpdateLastVersion(const base::FilePath& user_data_dir) {

chrome/browser/downgrade/snapshot_manager.cc

@@ -313,8 +313,7 @@ void SnapshotManager::RestoreSnapshot(const base::Version& version) {
 }
 
 void SnapshotManager::PurgeInvalidAndOldSnapshots(
-    int max_number_of_snapshots,
-    base::Optional<uint32_t> milestone) const {
+    int max_number_of_snapshots) const {
   const auto snapshot_dir = user_data_dir_.Append(kSnapshotsDir);
 
   // Move the invalid snapshots within from Snapshots/NN to Snapshots.DELETE/NN.
@@ -330,17 +329,7 @@ void SnapshotManager::PurgeInvalidAndOldSnapshots(
                                "Downgrade.InvalidSnapshotMove.FailureCount");
   }
 
-  base::flat_set<base::Version> available_snapshots =
-      GetAvailableSnapshots(snapshot_dir);
-  if (milestone.has_value()) {
-    // Only consider versions for the specified milestone.
-    available_snapshots.erase(available_snapshots.upper_bound(
-                                  base::Version({*milestone + 1, 0, 0, 0})),
-                              available_snapshots.end());
-    available_snapshots.erase(
-        available_snapshots.begin(),
-        available_snapshots.lower_bound(base::Version({*milestone, 0, 0, 0})));
-  }
+  auto available_snapshots = GetAvailableSnapshots(snapshot_dir);
 
   if (available_snapshots.size() <=
       base::checked_cast<size_t>(max_number_of_snapshots)) {

chrome/browser/downgrade/snapshot_manager.h

@@ -9,7 +9,6 @@
 #include <vector>
 
 #include "base/files/file_path.h"
-#include "base/optional.h"
 #include "base/time/time.h"
 #include "base/version.h"
 
@@ -43,10 +42,8 @@ class SnapshotManager {
   void RestoreSnapshot(const base::Version& version);
 
   // Keeps the number of snapshots on the disk under |max_number_of_snapshots|
-  // by moving invalid and older snapshots for later deletion. If |milestone| is
-  // specified, limit the deletion to the snapshots from that milestone.
-  void PurgeInvalidAndOldSnapshots(int max_number_of_snapshots,
-                                   base::Optional<uint32_t> milestone) const;
+  // by moving invalid and older snapshots for later deletion.
+  void PurgeInvalidAndOldSnapshots(int max_number_of_snapshots) const;
 
   // Deletes snapshot data created after |delete_begin| for |profile_base_name|.
   // |remove_mask| (of bits from ChromeBrowsingDataRemoverDelegate::DataType)

chrome/browser/downgrade/snapshot_manager_unittest.cc

@@ -383,8 +383,7 @@ TEST_F(SnapshotManagerTest, PurgeInvalidAndOldSnapshotsKeepsMaxValidSnapshots) {
 
   int max_number_of_snapshots = 3;
   SnapshotManager snapshot_manager(user_data_dir());
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots,
-                                               base::nullopt);
+  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots);
 
   const base::FilePath deletion_directory =
       user_data_dir()
@@ -419,38 +418,12 @@ TEST_F(SnapshotManagerTest, PurgeInvalidAndOldSnapshotsKeepsValidSnapshots) {
                base::File::FLAG_CREATE | base::File::FLAG_WRITE);
   }
 
-  int max_number_of_snapshots = 3;
+  size_t max_number_of_snapshots = 3;
   SnapshotManager snapshot_manager(user_data_dir());
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots,
-                                               base::nullopt);
+  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots);
 
   for (const auto& path : valid_snapshot_paths)
     EXPECT_TRUE(base::PathExists(path));
 }
 
-TEST_F(SnapshotManagerTest,
-       PurgeInvalidAndOldSnapshotsKeepsValidSnapshotsPerMilestone) {
-  std::vector<base::FilePath> valid_snapshot_paths{
-      user_data_dir().Append(kSnapshotsDir).AppendASCII("19.0.0"),
-      user_data_dir().Append(kSnapshotsDir).AppendASCII("20.0.0"),
-      user_data_dir().Append(kSnapshotsDir).AppendASCII("20.0.1"),
-      user_data_dir().Append(kSnapshotsDir).AppendASCII("21.0.1"),
-  };
-
-  for (const auto& path : valid_snapshot_paths) {
-    ASSERT_TRUE(base::CreateDirectory(path));
-    base::File(path.Append(kDowngradeLastVersionFile),
-               base::File::FLAG_CREATE | base::File::FLAG_WRITE);
-  }
-
-  int max_number_of_snapshots = 1;
-  SnapshotManager snapshot_manager(user_data_dir());
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots, 20);
-
-  EXPECT_TRUE(base::PathExists(valid_snapshot_paths[0]));
-  EXPECT_FALSE(base::PathExists(valid_snapshot_paths[1]));
-  EXPECT_TRUE(base::PathExists(valid_snapshot_paths[2]));
-  EXPECT_TRUE(base::PathExists(valid_snapshot_paths[3]));
-}
-
 }  // namespace downgrade

chrome/browser/downgrade/user_data_downgrade.cc

@@ -111,7 +111,6 @@ std::vector<base::FilePath> GetInvalidSnapshots(
 base::Optional<base::Version> GetSnapshotToRestore(
     const base::Version& version,
     const base::FilePath& user_data_dir) {
-  DCHECK(version.IsValid());
   base::FilePath top_snapshot_dir = user_data_dir.Append(kSnapshotsDir);
   auto available_snapshots = GetAvailableSnapshots(top_snapshot_dir);
 

chrome/browser/downgrade/user_data_downgrade.h

@@ -40,7 +40,7 @@ base::Optional<base::Version> GetLastVersion(
 // within the User Data directory).
 base::FilePath GetDiskCacheDir();
 
-// Returns the versions that have a complete snapshot available.
+// Returns the milestones that have a complete snapshot available.
 base::flat_set<base::Version> GetAvailableSnapshots(
     const base::FilePath& snapshot_dir);
 
@@ -50,7 +50,7 @@ std::vector<base::FilePath> GetInvalidSnapshots(
     const base::FilePath& snapshot_dir);
 
 // Return the highest available snapshot version that is not greater than
-// |version|.
+// |milestone|.
 base::Optional<base::Version> GetSnapshotToRestore(
     const base::Version& version,
     const base::FilePath& user_data_dir);

chrome/browser/downgrade/user_data_snapshot_browsertest.cc

@@ -18,7 +18,6 @@
 #include "base/test/mock_callback.h"
 #include "base/time/time.h"
 #include "base/version.h"
-#include "build/branding_buildflags.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/downgrade/downgrade_manager.h"
 #include "chrome/browser/first_run/scoped_relaunch_chrome_browser_override.h"
@@ -46,12 +45,6 @@
 #include "ui/base/page_transition_types.h"
 #include "ui/base/window_open_disposition.h"
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-#include "base/threading/thread_restrictions.h"
-#include "chrome/install_static/install_modes.h"
-#include "chrome/install_static/test/scoped_install_details.h"
-#endif
-
 namespace downgrade {
 
 namespace {
@@ -424,81 +417,4 @@ IN_PROC_BROWSER_TEST_F(TabsSnapshotTest, PRE_PRE_Test) {}
 IN_PROC_BROWSER_TEST_F(TabsSnapshotTest, PRE_Test) {}
 IN_PROC_BROWSER_TEST_F(TabsSnapshotTest, Test) {}
 
-// Tests that Google Chrome does not takes snapshots on mid-milestone updates.
-IN_PROC_BROWSER_TEST_F(InProcessBrowserTest, SameMilestoneSnapshot) {
-  DowngradeManager::EnableSnapshotsForTesting(true);
-  base::ScopedAllowBlockingForTesting scoped_allow_blocking;
-  base::FilePath user_data_dir;
-  ASSERT_TRUE(base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
-  auto current_version = version_info::GetVersion().GetString();
-
-  downgrade::DowngradeManager downgrade_manager;
-
-  // No snapshots for same version.
-  base::WriteFile(user_data_dir.Append(kDowngradeLastVersionFile),
-                  current_version.c_str(), current_version.size());
-  EXPECT_FALSE(downgrade_manager.PrepareUserDataDirectoryForCurrentVersion(
-      user_data_dir));
-  EXPECT_FALSE(
-      base::PathExists(user_data_dir.Append(downgrade::kSnapshotsDir)));
-
-  // Snapshot taken for minor update
-  std::vector<uint32_t> last_minor_version_components;
-  for (const auto& component : version_info::GetVersion().components()) {
-    // Decrement all but the major version.
-    last_minor_version_components.push_back(
-        !last_minor_version_components.empty() && component > 0 ? component - 1
-                                                                : component);
-  }
-  auto last_minor_version =
-      base::Version(last_minor_version_components).GetString();
-  base::WriteFile(user_data_dir.Append(kDowngradeLastVersionFile),
-                  last_minor_version.c_str(), last_minor_version.size());
-
-  EXPECT_FALSE(downgrade_manager.PrepareUserDataDirectoryForCurrentVersion(
-      user_data_dir));
-  EXPECT_FALSE(
-      base::PathExists(user_data_dir.Append(downgrade::kSnapshotsDir)));
-}
-
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-// Tests that Google Chrome canary takes snapshots on mid-milestone updates.
-IN_PROC_BROWSER_TEST_F(InProcessBrowserTest, CanarySameMilestoneSnapshot) {
-  DowngradeManager::EnableSnapshotsForTesting(true);
-  install_static::ScopedInstallDetails install_details(
-      /*system_level=*/false, install_static::CANARY_INDEX);
-  base::ScopedAllowBlockingForTesting scoped_allow_blocking;
-  base::FilePath user_data_dir;
-  ASSERT_TRUE(base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
-  auto current_version = version_info::GetVersion().GetString();
-
-  downgrade::DowngradeManager downgrade_manager;
-
-  // No snapshots for same version.
-  base::WriteFile(user_data_dir.Append(kDowngradeLastVersionFile),
-                  current_version.c_str(), current_version.size());
-  EXPECT_FALSE(downgrade_manager.PrepareUserDataDirectoryForCurrentVersion(
-      user_data_dir));
-  EXPECT_FALSE(
-      base::PathExists(user_data_dir.Append(downgrade::kSnapshotsDir)));
-
-  // Snapshot taken for minor update
-  std::vector<uint32_t> last_minor_version_components;
-  for (const auto& component : version_info::GetVersion().components()) {
-    // Decrement all but the major version.
-    last_minor_version_components.push_back(
-        !last_minor_version_components.empty() && component > 0 ? component - 1
-                                                                : component);
-  }
-  auto last_minor_version =
-      base::Version(last_minor_version_components).GetString();
-  base::WriteFile(user_data_dir.Append(kDowngradeLastVersionFile),
-                  last_minor_version.c_str(), last_minor_version.size());
-
-  EXPECT_FALSE(downgrade_manager.PrepareUserDataDirectoryForCurrentVersion(
-      user_data_dir));
-  EXPECT_TRUE(base::PathExists(user_data_dir.Append(downgrade::kSnapshotsDir)));
-}
-#endif
-
 }  // namespace downgrade