Commit 9b871fda authored by Dominick Ng's avatar Dominick Ng Committed by Commit Bot

Update ApkWebAppInstaller to work with new web apps backend.

This CL de-extension-ifies ApkWebAppInstaller, and updates its tests to
be run over both the extension backend and the new web app backend.

BUG=1062145

Change-Id: I6277fc02873eb1b99101cf69eda57db9f332494e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106035
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750863}
parent 9f1cf9d2
......@@ -125,9 +125,9 @@ void ApkWebAppInstaller::OnWebAppCreated(const GURL& app_url,
return;
}
// Otherwise, insert this web app into the extensions ID map so it is not
// removed automatically. TODO(crbug.com/910008): have a less bad way of doing
// this.
// Otherwise, insert this web app into the externally installed ID map so it
// is not removed automatically. TODO(crbug.com/910008): have a less bad way
// of doing this.
web_app::ExternallyInstalledWebAppPrefs(profile_->GetPrefs())
.Insert(app_url, app_id, web_app::ExternalInstallSource::kArc);
CompleteInstallation(app_id, code);
......
......@@ -7,23 +7,22 @@
#include <utility>
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/feature_list.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/chromeos/apps/apk_web_app_service_factory.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
#include "chrome/browser/web_applications/components/externally_installed_web_app_prefs.h"
#include "chrome/browser/web_applications/components/install_finalizer.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/common/chrome_features.h"
#include "components/arc/mojom/app.mojom.h"
#include "components/arc/session/connection_holder.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/uninstall_reason.h"
#include "extensions/common/extension.h"
#include "url/gurl.h"
namespace {
......@@ -32,11 +31,11 @@ namespace {
// {
// ...
// "web_app_apks" : {
// <extension_id_1> : {
// <web_app_id_1> : {
// "package_name" : <apk_package_name_1>,
// "should_remove": <bool>
// },
// <extension_id_2> : {
// <web_app_id_2> : {
// "package_name" : <apk_package_name_2>,
// "should_remove": <bool>
// },
......@@ -69,12 +68,15 @@ void ApkWebAppService::RegisterProfilePrefs(
}
ApkWebAppService::ApkWebAppService(Profile* profile)
: profile_(profile), arc_app_list_prefs_(ArcAppListPrefs::Get(profile)) {
: profile_(profile),
arc_app_list_prefs_(ArcAppListPrefs::Get(profile)),
provider_(web_app::WebAppProvider::Get(profile)) {
// Can be null in tests.
if (arc_app_list_prefs_)
arc_app_list_prefs_->AddObserver(this);
observer_.Add(extensions::ExtensionRegistry::Get(profile));
DCHECK(provider_);
registrar_observer_.Add(&provider_->registrar());
}
ApkWebAppService::~ApkWebAppService() = default;
......@@ -106,16 +108,8 @@ void ApkWebAppService::UninstallWebApp(const web_app::AppId& web_app_id) {
return;
}
extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(profile_);
const extensions::Extension* extension = registry->GetExtensionById(
web_app_id, extensions::ExtensionRegistry::EVERYTHING);
if (extension) {
extensions::ExtensionSystem::Get(profile_)
->extension_service()
->UninstallExtension(extension->id(), extensions::UNINSTALL_REASON_ARC,
/*error=*/nullptr);
}
provider_->install_finalizer().UninstallExternalWebApp(
web_app_id, web_app::ExternalInstallSource::kArc, base::DoNothing());
}
void ApkWebAppService::UpdateShelfPin(
......@@ -242,10 +236,10 @@ void ApkWebAppService::OnPackageRemoved(const std::string& package_name,
// associated with an installed web app. If it is, there are 2 potential
// cases:
// 1) The user has uninstalled the web app already (e.g. via the
// launcher), which has called OnExtensionUninstalled() below and triggered
// launcher), which has called OnWebAppUninstalled() below and triggered
// the uninstallation of the Android package.
//
// In this case, OnExtensionUninstalled() will have removed the associated
// In this case, OnWebAppUninstalled() will have removed the associated
// web_app_id from the pref dict before triggering uninstallation, so this
// method will do nothing.
//
......@@ -256,7 +250,7 @@ void ApkWebAppService::OnPackageRemoved(const std::string& package_name,
// called, so the associated web_app_id is in the pref dict, and this method
// will trigger the uninstallation of the web app. Similarly, this method
// removes the associated web_app_id before triggering uninstallation, so
// OnExtensionUninstalled() will do nothing.
// OnWebAppUninstalled() will do nothing.
if (!base::FeatureList::IsEnabled(features::kApkWebAppInstalls))
return;
......@@ -326,10 +320,7 @@ void ApkWebAppService::OnPackageListInitialRefreshed() {
}
}
void ApkWebAppService::OnExtensionUninstalled(
content::BrowserContext* browser_context,
const extensions::Extension* extension,
extensions::UninstallReason reason) {
void ApkWebAppService::OnWebAppUninstalled(const web_app::AppId& web_app_id) {
if (!base::FeatureList::IsEnabled(features::kApkWebAppInstalls))
return;
......@@ -338,7 +329,7 @@ void ApkWebAppService::OnExtensionUninstalled(
// Find the package name associated with the provided web app id.
const base::Value* package_name_value = web_apps_to_apks->FindPathOfType(
{extension->id(), kPackageNameKey}, base::Value::Type::STRING);
{web_app_id, kPackageNameKey}, base::Value::Type::STRING);
const std::string package_name =
package_name_value ? package_name_value->GetString() : "";
......@@ -349,22 +340,22 @@ void ApkWebAppService::OnExtensionUninstalled(
if (instance) {
// Remove the web app id from prefs, otherwise the corresponding call to
// OnPackageRemoved will start an uninstallation cycle.
web_apps_to_apks->RemoveKey(extension->id());
web_apps_to_apks->RemoveKey(web_app_id);
instance->UninstallPackage(package_name);
} else {
// Set that the app should be removed next time the ARC container is
// ready.
web_apps_to_apks->SetPath({extension->id(), kShouldRemoveKey},
web_apps_to_apks->SetPath({web_app_id, kShouldRemoveKey},
base::Value(true));
}
}
// Post task to make sure that all OnExtensionUninstalled observers get
// fired before the callback called.
// Post task to make sure that all observers get fired before the callback
// called.
if (web_app_uninstalled_callback_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(web_app_uninstalled_callback_),
package_name, extension->id()));
package_name, web_app_id));
}
}
......
......@@ -15,10 +15,10 @@
#include "base/scoped_observer.h"
#include "chrome/browser/chromeos/apps/apk_web_app_installer.h"
#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
#include "chrome/browser/web_applications/components/app_registrar.h"
#include "chrome/browser/web_applications/components/app_registrar_observer.h"
#include "chrome/browser/web_applications/components/web_app_id.h"
#include "components/keyed_service/core/keyed_service.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_registry_observer.h"
class ArcAppListPrefs;
class Profile;
......@@ -29,6 +29,7 @@ class PrefRegistrySyncable;
namespace web_app {
enum class InstallResultCode;
class WebAppProvider;
} // namespace web_app
namespace chromeos {
......@@ -36,7 +37,7 @@ namespace chromeos {
class ApkWebAppService : public KeyedService,
public ApkWebAppInstaller::Owner,
public ArcAppListPrefs::Observer,
public extensions::ExtensionRegistryObserver {
public web_app::AppRegistrarObserver {
public:
static ApkWebAppService* Get(Profile* profile);
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
......@@ -77,10 +78,8 @@ class ApkWebAppService : public KeyedService,
bool uninstalled) override;
void OnPackageListInitialRefreshed() override;
// extensions::ExtensionRegistryObserver overrides.
void OnExtensionUninstalled(content::BrowserContext* browser_context,
const extensions::Extension* extension,
extensions::UninstallReason reason) override;
// web_app::AppRegistrarObserver overrides.
void OnWebAppUninstalled(const web_app::AppId& web_app_id) override;
void OnDidGetWebAppIcon(const std::string& package_name,
arc::mojom::WebAppInfoPtr web_app_info,
......@@ -94,10 +93,10 @@ class ApkWebAppService : public KeyedService,
Profile* profile_;
ArcAppListPrefs* arc_app_list_prefs_;
web_app::WebAppProvider* provider_;
ScopedObserver<extensions::ExtensionRegistry,
extensions::ExtensionRegistryObserver>
observer_{this};
ScopedObserver<web_app::AppRegistrar, web_app::AppRegistrarObserver>
registrar_observer_{this};
// Must go last.
base::WeakPtrFactory<ApkWebAppService> weak_ptr_factory_{this};
......
......@@ -9,6 +9,7 @@
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs_factory.h"
#include "chrome/browser/web_applications/web_app_provider_factory.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
namespace chromeos {
......@@ -29,6 +30,7 @@ ApkWebAppServiceFactory::ApkWebAppServiceFactory()
"ApkWebAppService",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(ArcAppListPrefsFactory::GetInstance());
DependsOn(web_app::WebAppProviderFactory::GetInstance());
}
ApkWebAppServiceFactory::~ApkWebAppServiceFactory() {}
......
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