Reland "Take user data snapshots on canary builds for each minor

upgrade."

Reason for reland: Added trace event.

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}

Bug: 958893
Change-Id: Ia72ab78c6fa56bee69051265d883f7ca6d03b79d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144836Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Yann Dago <ydago@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760091}

chrome/browser/downgrade/downgrade_manager.cc

@@ -32,6 +32,7 @@
 #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"
@@ -222,30 +223,36 @@ bool DowngradeManager::PrepareUserDataDirectoryForCurrentVersion(
     return type_ == Type::kAdministrativeWipe;
   }
 
-  auto current_milestone = current_version.components()[0];
-  auto last_milestone = last_version->components()[0];
-
-  // 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;
-  }
+  if (current_version == *last_version)
+    return false;  // Nothing to do if the version has not changed.
 
-  if (current_version >= *last_version)
-    return false;  // Same version or mid-milestone upgrade.
+  if (current_version < *last_version) {
+    type_ = GetDowngradeTypeWithSnapshot(user_data_dir, current_version,
+                                         *last_version);
+    if (type_ != Type::kNone)
+      base::UmaHistogramEnumeration("Downgrade.Type", type_);
 
-  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;
+  }
 
-  return type_ == Type::kAdministrativeWipe || type_ == Type::kSnapshotRestore;
+  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;
 }
 
 void DowngradeManager::UpdateLastVersion(const base::FilePath& user_data_dir) {

chrome/browser/downgrade/snapshot_manager.cc

@@ -13,6 +13,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/trace_event/trace_event.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
 #include "chrome/browser/downgrade/downgrade_utils.h"
 #include "chrome/browser/downgrade/snapshot_file_collector.h"
@@ -147,6 +148,7 @@ SnapshotManager::SnapshotManager(const base::FilePath& user_data_dir)
 SnapshotManager::~SnapshotManager() = default;
 
 void SnapshotManager::TakeSnapshot(const base::Version& version) {
+  TRACE_EVENT0("browser", "SnapshotManager::TakeSnapshot");
   DCHECK(version.IsValid());
   base::FilePath snapshot_dir =
       user_data_dir_.Append(kSnapshotsDir).AppendASCII(version.GetString());
@@ -235,6 +237,7 @@ void SnapshotManager::TakeSnapshot(const base::Version& version) {
 }
 
 void SnapshotManager::RestoreSnapshot(const base::Version& version) {
+  TRACE_EVENT0("browser", "SnapshotManager::RestoreSnapshot");
   DCHECK(version.IsValid());
   auto snapshot_version = GetSnapshotToRestore(version, user_data_dir_);
   if (!snapshot_version)
@@ -313,7 +316,8 @@ void SnapshotManager::RestoreSnapshot(const base::Version& version) {
 }
 
 void SnapshotManager::PurgeInvalidAndOldSnapshots(
-    int max_number_of_snapshots) const {
+    int max_number_of_snapshots,
+    base::Optional<uint32_t> milestone) const {
   const auto snapshot_dir = user_data_dir_.Append(kSnapshotsDir);
 
   // Move the invalid snapshots within from Snapshots/NN to Snapshots.DELETE/NN.
@@ -329,7 +333,17 @@ void SnapshotManager::PurgeInvalidAndOldSnapshots(
                                "Downgrade.InvalidSnapshotMove.FailureCount");
   }
 
-  auto available_snapshots = GetAvailableSnapshots(snapshot_dir);
+  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})));
+  }
 
   if (available_snapshots.size() <=
       base::checked_cast<size_t>(max_number_of_snapshots)) {

chrome/browser/downgrade/snapshot_manager.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/files/file_path.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "base/version.h"
 
@@ -42,8 +43,10 @@ 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.
-  void PurgeInvalidAndOldSnapshots(int max_number_of_snapshots) const;
+  // 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;
 
   // 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,7 +383,8 @@ TEST_F(SnapshotManagerTest, PurgeInvalidAndOldSnapshotsKeepsMaxValidSnapshots) {
 
   int max_number_of_snapshots = 3;
   SnapshotManager snapshot_manager(user_data_dir());
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots);
+  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots,
+                                               base::nullopt);
 
   const base::FilePath deletion_directory =
       user_data_dir()
@@ -418,12 +419,38 @@ TEST_F(SnapshotManagerTest, PurgeInvalidAndOldSnapshotsKeepsValidSnapshots) {
                base::File::FLAG_CREATE | base::File::FLAG_WRITE);
   }
 
-  size_t max_number_of_snapshots = 3;
+  int max_number_of_snapshots = 3;
   SnapshotManager snapshot_manager(user_data_dir());
-  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots);
+  snapshot_manager.PurgeInvalidAndOldSnapshots(max_number_of_snapshots,
+                                               base::nullopt);
 
   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,6 +111,7 @@ 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 milestones that have a complete snapshot available.
+// Returns the versions 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
-// |milestone|.
+// |version|.
 base::Optional<base::Version> GetSnapshotToRestore(
     const base::Version& version,
     const base::FilePath& user_data_dir);

chrome/browser/downgrade/user_data_snapshot_browsertest.cc

@@ -18,6 +18,7 @@
 #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"
@@ -45,6 +46,12 @@
 #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 {
@@ -417,4 +424,81 @@ 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