Collapse crostini shared paths when a parent is shared.

If 2 child dirs such as /a/a, and /a/b, are shared, then parent
/a is shared, we no longer register both children as being shared
and we only show the parent in settings page as being shared.
If the parent is subsequently unshared, then children will also be
unshared.

Bug: 878324
Change-Id: I1c85d75ad934d48cd353299943f7de893ca62017
Reviewed-on: https://chromium-review.googlesource.com/c/1306945
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604465}
diff --git a/chrome/browser/chromeos/crostini/crostini_share_path.cc b/chrome/browser/chromeos/crostini/crostini_share_path.cc
index d3e06a6..aa6475c 100644
--- a/chrome/browser/chromeos/crostini/crostini_share_path.cc
+++ b/chrome/browser/chromeos/crostini/crostini_share_path.cc
@@ -143,10 +143,7 @@
 
   // Even if VM is not running, save to prefs now.
   if (persist) {
-    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()));
+    crostini::RegisterPersistedPath(profile, path);
   }
 
   request.mutable_shared_path()->set_path(relative_path.value());
@@ -273,4 +270,21 @@
              false /* persist */, std::move(callback));
 }
 
+void RegisterPersistedPath(Profile* profile, 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.
+  auto it = shared_paths->begin();
+  while (it != shared_paths->end()) {
+    base::FilePath existing(it->GetString());
+    if (path.IsParent(existing)) {
+      it = shared_paths->Erase(it, nullptr);
+      continue;
+    }
+    ++it;
+  }
+}
+
 }  // namespace crostini
diff --git a/chrome/browser/chromeos/crostini/crostini_share_path.h b/chrome/browser/chromeos/crostini/crostini_share_path.h
index 7e5a6cb..20af7d6 100644
--- a/chrome/browser/chromeos/crostini/crostini_share_path.h
+++ b/chrome/browser/chromeos/crostini/crostini_share_path.h
@@ -15,25 +15,25 @@
 
 namespace crostini {
 
-// Share specified absolute path with vm. If |persist| is set, the path will be
-// automatically shared at container startup. Callback receives success bool and
-// failure reason string.
+// Share specified absolute |path| with vm. If |persist| is set, the path will
+// be automatically shared at container startup. Callback receives success bool
+// and failure reason string.
 void SharePath(Profile* profile,
                std::string vm_name,
                const base::FilePath& path,
                bool persist,
                base::OnceCallback<void(bool, std::string)> callback);
 
-// Share specified absolute paths with vm. If persist is set, the paths will be
-// automatically shared at container startup. Callback receives success bool and
-// failure reason string of the first error.
+// Share specified absolute |paths| with vm. If |persist| is set, the paths will
+// be automatically shared at container startup. Callback receives success bool
+// and failure reason string of the first error.
 void SharePaths(Profile* profile,
                 std::string vm_name,
                 std::vector<base::FilePath> paths,
                 bool persist,
                 base::OnceCallback<void(bool, std::string)> callback);
 
-// Unshare specified path with vm.
+// Unshare specified |path| with vm.
 // Callback receives success bool and failure reason string.
 void UnsharePath(Profile* profile,
                  std::string vm_name,
@@ -48,6 +48,9 @@
 void SharePersistedPaths(Profile* profile,
                          base::OnceCallback<void(bool, std::string)> callback);
 
+// Save |path| into prefs.
+void RegisterPersistedPath(Profile* profile, const base::FilePath& path);
+
 }  // namespace crostini
 
 #endif  // CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_SHARE_PATH_H_
diff --git a/chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc b/chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc
index ab7f17d4..01bd04e 100644
--- a/chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc
+++ b/chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc
@@ -364,4 +364,54 @@
   run_loop()->Run();
 }
 
