Add uninstall menu option to Crostini apps

Add uninstall menu option to Crostini apps that can be uninstalled from
the container.

BUG=chromium:822514
TEST=Apps from .desktop files associated with a package now have
'uninstall' on right-click menu; Apps from .desktop files I created do
not have 'uninstall'

R=timloh@chromium.org

Change-Id: If7bcd69c08a806b98a4200d2de7ca4f550c57c21
Reviewed-on: https://chromium-review.googlesource.com/c/1213519
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612850}

chrome/browser/chromeos/crostini/crostini_registry_service.cc

@@ -49,6 +49,7 @@ constexpr char kAppMimeTypesKey[] = "mime_types";
 constexpr char kAppNameKey[] = "name";
 constexpr char kAppNoDisplayKey[] = "no_display";
 constexpr char kAppScaledKey[] = "scaled";
+constexpr char kAppPackageIdKey[] = "package_id";
 constexpr char kAppStartupWMClassKey[] = "startup_wm_class";
 constexpr char kAppStartupNotifyKey[] = "startup_notify";
 constexpr char kAppInstallTimeKey[] = "install_time";
@@ -309,6 +310,26 @@ bool CrostiniRegistryService::Registration::NoDisplay() const {
   return false;
 }
 
+bool CrostiniRegistryService::Registration::CanUninstall() const {
+  if (pref_.is_none())
+    return false;
+  // We can uninstall if and only if there is a package that owns the
+  // application. If no package owns the application, we don't know how to
+  // uninstall the app.
+  //
+  // We don't check other things that might prevent us from uninstalling the
+  // app. In particular, we don't check if there are other packages which
+  // depend on the owning package. This should be rare for packages that have
+  // desktop files, and it's better to show an error message (which the user can
+  // then Google to learn more) than to just not have an uninstall option at
+  // all.
+  const base::Value* package_id =
+      pref_.FindKeyOfType(kAppPackageIdKey, base::Value::Type::STRING);
+  if (package_id)
+    return !package_id->GetString().empty();
+  return false;
+}
+
 base::Time CrostiniRegistryService::Registration::InstallTime() const {
   return GetTime(pref_, kAppInstallTimeKey);
 }
@@ -661,6 +682,7 @@ void CrostiniRegistryService::UpdateApplicationList(
                                base::Value(app.startup_wm_class()));
       pref_registration.SetKey(kAppStartupNotifyKey,
                                base::Value(app.startup_notify()));
+      pref_registration.SetKey(kAppPackageIdKey, base::Value(app.package_id()));
 
       base::Value* old_app = apps->FindKey(app_id);
       if (old_app && EqualsExcludingTimestamps(pref_registration, *old_app))

chrome/browser/chromeos/crostini/crostini_registry_service.h

@@ -90,6 +90,7 @@ class CrostiniRegistryService : public KeyedService {
 
     // Whether this app should scale up when displayed.
     bool IsScaled() const;
+    bool CanUninstall() const;
 
     // Whether this app is the default terminal app.
     bool is_terminal_app() const { return is_terminal_app_; }

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

@@ -6,7 +6,10 @@
 
 #include "ash/public/cpp/app_menu_constants.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
+#include "chrome/browser/chromeos/crostini/crostini_registry_service.h"
+#include "chrome/browser/chromeos/crostini/crostini_registry_service_factory.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/ui_base_features.h"
 
@@ -18,17 +21,39 @@ CrostiniAppContextMenu::CrostiniAppContextMenu(
 
 CrostiniAppContextMenu::~CrostiniAppContextMenu() = default;
 
-// TODO(timloh): Add support for "App Info", "Uninstall", and possibly actions
-// defined in .desktop files.
+bool CrostiniAppContextMenu::IsUninstallable() const {
+  if (!crostini::IsCrostiniEnabled(profile())) {
+    return false;
+  }
+  if (app_id() == crostini::kCrostiniTerminalId) {
+    return true;  // Crostini can always be uninstalled if enabled.
+  }
+
+  if (!base::FeatureList::IsEnabled(features::kCrostiniAppUninstallGui)) {
+    return false;
+  }
+
+  crostini::CrostiniRegistryService* registry_service =
+      crostini::CrostiniRegistryServiceFactory::GetForProfile(profile());
+  base::Optional<crostini::CrostiniRegistryService::Registration> registration =
+      registry_service->GetRegistration(app_id());
+  if (registration) {
+    return registration->CanUninstall();
+  }
+  return false;
+}
+
+// TODO(timloh): Add support for "App Info" and possibly actions defined in
+// .desktop files.
 void CrostiniAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   app_list::AppContextMenu::BuildMenu(menu_model);
 
-  if (app_id() == crostini::kCrostiniTerminalId) {
-    if (!features::IsTouchableAppContextMenuEnabled())
-      menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
-
+  if (IsUninstallable()) {
     AddContextMenuOption(menu_model, ash::UNINSTALL,
                          IDS_APP_LIST_UNINSTALL_ITEM);
+  }
+
+  if (app_id() == crostini::kCrostiniTerminalId) {
     AddContextMenuOption(menu_model, ash::STOP_APP,
                          IDS_CROSTINI_SHUT_DOWN_LINUX_MENU_ITEM);
   }
@@ -36,9 +61,7 @@ void CrostiniAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
 
 bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const {
   if (command_id == ash::UNINSTALL) {
-    if (app_id() == crostini::kCrostiniTerminalId) {
-      return crostini::IsCrostiniEnabled(profile());
-    }
+    return IsUninstallable();
   } else if (command_id == ash::STOP_APP) {
     if (app_id() == crostini::kCrostiniTerminalId) {
       return crostini::IsCrostiniRunning(profile());

chrome/browser/ui/app_list/crostini/crostini_app_context_menu.h

@@ -27,6 +27,7 @@ class CrostiniAppContextMenu : public app_list::AppContextMenu {
   void BuildMenu(ui::SimpleMenuModel* menu_model) override;
 
  private:
+  bool IsUninstallable() const;
   DISALLOW_COPY_AND_ASSIGN(CrostiniAppContextMenu);
 };
 

chrome/common/chrome_features.cc

@@ -184,6 +184,12 @@ const base::Feature kClipboardContentSetting{"ClipboardContentSetting",
 // Enable project Crostini, Linux VMs on Chrome OS.
 const base::Feature kCrostini{"Crostini", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables an uninstall option in the right-click menu of Crostini (Linux)
+// applications.
+// TODO(iby): Remove once remaining bugs fixed.
+const base::Feature kCrostiniAppUninstallGui{"CrostiniAppUninstallGui",
+                                             base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Whether the UsageTimeLimit policy should be applied to the user.
 const base::Feature kUsageTimeLimitPolicy{"UsageTimeLimitPolicy",
                                           base::FEATURE_ENABLED_BY_DEFAULT};

chrome/common/chrome_features.h

@@ -119,6 +119,8 @@ COMPONENT_EXPORT(CHROME_FEATURES) extern const base::Feature kContentFullscreen;
 #if defined(OS_CHROMEOS)
 COMPONENT_EXPORT(CHROME_FEATURES) extern const base::Feature kCrostini;
 COMPONENT_EXPORT(CHROME_FEATURES)
+extern const base::Feature kCrostiniAppUninstallGui;
+COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kUsageTimeLimitPolicy;
 #endif