Record last known Crostini OS version in user prefs.

This is needed so we can offer an upgrade on the next Crostini start.

Bug: 1024693
Change-Id: I204ab5348330d9a2869e420c1c0f828e55116323
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011472Reviewed-by: David Munro <davidmunro@google.com>
Reviewed-by: Fergus Dall <sidereal@google.com>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733889}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -645,11 +645,35 @@ void CrostiniManager::SetContainerSshfsMounted(std::string vm_name,
   }
 }
 
+namespace {
+
+ContainerOsVersion VersionFromOsRelease(
+    const vm_tools::cicerone::OsRelease& os_release) {
+  if (os_release.id() == "debian") {
+    if (os_release.version_id() == "9") {
+      return ContainerOsVersion::kDebianStretch;
+    } else if (os_release.version_id() == "10") {
+      return ContainerOsVersion::kDebianBuster;
+    } else {
+      return ContainerOsVersion::kDebianOther;
+    }
+  }
+  return ContainerOsVersion::kOtherOs;
+}
+
+}  // namespace
+
 void CrostiniManager::SetContainerOsRelease(
     std::string vm_name,
     std::string container_name,
     const vm_tools::cicerone::OsRelease& os_release) {
   ContainerId container_id(vm_name, container_name);
+  ContainerOsVersion version = VersionFromOsRelease(os_release);
+  // Store the os release version in prefs. We can use this value to decide if
+  // an upgrade can be offered.
+  UpdateContainerPref(profile_, container_id, prefs::kContainerOsVersionKey,
+                      base::Value(static_cast<int>(version)));
+
   VLOG(1) << container_id;
   VLOG(1) << "os_release.pretty_name " << os_release.pretty_name();
   VLOG(1) << "os_release.name " << os_release.name();
@@ -657,23 +681,7 @@ void CrostiniManager::SetContainerOsRelease(
   VLOG(1) << "os_release.version_id " << os_release.version_id();
   VLOG(1) << "os_release.id " << os_release.id();
   container_os_releases_.emplace(std::move(container_id), os_release);
-  EmitContainerVersionMetric(os_release);
-}
 
-void CrostiniManager::EmitContainerVersionMetric(
-    const vm_tools::cicerone::OsRelease& os_release) {
-  ContainerOsVersion version;
-  if (os_release.id() == "debian") {
-    if (os_release.version_id() == "9") {
-      version = ContainerOsVersion::kDebianStretch;
-    } else if (os_release.version_id() == "10") {
-      version = ContainerOsVersion::kDebianBuster;
-    } else {
-      version = ContainerOsVersion::kDebianOther;
-    }
-  } else {
-    version = ContainerOsVersion::kOtherOs;
-  }
   base::UmaHistogramEnumeration("Crostini.ContainerOsVersion", version);
 }
 

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -781,10 +781,6 @@ class CrostiniManager : public KeyedService,
 
   void OnVmStoppedCleanup(const std::string& vm_name);
 
-  // Emits a UMA recording the OS version.
-  void EmitContainerVersionMetric(
-      const vm_tools::cicerone::OsRelease& os_release);
-
   // Configure the container so that it can sideload apps into Arc++.
   void ConfigureForArcSideload();
 

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -1145,6 +1145,14 @@ TEST_F(CrostiniManagerRestartTest, OsReleaseSetCorrectly) {
             stored_os_release->SerializeAsString());
   histogram_tester.ExpectUniqueSample("Crostini.ContainerOsVersion",
                                       ContainerOsVersion::kDebianBuster, 1);
+
+  // The data for this container should also be stored in prefs.
+  const base::Value* os_release_pref_value =
+      GetContainerPrefValue(profile(), ContainerId(kVmName, kContainerName),
+                            prefs::kContainerOsVersionKey);
+  EXPECT_NE(os_release_pref_value, nullptr);
+  EXPECT_EQ(os_release_pref_value->GetInt(),
+            static_cast<int>(ContainerOsVersion::kDebianBuster));
 }
 
 TEST_F(CrostiniManagerRestartTest, RestartThenUninstall) {

chrome/browser/chromeos/crostini/crostini_pref_names.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/values.h"
+#include "chrome/browser/chromeos/crostini/crostini_simple_types.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_registry_simple.h"
@@ -28,6 +29,7 @@ const char kCrostiniContainers[] = "crostini.containers";
 const char kCrostiniTerminalSettings[] = "crostini.terminal_settings";
 const char kVmKey[] = "vm_name";
 const char kContainerKey[] = "container_name";
+const char kContainerOsVersionKey[] = "container_os_version";
 
 // A boolean preference representing a user level enterprise policy to enable
 // Crostini use.

chrome/browser/chromeos/crostini/crostini_pref_names.h

@@ -18,6 +18,7 @@ extern const char kCrostiniContainers[];
 extern const char kCrostiniTerminalSettings[];
 extern const char kVmKey[];
 extern const char kContainerKey[];
+extern const char kContainerOsVersionKey[];
 
 extern const char kUserCrostiniAllowedByPolicy[];
 extern const char kUserCrostiniExportImportUIAllowedByPolicy[];

chrome/browser/chromeos/crostini/crostini_simple_types.h

@@ -188,7 +188,7 @@ enum class CorruptionStates {
 }  // namespace crostini
 
 enum class ContainerOsVersion {
-  kUnkown = 0,
+  kUnknown = 0,
   kDebianStretch = 1,
   kDebianBuster = 2,
   kDebianOther = 3,

chrome/browser/chromeos/crostini/crostini_util.cc

@@ -503,22 +503,36 @@ void AddNewLxdContainerToPrefs(Profile* profile,
   base::Value new_container(base::Value::Type::DICTIONARY);
   new_container.SetKey(prefs::kVmKey, base::Value(vm_name));
   new_container.SetKey(prefs::kContainerKey, base::Value(container_name));
+  new_container.SetIntKey(prefs::kContainerOsVersionKey,
+                          static_cast<int>(ContainerOsVersion::kUnknown));
 
   ListPrefUpdate updater(pref_service, crostini::prefs::kCrostiniContainers);
   updater->Append(std::move(new_container));
 }
 
+namespace {
+
+bool MatchContainerDict(const base::Value& dict,
+                        const ContainerId& container_id) {
+  const std::string* vm_name = dict.FindStringKey(prefs::kVmKey);
+  const std::string* container_name = dict.FindStringKey(prefs::kContainerKey);
+  return (vm_name && *vm_name == container_id.vm_name) &&
+         (container_name && *container_name == container_id.container_name);
+}
+
+}  // namespace
+
 void RemoveLxdContainerFromPrefs(Profile* profile,
                                  std::string vm_name,
                                  std::string container_name) {
   auto* pref_service = profile->GetPrefs();
   ListPrefUpdate updater(pref_service, crostini::prefs::kCrostiniContainers);
-  updater->EraseListIter(std::find_if(
-      updater->GetList().begin(), updater->GetList().end(),
-      [&](const auto& dict) {
-        return *dict.FindStringKey(prefs::kVmKey) == vm_name &&
-               *dict.FindStringKey(prefs::kContainerKey) == container_name;
-      }));
+  ContainerId container_id(vm_name, container_name);
+  updater->EraseListIter(
+      std::find_if(updater->GetList().begin(), updater->GetList().end(),
+                   [&](const auto& dict) {
+                     return MatchContainerDict(dict, container_id);
+                   }));
 
   CrostiniRegistryServiceFactory::GetForProfile(profile)->ClearApplicationList(
       vm_name, container_name);
@@ -526,6 +540,37 @@ void RemoveLxdContainerFromPrefs(Profile* profile,
       vm_name, container_name);
 }
 
+const base::Value* GetContainerPrefValue(Profile* profile,
+                                         const ContainerId& container_id,
+                                         const std::string& key) {
+  const base::ListValue* containers =
+      profile->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
+  if (!containers) {
+    return nullptr;
+  }
+  auto it = std::find_if(
+      containers->begin(), containers->end(),
+      [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
+  if (it == containers->end()) {
+    return nullptr;
+  }
+  return it->FindKey(key);
+}
+
+void UpdateContainerPref(Profile* profile,
+                         const ContainerId& container_id,
+                         const std::string& key,
+                         base::Value value) {
+  ListPrefUpdate updater(profile->GetPrefs(),
+                         crostini::prefs::kCrostiniContainers);
+  auto it = std::find_if(
+      updater->GetList().begin(), updater->GetList().end(),
+      [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
+  if (it != updater->GetList().end()) {
+    it->SetKey(key, std::move(value));
+  }
+}
+
 base::string16 GetTimeRemainingMessage(base::TimeTicks start, int percent) {
   // Only estimate once we've spent at least 3 seconds OR gotten 10% of the way
   // through.

chrome/browser/chromeos/crostini/crostini_util.h

@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/optional.h"
+#include "base/values.h"
 #include "storage/browser/file_system/file_system_url.h"
 #include "ui/base/resource/scale_factor.h"
 
@@ -213,6 +214,17 @@ base::string16 GetTimeRemainingMessage(base::TimeTicks start, int percent);
 std::vector<int64_t> GetTicksForDiskSize(int64_t min_size,
                                          int64_t available_space);
 
+// Returns a pref value stored for a specific container.
+const base::Value* GetContainerPrefValue(Profile* profile,
+                                         const ContainerId& container_id,
+                                         const std::string& key);
+
+// Sets a pref value for a specific container.
+void UpdateContainerPref(Profile* profile,
+                         const ContainerId& container_id,
+                         const std::string& key,
+                         base::Value value);
+
 }  // namespace crostini
 
 #endif  // CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_UTIL_H_