+TEST_F(CrostiniSharePathTest, RegisterPersistedPaths) {
+  RegisterPersistedPath(profile(), base::FilePath("/a/a/a"));
+  const base::ListValue* prefs =
+      profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
+  EXPECT_EQ(prefs->GetSize(), 1U);
+  std::string path;
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/a/a/a");
+
+  RegisterPersistedPath(profile(), base::FilePath("/a/a/b"));
+  RegisterPersistedPath(profile(), base::FilePath("/a/a/c"));
+  RegisterPersistedPath(profile(), base::FilePath("/a/b/a"));
+  RegisterPersistedPath(profile(), base::FilePath("/b/a/a"));
+  prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
+  EXPECT_EQ(prefs->GetSize(), 5U);
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/a/a/a");
+  prefs->GetString(1, &path);
+  EXPECT_EQ(path, "/a/a/b");
+  prefs->GetString(2, &path);
+  EXPECT_EQ(path, "/a/a/c");
+  prefs->GetString(3, &path);
+  EXPECT_EQ(path, "/a/b/a");
+  prefs->GetString(4, &path);
+  EXPECT_EQ(path, "/b/a/a");
+
+  RegisterPersistedPath(profile(), base::FilePath("/a/a"));
+  prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
+  EXPECT_EQ(prefs->GetSize(), 3U);
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/a/b/a");
+  prefs->GetString(1, &path);
+  EXPECT_EQ(path, "/b/a/a");
+  prefs->GetString(2, &path);
+  EXPECT_EQ(path, "/a/a");
+
+  RegisterPersistedPath(profile(), base::FilePath("/a"));
+  prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
+  EXPECT_EQ(prefs->GetSize(), 2U);
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/b/a/a");
+  prefs->GetString(1, &path);
+  EXPECT_EQ(path, "/a");
+
+  RegisterPersistedPath(profile(), base::FilePath("/"));
+  prefs = profile()->GetPrefs()->GetList(prefs::kCrostiniSharedPaths);
+  EXPECT_EQ(prefs->GetSize(), 1U);
+  prefs->GetString(0, &path);
+  EXPECT_EQ(path, "/");
+}
 }  // namespace crostini
diff --git a/ui/file_manager/file_manager/foreground/js/crostini.js b/ui/file_manager/file_manager/foreground/js/crostini.js
index 13e746b..2795e08a 100644
--- a/ui/file_manager/file_manager/foreground/js/crostini.js
+++ b/ui/file_manager/file_manager/foreground/js/crostini.js
@@ -59,6 +59,11 @@
     paths = {};
     Crostini.SHARED_PATHS_[info.rootType] = paths;
   }
+  // Remove any existing paths that are children of the new path.
+  for (let path in paths) {
+    if (path.startsWith(entry.fullPath))
+      delete paths[path];
+  }
   paths[entry.fullPath] = true;
 
   // Record UMA.
@@ -90,7 +95,7 @@
  * @param {!Entry} entry
  * @param {!VolumeManager} volumeManager
  * @return {boolean} True if path is shared either by a direct
