Reland "Use AppService to uninstall apps on Chrome OS."

The browser test AppListClientImplBrowserTest.UninstallApp is flaky,
because AppService is mojom based, which is slow when loading icon,
compared with sync Extensions. Update the test case to flush mojo call.

BUG=1009248

Change-Id: I7e5702392ec9369991ffb357c5dcb402dd6cb0f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885134Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710615}

chrome/browser/apps/app_service/uninstall_dialog.cc

@@ -6,6 +6,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/native_window_tracker.h"
 #include "chrome/services/app_service/public/cpp/icon_loader.h"
 #include "extensions/browser/uninstall_reason.h"
 
@@ -36,6 +37,9 @@ UninstallDialog::UninstallDialog(Profile* profile,
       app_name_(app_name),
       parent_window_(parent_window),
       uninstall_callback_(std::move(uninstall_callback)) {
+  if (parent_window)
+    parent_window_tracker_ = NativeWindowTracker::Create(parent_window);
+
   int32_t size_hint_in_dip;
   switch (app_type) {
     case apps::mojom::AppType::kCrostini:
@@ -86,6 +90,12 @@ void UninstallDialog::OnDialogClosed(bool uninstall,
 void UninstallDialog::OnLoadIcon(apps::mojom::IconValuePtr icon_value) {
   if (icon_value->icon_compression !=
       apps::mojom::IconCompression::kUncompressed) {
+    OnDialogClosed(false, false, false);
+    return;
+  }
+
+  if (parent_window_ && parent_window_tracker_->WasNativeWindowClosed()) {
+    OnDialogClosed(false, false, false);
     return;
   }
 

chrome/browser/apps/app_service/uninstall_dialog.h

@@ -11,6 +11,7 @@
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
 #include "ui/gfx/native_widget_types.h"
 
+class NativeWindowTracker;
 class Profile;
 
 namespace gfx {
@@ -103,6 +104,9 @@ class UninstallDialog {
   gfx::NativeWindow parent_window_;
   UninstallCallback uninstall_callback_;
 
+  // Tracks whether |parent_window_| got destroyed.
+  std::unique_ptr<NativeWindowTracker> parent_window_tracker_;
+
   base::WeakPtrFactory<UninstallDialog> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(UninstallDialog);

chrome/browser/ui/app_list/app_list_client_impl_browsertest.cc

@@ -20,6 +20,8 @@
 #include "build/branding_buildflags.h"
 #include "build/build_config.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/apps/launch_service/launch_service.h"
 #include "chrome/browser/apps/platform_apps/app_browsertest_util.h"
 #include "chrome/browser/browser_process.h"
@@ -114,6 +116,12 @@ IN_PROC_BROWSER_TEST_F(AppListClientImplBrowserTest, UninstallApp) {
   // Open the uninstall dialog.
   base::RunLoop run_loop;
   client->UninstallApp(profile(), app->id());
+
+  apps::AppServiceProxy* app_service_proxy_ =
+      apps::AppServiceProxyFactory::GetForProfile(profile());
+  DCHECK(app_service_proxy_);
+  app_service_proxy_->FlushMojoCallsForTesting();
+
   run_loop.RunUntilIdle();
   EXPECT_FALSE(wm::GetTransientChildren(client->GetAppListWindow()).empty());
 

chrome/browser/ui/app_list/app_list_controller_delegate.cc

@@ -11,6 +11,8 @@
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/install_tracker_factory.h"
 #include "chrome/browser/extensions/launch_util.h"
@@ -127,10 +129,10 @@ void AppListControllerDelegate::DoShowAppInfoFlow(
 
 void AppListControllerDelegate::UninstallApp(Profile* profile,
                                              const std::string& app_id) {
-  // ExtensionUninstall deletes itself when done or aborted.
-  ExtensionUninstaller* uninstaller =
-      new ExtensionUninstaller(profile, app_id, GetAppListWindow());
-  uninstaller->Run();
+  apps::AppServiceProxy* proxy =
+      apps::AppServiceProxyFactory::GetForProfile(profile);
+  DCHECK(proxy);
+  proxy->Uninstall(app_id, GetAppListWindow());
 }
 
 bool AppListControllerDelegate::IsAppFromWebStore(Profile* profile,

chrome/browser/ui/app_list/arc/arc_app_context_menu.cc

@@ -9,6 +9,8 @@
 #include "ash/public/cpp/tablet_mode.h"
 #include "base/bind.h"
 #include "base/feature_list.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcuts_menu_builder.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_context_menu_delegate.h"
@@ -89,7 +91,11 @@ void ArcAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
   if (command_id == ash::LAUNCH_NEW) {
     delegate()->ExecuteLaunchCommand(event_flags);
   } else if (command_id == ash::UNINSTALL) {
-    arc::ShowArcAppUninstallDialog(profile(), controller(), app_id());
+    apps::AppServiceProxy* proxy =
+        apps::AppServiceProxyFactory::GetForProfile(profile());
+    DCHECK(proxy);
+    proxy->Uninstall(app_id(),
+                     controller() ? controller()->GetAppListWindow() : nullptr);
   } else if (command_id == ash::SHOW_APP_INFO) {
     ShowPackageInfo();
   } else if (command_id >= ash::LAUNCH_APP_SHORTCUT_FIRST &&

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

@@ -6,6 +6,8 @@
 
 #include "ash/public/cpp/app_menu_constants.h"
 #include "base/bind_helpers.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "chrome/grit/generated_resources.h"
@@ -31,11 +33,14 @@ bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const {
 
 void CrostiniAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
   switch (command_id) {
-    case ash::UNINSTALL:
+    case ash::UNINSTALL: {
       DCHECK_NE(app_id(), crostini::kCrostiniTerminalId);
-      crostini::ShowCrostiniAppUninstallerView(profile(), app_id());
+      apps::AppServiceProxy* proxy =
+          apps::AppServiceProxyFactory::GetForProfile(profile());
+      DCHECK(proxy);
+      proxy->Uninstall(app_id(), nullptr /* parent_window */);
       return;
-
+    }
     case ash::STOP_APP:
       if (app_id() == crostini::kCrostiniTerminalId) {
         crostini::CrostiniManager::GetForProfile(profile())->StopVm(

chrome/browser/ui/ash/launcher/arc_launcher_context_menu.cc

@@ -67,16 +67,10 @@ void ArcLauncherContextMenu::ExecuteCommand(int command_id, int event_flags) {
     return;
   }
   if (command_id == ash::UNINSTALL) {
-    if (base::FeatureList::IsEnabled(features::kAppServiceShelf)) {
-      apps::AppServiceProxy* proxy =
-          apps::AppServiceProxyFactory::GetForProfile(controller()->profile());
-      DCHECK(proxy);
-      proxy->Uninstall(item().id.app_id, nullptr /* parent_window */);
-      return;
-    }
-
-    arc::ShowArcAppUninstallDialog(controller()->profile(),
-                                   nullptr /* controller */, item().id.app_id);
+    apps::AppServiceProxy* proxy =
+        apps::AppServiceProxyFactory::GetForProfile(controller()->profile());
+    DCHECK(proxy);
+    proxy->Uninstall(item().id.app_id, nullptr /* parent_window */);
     return;
   }
 

chrome/browser/ui/ash/launcher/crostini_shelf_context_menu.cc

@@ -10,6 +10,8 @@
 #include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "base/bind_helpers.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.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"
@@ -68,11 +70,14 @@ bool CrostiniShelfContextMenu::IsCommandIdEnabled(int command_id) const {
 
 void CrostiniShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
   switch (command_id) {
-    case ash::UNINSTALL:
+    case ash::UNINSTALL: {
       DCHECK_NE(item().id.app_id, crostini::kCrostiniTerminalId);
-      crostini::ShowCrostiniAppUninstallerView(controller()->profile(),
-                                               item().id.app_id);
+      apps::AppServiceProxy* proxy =
+          apps::AppServiceProxyFactory::GetForProfile(controller()->profile());
+      DCHECK(proxy);
+      proxy->Uninstall(item().id.app_id, nullptr /* parent_window */);
       return;
+    }
     case ash::STOP_APP:
       if (item().id.app_id == crostini::kCrostiniTerminalId) {
         crostini::CrostiniManager::GetForProfile(controller()->profile())