Commit b4234609 authored by Meredith Lane's avatar Meredith Lane Committed by Commit Bot

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

This reverts commit 07e39f96.

Reason for revert: Causing flakiness on AppListClientImplBrowserTest.UninstallApp.

Flakiness dashboard: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests&tests=AppListClientImplBrowserTest.UninstallApp

Original change's description:
> Reland "Use AppService to uninstall apps on Chrome OS."
> 
> This CL is used to reland the CL:1864496 and fix the browser_tests.
> 
> CL:1864496 is reverted because it breaks the browser_tests:
> AppListClientImplBrowserTest.UninstallApp
> 
> The reason is a NativeWindowTracker should be created to track whether
> |parent_window_| got destroyed. If destroyed, or load icon failed,
> call OnDialogClosed to notify AppServiceProxy to remove the
> UninstallDialog from set, and destroy UninstallDialog.
> 
> BUG=1009248
> 
> Change-Id: I1a2fe6f087a5cc2f723ac0256510a385555e9347
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875865
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#709487}

TBR=xiyuan@chromium.org,dominickn@chromium.org,nancylingwang@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1009248
Change-Id: Ib3b60ce1f11e9b0dcd5f5bc3870702eaf96c9896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1882195Reviewed-by: default avatarMeredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709783}
parent 3ed072e8
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "chrome/browser/profiles/profile.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 "chrome/services/app_service/public/cpp/icon_loader.h"
#include "extensions/browser/uninstall_reason.h" #include "extensions/browser/uninstall_reason.h"
...@@ -37,9 +36,6 @@ UninstallDialog::UninstallDialog(Profile* profile, ...@@ -37,9 +36,6 @@ UninstallDialog::UninstallDialog(Profile* profile,
app_name_(app_name), app_name_(app_name),
parent_window_(parent_window), parent_window_(parent_window),
uninstall_callback_(std::move(uninstall_callback)) { uninstall_callback_(std::move(uninstall_callback)) {
if (parent_window)
parent_window_tracker_ = NativeWindowTracker::Create(parent_window);
int32_t size_hint_in_dip; int32_t size_hint_in_dip;
switch (app_type) { switch (app_type) {
case apps::mojom::AppType::kCrostini: case apps::mojom::AppType::kCrostini:
...@@ -90,12 +86,6 @@ void UninstallDialog::OnDialogClosed(bool uninstall, ...@@ -90,12 +86,6 @@ void UninstallDialog::OnDialogClosed(bool uninstall,
void UninstallDialog::OnLoadIcon(apps::mojom::IconValuePtr icon_value) { void UninstallDialog::OnLoadIcon(apps::mojom::IconValuePtr icon_value) {
if (icon_value->icon_compression != if (icon_value->icon_compression !=
apps::mojom::IconCompression::kUncompressed) { apps::mojom::IconCompression::kUncompressed) {
OnDialogClosed(false, false, false);
return;
}
if (parent_window_ && parent_window_tracker_->WasNativeWindowClosed()) {
OnDialogClosed(false, false, false);
return; return;
} }
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "chrome/services/app_service/public/mojom/types.mojom.h" #include "chrome/services/app_service/public/mojom/types.mojom.h"
#include "ui/gfx/native_widget_types.h" #include "ui/gfx/native_widget_types.h"
class NativeWindowTracker;
class Profile; class Profile;
namespace gfx { namespace gfx {
...@@ -104,9 +103,6 @@ class UninstallDialog { ...@@ -104,9 +103,6 @@ class UninstallDialog {
gfx::NativeWindow parent_window_; gfx::NativeWindow parent_window_;
UninstallCallback uninstall_callback_; UninstallCallback uninstall_callback_;
// Tracks whether |parent_window_| got destroyed.
std::unique_ptr<NativeWindowTracker> parent_window_tracker_;
base::WeakPtrFactory<UninstallDialog> weak_ptr_factory_{this}; base::WeakPtrFactory<UninstallDialog> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(UninstallDialog); DISALLOW_COPY_AND_ASSIGN(UninstallDialog);
......
...@@ -11,8 +11,6 @@ ...@@ -11,8 +11,6 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "build/build_config.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/extension_util.h"
#include "chrome/browser/extensions/install_tracker_factory.h" #include "chrome/browser/extensions/install_tracker_factory.h"
#include "chrome/browser/extensions/launch_util.h" #include "chrome/browser/extensions/launch_util.h"
...@@ -129,10 +127,10 @@ void AppListControllerDelegate::DoShowAppInfoFlow( ...@@ -129,10 +127,10 @@ void AppListControllerDelegate::DoShowAppInfoFlow(
void AppListControllerDelegate::UninstallApp(Profile* profile, void AppListControllerDelegate::UninstallApp(Profile* profile,
const std::string& app_id) { const std::string& app_id) {
apps::AppServiceProxy* proxy = // ExtensionUninstall deletes itself when done or aborted.
apps::AppServiceProxyFactory::GetForProfile(profile); ExtensionUninstaller* uninstaller =
DCHECK(proxy); new ExtensionUninstaller(profile, app_id, GetAppListWindow());
proxy->Uninstall(app_id, GetAppListWindow()); uninstaller->Run();
} }
bool AppListControllerDelegate::IsAppFromWebStore(Profile* profile, bool AppListControllerDelegate::IsAppFromWebStore(Profile* profile,
......
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
#include "ash/public/cpp/tablet_mode.h" #include "ash/public/cpp/tablet_mode.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/feature_list.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/chromeos/arc/app_shortcuts/arc_app_shortcuts_menu_builder.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/app_context_menu_delegate.h" #include "chrome/browser/ui/app_list/app_context_menu_delegate.h"
...@@ -91,11 +89,7 @@ void ArcAppContextMenu::ExecuteCommand(int command_id, int event_flags) { ...@@ -91,11 +89,7 @@ void ArcAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
if (command_id == ash::LAUNCH_NEW) { if (command_id == ash::LAUNCH_NEW) {
delegate()->ExecuteLaunchCommand(event_flags); delegate()->ExecuteLaunchCommand(event_flags);
} else if (command_id == ash::UNINSTALL) { } else if (command_id == ash::UNINSTALL) {
apps::AppServiceProxy* proxy = arc::ShowArcAppUninstallDialog(profile(), controller(), app_id());
apps::AppServiceProxyFactory::GetForProfile(profile());
DCHECK(proxy);
proxy->Uninstall(app_id(),
controller() ? controller()->GetAppListWindow() : nullptr);
} else if (command_id == ash::SHOW_APP_INFO) { } else if (command_id == ash::SHOW_APP_INFO) {
ShowPackageInfo(); ShowPackageInfo();
} else if (command_id >= ash::LAUNCH_APP_SHORTCUT_FIRST && } else if (command_id >= ash::LAUNCH_APP_SHORTCUT_FIRST &&
......
...@@ -6,8 +6,6 @@ ...@@ -6,8 +6,6 @@
#include "ash/public/cpp/app_menu_constants.h" #include "ash/public/cpp/app_menu_constants.h"
#include "base/bind_helpers.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_manager.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h" #include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -33,14 +31,11 @@ bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const { ...@@ -33,14 +31,11 @@ bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const {
void CrostiniAppContextMenu::ExecuteCommand(int command_id, int event_flags) { void CrostiniAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
switch (command_id) { switch (command_id) {
case ash::UNINSTALL: { case ash::UNINSTALL:
DCHECK_NE(app_id(), crostini::kCrostiniTerminalId); DCHECK_NE(app_id(), crostini::kCrostiniTerminalId);
apps::AppServiceProxy* proxy = crostini::ShowCrostiniAppUninstallerView(profile(), app_id());
apps::AppServiceProxyFactory::GetForProfile(profile());
DCHECK(proxy);
proxy->Uninstall(app_id(), nullptr /* parent_window */);
return; return;
}
case ash::STOP_APP: case ash::STOP_APP:
if (app_id() == crostini::kCrostiniTerminalId) { if (app_id() == crostini::kCrostiniTerminalId) {
crostini::CrostiniManager::GetForProfile(profile())->StopVm( crostini::CrostiniManager::GetForProfile(profile())->StopVm(
......
...@@ -67,10 +67,16 @@ void ArcLauncherContextMenu::ExecuteCommand(int command_id, int event_flags) { ...@@ -67,10 +67,16 @@ void ArcLauncherContextMenu::ExecuteCommand(int command_id, int event_flags) {
return; return;
} }
if (command_id == ash::UNINSTALL) { if (command_id == ash::UNINSTALL) {
apps::AppServiceProxy* proxy = if (base::FeatureList::IsEnabled(features::kAppServiceShelf)) {
apps::AppServiceProxyFactory::GetForProfile(controller()->profile()); apps::AppServiceProxy* proxy =
DCHECK(proxy); apps::AppServiceProxyFactory::GetForProfile(controller()->profile());
proxy->Uninstall(item().id.app_id, nullptr /* parent_window */); DCHECK(proxy);
proxy->Uninstall(item().id.app_id, nullptr /* parent_window */);
return;
}
arc::ShowArcAppUninstallDialog(controller()->profile(),
nullptr /* controller */, item().id.app_id);
return; return;
} }
......
...@@ -10,8 +10,6 @@ ...@@ -10,8 +10,6 @@
#include "ash/public/cpp/app_menu_constants.h" #include "ash/public/cpp/app_menu_constants.h"
#include "ash/public/cpp/shelf_item.h" #include "ash/public/cpp/shelf_item.h"
#include "base/bind_helpers.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_manager.h"
#include "chrome/browser/chromeos/crostini/crostini_registry_service.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_registry_service_factory.h"
...@@ -70,14 +68,11 @@ bool CrostiniShelfContextMenu::IsCommandIdEnabled(int command_id) const { ...@@ -70,14 +68,11 @@ bool CrostiniShelfContextMenu::IsCommandIdEnabled(int command_id) const {
void CrostiniShelfContextMenu::ExecuteCommand(int command_id, int event_flags) { void CrostiniShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
switch (command_id) { switch (command_id) {
case ash::UNINSTALL: { case ash::UNINSTALL:
DCHECK_NE(item().id.app_id, crostini::kCrostiniTerminalId); DCHECK_NE(item().id.app_id, crostini::kCrostiniTerminalId);
apps::AppServiceProxy* proxy = crostini::ShowCrostiniAppUninstallerView(controller()->profile(),
apps::AppServiceProxyFactory::GetForProfile(controller()->profile()); item().id.app_id);
DCHECK(proxy);
proxy->Uninstall(item().id.app_id, nullptr /* parent_window */);
return; return;
}
case ash::STOP_APP: case ash::STOP_APP:
if (item().id.app_id == crostini::kCrostiniTerminalId) { if (item().id.app_id == crostini::kCrostiniTerminalId) {
crostini::CrostiniManager::GetForProfile(controller()->profile()) crostini::CrostiniManager::GetForProfile(controller()->profile())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment