Make CrostiniRemover remove apps from non-default containers as well

Currently, if the user creates addition non-default containers and then
uninstalls crostini, and apps in those non-default containers will
continue to show up in the app launcher. This CL changes this behaviour
to remove all apps associated with the default VM instead.

Change-Id: Ieaa1ea6c434480cd5a2e4dfe7b05eb497ffc9917
Bug: 921163
Reviewed-on: https://chromium-review.googlesource.com/c/1429359Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Fergus Dall <sidereal@google.com>
Cr-Commit-Position: refs/heads/master@{#626912}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -2144,11 +2144,10 @@ void CrostiniManager::OnGetContainerSshKeys(
 }
 
 void CrostiniManager::RemoveCrostini(std::string vm_name,
-                                     std::string container_name,
                                      RemoveCrostiniCallback callback) {
   AddRemoveCrostiniCallback(std::move(callback));
   auto crostini_remover = base::MakeRefCounted<CrostiniRemover>(
-      profile_, std::move(vm_name), std::move(container_name),
+      profile_, std::move(vm_name),
       base::BindOnce(&CrostiniManager::OnRemoveCrostini,
                      weak_ptr_factory_.GetWeakPtr()));
   crostini_remover->RemoveCrostini();

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -480,7 +480,6 @@ class CrostiniManager : public KeyedService,
       const vm_tools::cicerone::LxdContainerStartingSignal& signal) override;
 
   void RemoveCrostini(std::string vm_name,
-                      std::string container_name,
                       RemoveCrostiniCallback callback);
 
   void SetVmState(std::string vm_name, VmState vm_state);

chrome/browser/chromeos/crostini/crostini_mime_types_service.cc

@@ -67,16 +67,12 @@ std::string CrostiniMimeTypesService::GetMimeType(
       ->GetString();
 }
 