- * share or from one of its ancestor directories.
+ *   share or from one of its ancestor directories.
  */
 Crostini.isPathShared = function(entry, volumeManager) {
   const root = volumeManager.getLocationInfo(entry).rootType;
@@ -99,12 +104,12 @@
     return false;
   // Check path and all ancestor directories.
   let path = entry.fullPath;
-  while (path != '') {
+  while (path.length > 1) {
     if (paths[path])
       return true;
     path = path.substring(0, path.lastIndexOf('/'));
   }
-  return false;
+  return !!paths['/'];
 };
 
 /**
diff --git a/ui/file_manager/file_manager/foreground/js/crostini_unittest.js b/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
index 6235e43..dc34ad8 100644
--- a/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
+++ b/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
@@ -23,24 +23,59 @@
 function testIsPathShared() {
   const mockFileSystem = new MockFileSystem('volumeId');
   const root = new MockDirectoryEntry(mockFileSystem, '/');
-  const foo1 = new MockDirectoryEntry(mockFileSystem, '/foo1');
-  const foobar1 = new MockDirectoryEntry(mockFileSystem, '/foo1/bar1');
-  const foo2 = new MockDirectoryEntry(mockFileSystem, '/foo2');
-  const foobar2 = new MockDirectoryEntry(mockFileSystem, '/foo2/bar2');
+  const a = new MockDirectoryEntry(mockFileSystem, '/a');
+  const aa = new MockDirectoryEntry(mockFileSystem, '/a/a');
+  const ab = new MockDirectoryEntry(mockFileSystem, '/a/b');
+  const b = new MockDirectoryEntry(mockFileSystem, '/b');
+  const bb = new MockDirectoryEntry(mockFileSystem, '/b/b');
 
-  assertFalse(Crostini.isPathShared(foo1, volumeManager));
+  assertFalse(Crostini.isPathShared(a, volumeManager));
 
-  Crostini.registerSharedPath(foo1, volumeManager);
+  Crostini.registerSharedPath(a, volumeManager);
   assertFalse(Crostini.isPathShared(root, volumeManager));
-  assertTrue(Crostini.isPathShared(foo1, volumeManager));
-  assertTrue(Crostini.isPathShared(foobar1, volumeManager));
+  assertTrue(Crostini.isPathShared(a, volumeManager));
+  assertTrue(Crostini.isPathShared(aa, volumeManager));
 
-  Crostini.registerSharedPath(foobar2, volumeManager);
-  assertFalse(Crostini.isPathShared(foo2, volumeManager));
-  assertTrue(Crostini.isPathShared(foobar2, volumeManager));
+  Crostini.registerSharedPath(bb, volumeManager);
+  assertFalse(Crostini.isPathShared(b, volumeManager));
+  assertTrue(Crostini.isPathShared(bb, volumeManager));
 
-  Crostini.unregisterSharedPath(foobar2, volumeManager);
-  assertFalse(Crostini.isPathShared(foobar2, volumeManager));
+  Crostini.unregisterSharedPath(bb, volumeManager);
+  assertFalse(Crostini.isPathShared(bb, volumeManager));
+
+  // Test collapsing.  Setup with /a/a, /a/b, /b
+  Crostini.unregisterSharedPath(a, volumeManager);
+  Crostini.registerSharedPath(aa, volumeManager);
+  Crostini.registerSharedPath(ab, volumeManager);
+  Crostini.registerSharedPath(b, volumeManager);
+  assertFalse(Crostini.isPathShared(a, volumeManager));
+  assertTrue(Crostini.isPathShared(aa, volumeManager));
+  assertTrue(Crostini.isPathShared(ab, volumeManager));
+  assertTrue(Crostini.isPathShared(b, volumeManager));
+  // Add /a, collapses /a/a, /a/b
+  Crostini.registerSharedPath(a, volumeManager);
+  assertTrue(Crostini.isPathShared(a, volumeManager));
+  assertTrue(Crostini.isPathShared(aa, volumeManager));
+  assertTrue(Crostini.isPathShared(ab, volumeManager));
+  assertTrue(Crostini.isPathShared(b, volumeManager));
+  // Unregister /a, /a/a and /a/b should be lost.
+  Crostini.unregisterSharedPath(a, volumeManager);
+  assertFalse(Crostini.isPathShared(a, volumeManager));
+  assertFalse(Crostini.isPathShared(aa, volumeManager));
+  assertFalse(Crostini.isPathShared(ab, volumeManager));
+  assertTrue(Crostini.isPathShared(b, volumeManager));
+  // Register root, collapses all.
+  Crostini.registerSharedPath(root, volumeManager);
+  assertTrue(Crostini.isPathShared(a, volumeManager));
+  assertTrue(Crostini.isPathShared(aa, volumeManager));
+  assertTrue(Crostini.isPathShared(ab, volumeManager));
+  assertTrue(Crostini.isPathShared(b, volumeManager));
+  // Unregister root, all should be lost.
+  Crostini.unregisterSharedPath(root, volumeManager);
+  assertFalse(Crostini.isPathShared(a, volumeManager));
+  assertFalse(Crostini.isPathShared(aa, volumeManager));
+  assertFalse(Crostini.isPathShared(ab, volumeManager));
+  assertFalse(Crostini.isPathShared(b, volumeManager));
 }