Skip to content

GitLab

T
tangled
Commit 9d154fea authored by Alan Cutter's avatar Alan Cutter Committed by Chromium LUCI CQ
Browse files
Options

Remove unused InstallFinalizer methods 

InstallFinalizer::UninstallWebAppFromSyncByUser() and
InstallFinalizer::CanUserUninstallFromSync() are not called by
production code. This CL removes them.

Change-Id: I51a0cd97da31e3b631c8a9c2a00228341156c321
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515423Reviewed-by: default avatarEric Willigers <ericwilligers@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834993}
parent 6916fceb
Hide whitespace changes
Inline Side-by-side
Showing with 3 additions and 136 deletions
chrome/browser/web_applications/components/install_finalizer.h
View file @ 9d154fea
......@@ -77,10 +77,6 @@ class InstallFinalizer {
ExternalInstallSource external_install_source,
UninstallWebAppCallback callback);
virtual bool CanUserUninstallFromSync(const AppId& app_id) const = 0;
virtual void UninstallWebAppFromSyncByUser(const AppId& app_id,
UninstallWebAppCallback) = 0;
virtual bool CanUserUninstallExternalApp(const AppId& app_id) const = 0;
// If external app is synced, uninstalls it from sync and from all devices.
virtual void UninstallExternalAppByUser(const AppId& app_id,
......
chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc
View file @ 9d154fea
......@@ -156,22 +156,6 @@ void BookmarkAppInstallFinalizer::UninstallExternalWebApp(
UninstallExtension(app_id, std::move(callback));
}
bool BookmarkAppInstallFinalizer::CanUserUninstallFromSync(
const web_app::AppId& app_id) const {
// Bookmark apps don't support app installation from different sources.
// The old system uninstalls extension completely, the implementation is
// the same:
return CanUserUninstallExternalApp(app_id);
}
void BookmarkAppInstallFinalizer::UninstallWebAppFromSyncByUser(
const web_app::AppId& app_id,
UninstallWebAppCallback callback) {
// Bookmark apps don't support app installation from different sources.
// Uninstall extension completely:
UninstallExtension(app_id, std::move(callback));
}
bool BookmarkAppInstallFinalizer::CanUserUninstallExternalApp(
const web_app::AppId& app_id) const {
const Extension* app = GetEnabledExtension(app_id);
......
chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.h
View file @ 9d154fea
......@@ -45,9 +45,6 @@ class BookmarkAppInstallFinalizer : public web_app::InstallFinalizer {
const web_app::AppId& app_id,
web_app::ExternalInstallSource external_install_source,
UninstallWebAppCallback callback) override;
bool CanUserUninstallFromSync(const web_app::AppId& app_id) const override;
void UninstallWebAppFromSyncByUser(const web_app::AppId& app_id,
UninstallWebAppCallback callback) override;
bool CanUserUninstallExternalApp(const web_app::AppId& app_id) const override;
void UninstallExternalAppByUser(const web_app::AppId& app_id,
UninstallWebAppCallback callback) override;
......
chrome/browser/web_applications/extensions/pending_app_install_task_unittest.cc
View file @ 9d154fea
......@@ -206,17 +206,6 @@ class TestPendingAppInstallFinalizer : public InstallFinalizer {
}));
}
bool CanUserUninstallFromSync(const AppId& app_id) const override {
NOTIMPLEMENTED();
return false;
}
void UninstallWebAppFromSyncByUser(
const AppId& app_dd,
UninstallWebAppCallback callback) override {
NOTIMPLEMENTED();
}
bool CanUserUninstallExternalApp(const AppId& app_id) const override {
NOTIMPLEMENTED();
return false;
......
chrome/browser/web_applications/manifest_update_manager_browsertest.cc
View file @ 9d154fea
......@@ -406,7 +406,7 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
GURL url = GetAppURL();
UpdateCheckResultAwaiter awaiter(browser(), url);
ui_test_utils::NavigateToURL(browser(), url);
GetProvider().install_finalizer().UninstallWebAppFromSyncByUser(
GetProvider().install_finalizer().UninstallExternalAppByUser(
app_id, base::DoNothing());
EXPECT_EQ(std::move(awaiter).AwaitNextResult(),
ManifestUpdateResult::kAppUninstalled);
......@@ -712,7 +712,7 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
OverrideManifest(kManifestTemplate, {"blue", kInstallableIconList});
AppId app_id = InstallPolicyApp();
EXPECT_FALSE(
GetProvider().install_finalizer().CanUserUninstallFromSync(app_id));
GetProvider().install_finalizer().CanUserUninstallExternalApp(app_id));
OverrideManifest(kManifestTemplate, {"red", kInstallableIconList});
EXPECT_EQ(GetResultAfterPageLoad(GetAppURL(), &app_id),
......@@ -724,7 +724,7 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
// Policy installed apps should continue to be not uninstallable by the user
// after updating.
EXPECT_FALSE(
GetProvider().install_finalizer().CanUserUninstallFromSync(app_id));
GetProvider().install_finalizer().CanUserUninstallExternalApp(app_id));
}
IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
......
chrome/browser/web_applications/test/test_install_finalizer.cc
View file @ 9d154fea
......@@ -75,17 +75,6 @@ void TestInstallFinalizer::UninstallExternalWebAppByUrl(
}));
}
bool TestInstallFinalizer::CanUserUninstallFromSync(const AppId& app_id) const {
NOTIMPLEMENTED();
return false;
}
void TestInstallFinalizer::UninstallWebAppFromSyncByUser(
const AppId& app_url,
UninstallWebAppCallback callback) {
NOTIMPLEMENTED();
}
bool TestInstallFinalizer::CanUserUninstallExternalApp(
const AppId& app_id) const {
NOTIMPLEMENTED();
......
chrome/browser/web_applications/test/test_install_finalizer.h
View file @ 9d154fea
......@@ -41,9 +41,6 @@ class TestInstallFinalizer final : public InstallFinalizer {
const GURL& app_url,
ExternalInstallSource external_install_source,
UninstallWebAppCallback callback) override;
bool CanUserUninstallFromSync(const AppId& app_id) const override;
void UninstallWebAppFromSyncByUser(const AppId& app_id,
UninstallWebAppCallback callback) override;
bool CanUserUninstallExternalApp(const AppId& app_id) const override;
void UninstallExternalAppByUser(const AppId& app_id,
UninstallWebAppCallback callback) override;
......
chrome/browser/web_applications/web_app_install_finalizer.cc
View file @ 9d154fea
......@@ -219,20 +219,6 @@ void WebAppInstallFinalizer::UninstallExternalWebApp(
UninstallWebAppOrRemoveSource(app_id, source, std::move(callback));
}
bool WebAppInstallFinalizer::CanUserUninstallFromSync(
const AppId& app_id) const {
DCHECK(started_);
const WebApp* app = GetWebAppRegistrar().GetAppById(app_id);
return app ? app->IsSynced() : false;
}
void WebAppInstallFinalizer::UninstallWebAppFromSyncByUser(
const AppId& app_id,
UninstallWebAppCallback callback) {
DCHECK(CanUserUninstallFromSync(app_id));
UninstallWebAppOrRemoveSource(app_id, Source::kSync, std::move(callback));
}
bool WebAppInstallFinalizer::CanUserUninstallExternalApp(
const AppId& app_id) const {
DCHECK(started_);
......
chrome/browser/web_applications/web_app_install_finalizer.h
View file @ 9d154fea
......@@ -45,9 +45,6 @@ class WebAppInstallFinalizer final : public InstallFinalizer {
void UninstallExternalWebApp(const AppId& app_id,
ExternalInstallSource external_install_source,
UninstallWebAppCallback callback) override;
bool CanUserUninstallFromSync(const AppId& app_id) const override;
void UninstallWebAppFromSyncByUser(const AppId& app_id,
UninstallWebAppCallback callback) override;
bool CanUserUninstallExternalApp(const AppId& app_id) const override;
void UninstallExternalAppByUser(const AppId& app_id,
UninstallWebAppCallback callback) override;
......
chrome/browser/web_applications/web_app_install_manager_unittest.cc
View file @ 9d154fea
......@@ -400,18 +400,6 @@ class WebAppInstallManagerTest : public WebAppTest {
return result;
}
bool UninstallWebAppFromSyncByUser(const AppId& app_id) {
bool result = false;
base::RunLoop run_loop;
finalizer().UninstallWebAppFromSyncByUser(
app_id, base::BindLambdaForTesting([&](bool uninstalled) {
result = uninstalled;
run_loop.Quit();
}));
run_loop.Run();
return result;
}
bool UninstallExternalAppByUser(const AppId& app_id) {
bool result = false;
base::RunLoop run_loop;
......@@ -893,7 +881,6 @@ TEST_F(WebAppInstallManagerTest, PolicyAndUser_UninstallExternalWebApp) {
external_app_url, app_id, ExternalInstallSource::kExternalPolicy);
InitRegistrarWithApp(std::move(policy_and_user_app));
EXPECT_TRUE(finalizer().CanUserUninstallFromSync(app_id));
EXPECT_FALSE(finalizer().WasExternalAppUninstalledByUser(app_id));
bool observer_uninstall_called = false;
......@@ -913,63 +900,8 @@ TEST_F(WebAppInstallManagerTest, PolicyAndUser_UninstallExternalWebApp) {
EXPECT_TRUE(registrar().GetAppById(app_id));
EXPECT_FALSE(observer_uninstall_called);
EXPECT_TRUE(finalizer().CanUserUninstallFromSync(app_id));
EXPECT_FALSE(finalizer().WasExternalAppUninstalledByUser(app_id));
EXPECT_TRUE(finalizer().CanUserUninstallExternalApp(app_id));
// Uninstall user app last.
file_utils().SetNextDeleteFileRecursivelyResult(true);
EXPECT_TRUE(UninstallWebAppFromSyncByUser(app_id));
EXPECT_FALSE(registrar().GetAppById(app_id));
EXPECT_TRUE(observer_uninstall_called);
EXPECT_FALSE(finalizer().CanUserUninstallFromSync(app_id));
EXPECT_FALSE(finalizer().WasExternalAppUninstalledByUser(app_id));
EXPECT_FALSE(finalizer().CanUserUninstallExternalApp(app_id));
}
TEST_F(WebAppInstallManagerTest, PolicyAndUser_UninstallWebAppFromSyncByUser) {
std::unique_ptr<WebApp> policy_and_user_app =
CreateWebApp(GURL("https://example.com/path"), Source::kSync,
/*user_display_mode=*/DisplayMode::kStandalone);
policy_and_user_app->AddSource(Source::kPolicy);
const AppId app_id = policy_and_user_app->app_id();
const GURL external_app_url("https://example.com/path/policy");
externally_installed_app_prefs().Insert(
external_app_url, app_id, ExternalInstallSource::kExternalPolicy);
InitRegistrarWithApp(std::move(policy_and_user_app));
EXPECT_TRUE(finalizer().CanUserUninstallFromSync(app_id));
EXPECT_FALSE(finalizer().CanUserUninstallExternalApp(app_id));
bool observer_uninstall_called = false;
WebAppInstallObserver observer(&registrar());
observer.SetWebAppUninstalledDelegate(
base::BindLambdaForTesting([&](const AppId& uninstalled_app_id) {
observer_uninstall_called = true;
}));
// Uninstall user app first.
EXPECT_TRUE(UninstallWebAppFromSyncByUser(app_id));
EXPECT_TRUE(registrar().GetAppById(app_id));
EXPECT_FALSE(observer_uninstall_called);
EXPECT_FALSE(finalizer().CanUserUninstallFromSync(app_id));
EXPECT_FALSE(finalizer().WasExternalAppUninstalledByUser(app_id));
EXPECT_FALSE(finalizer().CanUserUninstallExternalApp(app_id));
// Uninstall policy app last.
file_utils().SetNextDeleteFileRecursivelyResult(true);
EXPECT_TRUE(UninstallExternalWebAppByUrl(
external_app_url, ExternalInstallSource::kExternalPolicy));
EXPECT_FALSE(registrar().GetAppById(app_id));
EXPECT_TRUE(observer_uninstall_called);
EXPECT_FALSE(finalizer().WasExternalAppUninstalledByUser(app_id));
EXPECT_FALSE(finalizer().CanUserUninstallExternalApp(app_id));
}
TEST_F(WebAppInstallManagerTest, DefaultAndUser_UninstallExternalAppByUser) {
......
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