-void CrostiniMimeTypesService::ClearMimeTypes(
-    const std::string& vm_name,
-    const std::string& container_name) {
+void CrostiniMimeTypesService::ClearMimeTypes(const std::string& vm_name) {
   DictionaryPrefUpdate update(prefs_, prefs::kCrostiniMimeTypes);
   base::DictionaryValue* mime_type_mappings = update.Get();
   std::vector<std::string> removed_ids;
   for (const auto& item : mime_type_mappings->DictItems()) {
-    if (item.second.FindKey(kAppVmNameKey)->GetString() == vm_name &&
-        item.second.FindKey(kAppContainerNameKey)->GetString() ==
-            container_name) {
+    if (item.second.FindKey(kAppVmNameKey)->GetString() == vm_name) {
       removed_ids.push_back(item.first);
     }
   }

chrome/browser/chromeos/crostini/crostini_mime_types_service.h

@@ -37,10 +37,9 @@ class CrostiniMimeTypesService : public KeyedService {
                           const std::string& vm_name,
                           const std::string& container_name) const;
 
-  // Remove all MIME type associations for the named container. Used in the
+  // Remove all MIME type associations for the named VM. Used in the
   // uninstall process.
-  void ClearMimeTypes(const std::string& vm_name,
-                      const std::string& container_name);
+  void ClearMimeTypes(const std::string& vm_name);
 
   // The existing list of MIME type mappings is replaced by
   // |mime_type_mappings|.

chrome/browser/chromeos/crostini/crostini_mime_types_service_unittest.cc

@@ -107,7 +107,7 @@ TEST_F(CrostiniMimeTypesServiceTest, MultipleContainers) {
 }
 
 // Test that ClearMimeTypes works, and only removes apps from the
-// specified container.
+// specified VM.
 TEST_F(CrostiniMimeTypesServiceTest, ClearMimeTypes) {
   service()->UpdateMimeTypes(
       CreateMimeTypesProto({"foo"}, {"foo/mime"}, "vm 1", "container 1"));
@@ -123,7 +123,7 @@ TEST_F(CrostiniMimeTypesServiceTest, ClearMimeTypes) {
   EXPECT_EQ("foobar/mime", service()->GetMimeType(base::FilePath("test.foobar"),
                                                   "vm 2", "container 1"));
 
-  service()->ClearMimeTypes("vm 2", "container 1");
+  service()->ClearMimeTypes("vm 2");
 
   EXPECT_EQ("foo/mime", service()->GetMimeType(base::FilePath("test.foo"),
                                                "vm 1", "container 1"));

chrome/browser/chromeos/crostini/crostini_registry_service.cc

@@ -659,9 +659,7 @@ void CrostiniRegistryService::MaybeRequestIcon(const std::string& app_id,
   RequestIcon(app_id, scale_factor);
 }
 
-void CrostiniRegistryService::ClearApplicationList(
-    const std::string& vm_name,
-    const std::string& container_name) {
+void CrostiniRegistryService::ClearApplicationList(const std::string& vm_name) {
   std::vector<std::string> removed_apps;
   // The DictionaryPrefUpdate should be destructed before calling the observer.
   {
@@ -671,11 +669,8 @@ void CrostiniRegistryService::ClearApplicationList(
     for (const auto& item : apps->DictItems()) {
       if (item.first == kCrostiniTerminalId)
         continue;
-      if (item.second.FindKey(kAppVmNameKey)->GetString() == vm_name &&
-          item.second.FindKey(kAppContainerNameKey)->GetString() ==
-              container_name) {
+      if (item.second.FindKey(kAppVmNameKey)->GetString() == vm_name)
         removed_apps.push_back(item.first);
-      }
     }
     for (const std::string& removed_app : removed_apps) {
       RemoveAppData(removed_app);

chrome/browser/chromeos/crostini/crostini_registry_service.h

@@ -166,9 +166,8 @@ class CrostiniRegistryService : public KeyedService {
   void MaybeRequestIcon(const std::string& app_id,
                         ui::ScaleFactor scale_factor);
 
-  // Remove all apps from the named container. Used in the uninstall process.
-  void ClearApplicationList(const std::string& vm_name,
-                            const std::string& container_name);
+  // Remove all apps from the named VM. Used in the uninstall process.
+  void ClearApplicationList(const std::string& vm_name);
 
   // The existing list of apps is replaced by |application_list|.
   void UpdateApplicationList(const vm_tools::apps::ApplicationList& app_list);

chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc

@@ -286,7 +286,7 @@ TEST_F(CrostiniRegistryServiceTest, MultipleContainers) {
 }
 
 // Test that ClearApplicationList works, and only removes apps from the
-// specified container.
+// specified VM.
 TEST_F(CrostiniRegistryServiceTest, ClearApplicationList) {
   service()->UpdateApplicationList(
       CrostiniTestHelper::BasicAppList("app", "vm 1", "container 1"));
@@ -309,7 +309,7 @@ TEST_F(CrostiniRegistryServiceTest, ClearApplicationList) {
               testing::UnorderedElementsAre(app_id_1, app_id_2, app_id_3,
                                             app_id_4, kCrostiniTerminalId));
 
-  service()->ClearApplicationList("vm 2", "container 1");
+  service()->ClearApplicationList("vm 2");
 
   EXPECT_THAT(
       service()->GetRegisteredAppIds(),

chrome/browser/chromeos/crostini/crostini_remover.cc

@@ -25,11 +25,9 @@ namespace crostini {
 CrostiniRemover::CrostiniRemover(
     Profile* profile,
     std::string vm_name,
-    std::string container_name,
     CrostiniManager::RemoveCrostiniCallback callback)
     : profile_(profile),
       vm_name_(std::move(vm_name)),
-      container_name_(std::move(container_name)),
       callback_(std::move(callback)) {}
 
 CrostiniRemover::~CrostiniRemover() = default;
@@ -70,10 +68,11 @@ void CrostiniRemover::StopVmFinished(CrostiniResult result) {
     std::move(callback_).Run(result);
     return;
   }
+
   CrostiniRegistryServiceFactory::GetForProfile(profile_)->ClearApplicationList(
-      vm_name_, container_name_);
+      vm_name_);
   CrostiniMimeTypesServiceFactory::GetForProfile(profile_)->ClearMimeTypes(
-      vm_name_, container_name_);
+      vm_name_);
   CrostiniManager::GetForProfile(profile_)->DestroyDiskImage(
       base::FilePath(vm_name_),
       vm_tools::concierge::StorageLocation::STORAGE_CRYPTOHOME_ROOT,

chrome/browser/chromeos/crostini/crostini_remover.h

@@ -13,7 +13,6 @@ class CrostiniRemover : public base::RefCountedThreadSafe<CrostiniRemover> {
  public:
   CrostiniRemover(Profile* profile,
                   std::string vm_name,
-                  std::string container_name,
                   CrostiniManager::RemoveCrostiniCallback callback);
 
   void RemoveCrostini();
@@ -31,7 +30,6 @@ class CrostiniRemover : public base::RefCountedThreadSafe<CrostiniRemover> {
 
   Profile* profile_;
   std::string vm_name_;
-  std::string container_name_;
   CrostiniManager::RemoveCrostiniCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(CrostiniRemover);

chrome/browser/ui/app_list/crostini/crostini_app_model_builder_unittest.cc

@@ -238,9 +238,7 @@ TEST_F(CrostiniAppModelBuilderTest, DisableCrostini) {
 
   // The uninstall flow removes all apps before setting the CrostiniEnabled pref
   // to false, so we need to do that explicitly too.
-  RegistryService()->ClearApplicationList(
-      crostini::kCrostiniDefaultVmName,
-      crostini::kCrostiniDefaultContainerName);
+  RegistryService()->ClearApplicationList(crostini::kCrostiniDefaultVmName);
   CrostiniTestHelper::DisableCrostini(profile());
   // Root folder is left. We rely on default handling of empty folder.
   EXPECT_EQ(1u, model_updater_->ItemCount());

chrome/browser/ui/views/crostini/crostini_installer_view.cc

@@ -203,7 +203,6 @@ bool CrostiniInstallerView::Cancel() {
               base::Unretained(
                   crostini::CrostiniManager::GetForProfile(profile_)),
               crostini::kCrostiniDefaultVmName,
-              crostini::kCrostiniDefaultContainerName,
               base::BindOnce(&CrostiniInstallerView::FinishCleanup,
                              weak_ptr_factory_.GetWeakPtr())));
       UpdateState(State::CLEANUP);

chrome/browser/ui/views/crostini/crostini_uninstaller_view.cc

@@ -87,7 +87,7 @@ bool CrostiniUninstallerView::Accept() {
 
   // Kick off the Crostini Remove sequence.
   crostini::CrostiniManager::GetForProfile(profile_)->RemoveCrostini(
-      crostini::kCrostiniDefaultVmName, crostini::kCrostiniDefaultContainerName,
+      crostini::kCrostiniDefaultVmName,
       base::BindOnce(&CrostiniUninstallerView::UninstallCrostiniFinished,
                      weak_ptr_factory_.GetWeakPtr()));