Only add new crostini shared path to prefs if it does not already exist.

Bug: 878324
Change-Id: I2d738bb3f980d200b946f0b6cf7522b830f4d5ac
Reviewed-on: https://chromium-review.googlesource.com/c/1334669Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607908}

chrome/browser/chromeos/crostini/crostini_share_path.cc

@@ -292,17 +292,21 @@ void CrostiniSharePath::RegisterPersistedPath(const base::FilePath& path) {
   PrefService* pref_service = profile_->GetPrefs();
   ListPrefUpdate update(pref_service, crostini::prefs::kCrostiniSharedPaths);
   base::ListValue* shared_paths = update.Get();
-  shared_paths->Append(std::make_unique<base::Value>(path.value()));
-  // Remove any existing paths that are children of new path.
+  // Check if path exists, remove paths that are children of new path.
+  bool exists = false;
   auto it = shared_paths->begin();
   while (it != shared_paths->end()) {
     base::FilePath existing(it->GetString());
-    if (path.IsParent(existing)) {
+    if (path == existing) {
+      exists = true;
+    } else if (path.IsParent(existing)) {
       it = shared_paths->Erase(it, nullptr);
       continue;
     }
     ++it;
   }
+  if (!exists)
+    shared_paths->Append(std::make_unique<base::Value>(path.value()));
 }
 
 }  // namespace crostini

chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc

@@ -433,6 +433,13 @@ TEST_F(CrostiniSharePathTest, RegisterPersistedPaths) {
   prefs->GetString(0, &path);
   EXPECT_EQ(path, "/a/a/a");
 
+  // Adding the same path again should not cause any changes.
+  crostini_share_path()->RegisterPersistedPath(base::FilePath("/a/a/a"));
+  EXPECT_EQ(prefs->GetSize(), 1U);
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/a/a/a");
+
+  // Add more paths.
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/a/a/b"));
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/a/a/c"));
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/a/b/a"));
@@ -450,6 +457,7 @@ TEST_F(CrostiniSharePathTest, RegisterPersistedPaths) {
   prefs->GetString(4, &path);
   EXPECT_EQ(path, "/b/a/a");
 
+  // Adding /a/a should remove /a/a/b, /a/a/c.
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/a/a"));
   prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
   EXPECT_EQ(prefs->GetSize(), 3U);
@@ -460,6 +468,7 @@ TEST_F(CrostiniSharePathTest, RegisterPersistedPaths) {
   prefs->GetString(2, &path);
   EXPECT_EQ(path, "/a/a");
 
+  // Adding /a should remove /a/a, /a/b/a.
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/a"));
   prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
   EXPECT_EQ(prefs->GetSize(), 2U);
@@ -468,10 +477,12 @@ TEST_F(CrostiniSharePathTest, RegisterPersistedPaths) {
   prefs->GetString(1, &path);
   EXPECT_EQ(path, "/a");
 
+  // Adding / should remove all others.
   crostini_share_path()->RegisterPersistedPath(base::FilePath("/"));
   prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
   EXPECT_EQ(prefs->GetSize(), 1U);
   prefs->GetString(0, &path);
   EXPECT_EQ(path, "/");
 }
+
 }  // namespace crostini