Keep a web_apps.extension_ids prefs-backed roster
Future commits will need it, as installing web apps from their URL will
need to know whether or not those web apps (and their underlying
extensions) are already installed.
Based on nigeltao's https://crrev.com/c/1179115
But rebased on top of latest CLs and added testing.
[email protected]
Bug: 876577
Change-Id: Ic3e837efe8fb2f79e30a2ffb044f6e02e988d20b
Reviewed-on: https://chromium-review.googlesource.com/1192462
Reviewed-by: Giovanni Ortuño Urquidi <[email protected]>
Reviewed-by: Dominick Ng <[email protected]>
Commit-Queue: Giovanni Ortuño Urquidi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#586669}diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 9625318..109d09b7 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -61,6 +61,7 @@
#include "chrome/browser/ui/webui/favicon_source.h"
#include "chrome/browser/ui/webui/theme_source.h"
#include "chrome/browser/upgrade_detector/upgrade_detector.h"
+#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h"
#include "chrome/common/buildflags.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/crash_keys.h"
@@ -146,6 +147,21 @@
return; // Yup, known extension, don't uninstall.
}
+ // Historically, the code under //chrome/browser/extensions has
+ // unsurprisingly managed all extensions. Later, Progressive Web Apps (PWAs)
+ // were implemented on top of extensions, more out of convenience than out of
+ // principle. As of mid 2018, there is work underway to separate PWAs's
+ // implementation details from the //c/b/e code. During the transition
+ // period, PWA-extensions are no longer managed solely by //c/b/e code. We
+ // add a special case here so that //c/b/e code doesn't uninstall
+ // PWA-extensions that it doesn't otherwise know about.
+ //
+ // Long term, PWAs will be completely separate from extensions, and we can
+ // remove this cross-link.
+ if (web_app::ExtensionIdsMap::HasExtensionId(profile_->GetPrefs(), id)) {
+ return;
+ }
+
// We get the list of external extensions to check from preferences.
// It is possible that an extension has preferences but is not loaded.
// For example, an extension that requires experimental permissions
diff --git a/chrome/browser/web_applications/extensions/BUILD.gn b/chrome/browser/web_applications/extensions/BUILD.gn
index 994d2c9..f1f5ce3 100644
--- a/chrome/browser/web_applications/extensions/BUILD.gn
+++ b/chrome/browser/web_applications/extensions/BUILD.gn
@@ -20,6 +20,8 @@
"bookmark_app_util.h",
"pending_bookmark_app_manager.cc",
"pending_bookmark_app_manager.h",
+ "web_app_extension_ids_map.cc",
+ "web_app_extension_ids_map.h",
"web_app_extension_shortcut.cc",
"web_app_extension_shortcut.h",
"web_app_extension_shortcut_mac.h",
@@ -31,6 +33,7 @@
"//chrome/browser/web_applications:web_app_group",
"//chrome/browser/web_applications/components",
"//chrome/common",
+ "//components/pref_registry",
"//content/public/browser",
"//extensions/browser",
"//skia",
@@ -54,6 +57,7 @@
"//chrome/browser/web_applications/components",
"//chrome/common",
"//chrome/test:test_support",
+ "//components/crx_file:crx_file",
"//content/public/browser",
"//content/test:test_support",
"//extensions/browser/install",
diff --git a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
index 39e1bc4..2d33daa 100644
--- a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
+++ b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
@@ -12,8 +12,11 @@
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
namespace extensions {
@@ -48,6 +51,7 @@
PendingBookmarkAppManager::PendingBookmarkAppManager(Profile* profile)
: profile_(profile),
+ extension_ids_map_(profile->GetPrefs()),
web_contents_factory_(base::BindRepeating(&WebContentsCreateWrapper)),
task_factory_(base::BindRepeating(&InstallationTaskCreateWrapper)),
timer_(std::make_unique<base::OneShotTimer>()) {}
@@ -66,6 +70,14 @@
weak_ptr_factory_.GetWeakPtr()));
}
+// TODO(nigeltao/ortuno): clarify whether the apps_to_install is relative or
+// absolute: in C++ terminology, an analogy is += versus = operators. Should we
+// install these apps *in addition to* what's already installed, or should we
+// make the list of installed apps is identical to the argument? If the former,
+// we also need a way to tell the PendingBookmarkAppManager to uninstall apps.
+// If the latter, we also need to pass the install source (a Manifest::Location
+// or something similar), so that "the list of policy-installed apps" doesn't
+// interfere with "the list of default-installed apps".
void PendingBookmarkAppManager::InstallApps(
std::vector<AppInfo> apps_to_install,
const RepeatingInstallCallback& callback) {
@@ -92,29 +104,66 @@
timer_ = std::move(timer);
}
+// Returns (as the base::Optional part) whether or not there is already a known
+// extension for the given ID. The bool inside the base::Optional is, when
+// known, whether the extension is installed (true) or uninstalled (false).
+base::Optional<bool> PendingBookmarkAppManager::IsExtensionPresentAndInstalled(
+ const std::string& extension_id) {
+ if (ExtensionRegistry::Get(profile_)->GetExtensionById(
+ extension_id, ExtensionRegistry::EVERYTHING) != nullptr) {
+ return base::Optional<bool>(true);
+ }
+ if (ExtensionPrefs::Get(profile_)->IsExternalExtensionUninstalled(
+ extension_id)) {
+ return base::Optional<bool>(false);
+ }
+
+ return base::nullopt;
+}
+
void PendingBookmarkAppManager::MaybeStartNextInstallation() {
if (current_task_and_callback_)
return;
- if (pending_tasks_and_callbacks_.empty()) {
- web_contents_.reset();
+ while (!pending_tasks_and_callbacks_.empty()) {
+ std::unique_ptr<TaskAndCallback> front =
+ std::move(pending_tasks_and_callbacks_.front());
+ pending_tasks_and_callbacks_.pop_front();
+
+ base::Optional<std::string> extension_id =
+ extension_ids_map_.Lookup(front->task->app_info().url);
+
+ if (extension_id) {
+ base::Optional<bool> opt =
+ IsExtensionPresentAndInstalled(extension_id.value());
+ if (opt.has_value()) {
+ // TODO(crbug.com/878262): Handle the case where the app is already
+ // installed but from a different source.
+ std::move(front->callback)
+ .Run(front->task->app_info().url,
+ opt.value() ? extension_id : base::nullopt);
+ continue;
+ }
+ }
+
+ current_task_and_callback_ = std::move(front);
+
+ CreateWebContentsIfNecessary();
+ Observe(web_contents_.get());
+
+ content::NavigationController::LoadURLParams load_params(
+ current_task_and_callback_->task->app_info().url);
+ load_params.transition_type = ui::PAGE_TRANSITION_GENERATED;
+ web_contents_->GetController().LoadURLWithParams(load_params);
+ timer_->Start(
+ FROM_HERE,
+ base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
+ base::BindOnce(&PendingBookmarkAppManager::OnWebContentsLoadTimedOut,
+ weak_ptr_factory_.GetWeakPtr()));
return;
}
- current_task_and_callback_ = std::move(pending_tasks_and_callbacks_.front());
- pending_tasks_and_callbacks_.pop_front();
-
- CreateWebContentsIfNecessary();
- Observe(web_contents_.get());
-
- content::NavigationController::LoadURLParams load_params(
- current_task_and_callback_->task->app_info().url);
- load_params.transition_type = ui::PAGE_TRANSITION_GENERATED;
- web_contents_->GetController().LoadURLWithParams(load_params);
- timer_->Start(
- FROM_HERE, base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
- base::BindOnce(&PendingBookmarkAppManager::OnWebContentsLoadTimedOut,
- weak_ptr_factory_.GetWeakPtr()));
+ web_contents_.reset();
}
void PendingBookmarkAppManager::CreateWebContentsIfNecessary() {
@@ -147,6 +196,12 @@
base::BindOnce(&PendingBookmarkAppManager::MaybeStartNextInstallation,
weak_ptr_factory_.GetWeakPtr()));
+ // An empty app_id means that the installation failed.
+ if (app_id) {
+ extension_ids_map_.Insert(current_task_and_callback_->task->app_info().url,
+ app_id.value());
+ }
+
std::unique_ptr<TaskAndCallback> task_and_callback;
task_and_callback.swap(current_task_and_callback_);
std::move(task_and_callback->callback)
diff --git a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
index 052909cd..910d64b 100644
--- a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
+++ b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
@@ -17,6 +17,7 @@
#include "base/timer/timer.h"
#include "chrome/browser/web_applications/components/pending_app_manager.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.h"
+#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h"
#include "content/public/browser/web_contents_observer.h"
class GURL;
@@ -52,12 +53,14 @@
void SetFactoriesForTesting(WebContentsFactory web_contents_factory,
TaskFactory task_factory);
-
void SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer);
private:
struct TaskAndCallback;
+ base::Optional<bool> IsExtensionPresentAndInstalled(
+ const std::string& extension_id);
+
void MaybeStartNextInstallation();
void CreateWebContentsIfNecessary();
@@ -77,6 +80,7 @@
const base::string16& error_description) override;
Profile* profile_;
+ web_app::ExtensionIdsMap extension_ids_map_;
WebContentsFactory web_contents_factory_;
TaskFactory task_factory_;
diff --git a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
index 2a139bba..cb2aca1 100644
--- a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
+++ b/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
@@ -19,7 +19,11 @@
#include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_profile.h"
+#include "components/crx_file/id_util.h"
#include "content/public/test/web_contents_tester.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension_builder.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace extensions {
@@ -29,6 +33,7 @@
const char kFooWebAppUrl[] = "https://foo.example";
const char kBarWebAppUrl[] = "https://bar.example";
const char kQuxWebAppUrl[] = "https://qux.example";
+const char kXyzWebAppUrl[] = "https://xyz.example";
const char kWrongUrl[] = "https://foobar.example";
@@ -49,6 +54,19 @@
web_app::PendingAppManager::LaunchContainer::kWindow);
}
+web_app::PendingAppManager::AppInfo GetXyzAppInfo() {
+ return web_app::PendingAppManager::AppInfo::Create(
+ GURL(kXyzWebAppUrl),
+ web_app::PendingAppManager::LaunchContainer::kWindow);
+}
+
+scoped_refptr<Extension> CreateDummyExtension(const std::string& id) {
+ return ExtensionBuilder("Dummy name")
+ .SetLocation(Manifest::INTERNAL)
+ .SetID(id)
+ .Build();
+}
+
} // namespace
class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
@@ -57,6 +75,7 @@
web_app::PendingAppManager::AppInfo app_info,
bool succeeds)
: BookmarkAppInstallationTask(profile, std::move(app_info)),
+ profile_(profile),
succeeds_(succeeds) {}
~TestBookmarkAppInstallationTask() override = default;
@@ -68,7 +87,10 @@
std::string app_id;
if (succeeds_) {
result_code = BookmarkAppInstallationTask::ResultCode::kSuccess;
- app_id = "fake_app_id_for:" + app_info().url.spec();
+ app_id = crx_file::id_util::GenerateId("fake_app_id_for:" +
+ app_info().url.spec());
+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
+ registry->AddEnabled(CreateDummyExtension(app_id));
}
std::move(on_install_called_).Run();
@@ -81,6 +103,7 @@
}
private:
+ Profile* profile_;
bool succeeds_;
base::OnceClosure on_install_called_;
@@ -129,8 +152,10 @@
auto task = std::make_unique<TestBookmarkAppInstallationTask>(
profile, std::move(app_info), succeeds);
auto* task_ptr = task.get();
- task->SetOnInstallCalled(base::BindLambdaForTesting(
- [task_ptr, this]() { last_app_info_ = task_ptr->app_info().Clone(); }));
+ task->SetOnInstallCalled(base::BindLambdaForTesting([task_ptr, this]() {
+ ++installation_task_run_count_;
+ last_app_info_ = task_ptr->app_info().Clone();
+ }));
return task;
}
@@ -151,14 +176,14 @@
void InstallCallback(const GURL& url,
const base::Optional<std::string>& app_id) {
install_callback_url_ = url;
- install_succeeded_ = app_id.has_value();
+ last_app_id_ = app_id;
}
protected:
void ResetResults() {
- install_succeeded_.reset();
install_callback_url_.reset();
- last_app_info_.reset();
+ last_app_id_.reset();
+ installation_task_run_count_ = 0;
}
const PendingBookmarkAppManager::WebContentsFactory&
@@ -193,20 +218,27 @@
return web_contents_tester_;
}
- bool install_succeeded() { return install_succeeded_.value(); }
+ bool install_succeeded() { return last_app_id_.has_value(); }
const GURL& install_callback_url() { return install_callback_url_.value(); }
+ const std::string& last_app_id() { return last_app_id_.value(); }
+
const web_app::PendingAppManager::AppInfo& last_app_info() {
CHECK(last_app_info_.get());
return *last_app_info_;
}
+ // Number of times BookmarkAppInstallationTask::InstallWebAppOrShorcut was
+ // called. Reflects how many times we've tried to create an Extension.
+ size_t installation_task_run_count() { return installation_task_run_count_; }
+
private:
content::WebContentsTester* web_contents_tester_ = nullptr;
- base::Optional<bool> install_succeeded_;
base::Optional<GURL> install_callback_url_;
+ base::Optional<std::string> last_app_id_;
std::unique_ptr<web_app::PendingAppManager::AppInfo> last_app_info_;
+ size_t installation_task_run_count_ = 0;
PendingBookmarkAppManager::WebContentsFactory test_web_contents_creator_;
PendingBookmarkAppManager::TaskFactory successful_installation_task_creator_;
@@ -225,12 +257,13 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
}
-TEST_F(PendingBookmarkAppManagerTest, Install_SucceedsTwice) {
+TEST_F(PendingBookmarkAppManagerTest, Install_SerialCallsDifferentApps) {
auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
pending_app_manager->Install(
GetFooAppInfo(),
@@ -240,6 +273,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -253,12 +287,13 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
}
-TEST_F(PendingBookmarkAppManagerTest, Install_ConcurrentCalls) {
+TEST_F(PendingBookmarkAppManagerTest, Install_ConcurrentCallsDifferentApps) {
auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
pending_app_manager->Install(
GetFooAppInfo(),
@@ -273,6 +308,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -282,6 +318,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -305,6 +342,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -314,6 +352,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -337,6 +376,8 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kWrongUrl));
+ // The installation didn't run because we loaded the wrong url.
+ EXPECT_EQ(0u, installation_task_run_count());
EXPECT_FALSE(install_succeeded());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
ResetResults();
@@ -345,6 +386,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -369,6 +411,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -377,12 +420,41 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
}
-TEST_F(PendingBookmarkAppManagerTest, Install_SucceedsSameInstallPending) {
+TEST_F(PendingBookmarkAppManagerTest, Install_SerialCallsSameApp) {
+ auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+
+ base::RunLoop().RunUntilIdle();
+ SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+ EXPECT_EQ(1u, installation_task_run_count());
+ EXPECT_TRUE(install_succeeded());
+ EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
+ ResetResults();
+
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+
+ base::RunLoop().RunUntilIdle();
+
+ // The app is already installed so we shouldn't try to install it again.
+ EXPECT_EQ(0u, installation_task_run_count());
+ EXPECT_TRUE(install_succeeded());
+ EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
+}
+
+TEST_F(PendingBookmarkAppManagerTest, Install_ConcurrentCallsSameApp) {
auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
pending_app_manager->Install(
GetFooAppInfo(),
@@ -396,18 +468,18 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
ResetResults();
base::RunLoop().RunUntilIdle();
- SuccessfullyLoad(GURL(kFooWebAppUrl));
// The second installation should succeed even though the app is installed
// already.
+ EXPECT_EQ(0u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
- EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
}
@@ -420,6 +492,8 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kWrongUrl));
+
+ EXPECT_EQ(0u, installation_task_run_count());
EXPECT_FALSE(install_succeeded());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
}
@@ -437,6 +511,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -455,6 +530,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kWrongUrl));
+ EXPECT_EQ(0u, installation_task_run_count());
EXPECT_FALSE(install_succeeded());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
}
@@ -475,6 +551,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -484,6 +561,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -516,6 +594,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -525,6 +604,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -553,6 +633,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kQuxWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetQuxAppInfo(), last_app_info());
EXPECT_EQ(GURL(kQuxWebAppUrl), install_callback_url());
@@ -562,6 +643,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
@@ -570,6 +652,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -597,6 +680,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kQuxWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetQuxAppInfo(), last_app_info());
EXPECT_EQ(GURL(kQuxWebAppUrl), install_callback_url());
@@ -606,6 +690,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kFooWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
EXPECT_EQ(GetFooAppInfo(), last_app_info());
@@ -614,6 +699,7 @@
base::RunLoop().RunUntilIdle();
SuccessfullyLoad(GURL(kBarWebAppUrl));
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -626,36 +712,37 @@
pending_app_manager->SetTimerForTesting(std::move(timer_to_pass));
- // Queue through Install.
+ // Queue an app through Install.
pending_app_manager->Install(
GetQuxAppInfo(),
base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
base::Unretained(this)));
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(timer->IsRunning());
// Verify that the timer is stopped after a successful load.
+ EXPECT_TRUE(timer->IsRunning());
SuccessfullyLoad(GURL(kQuxWebAppUrl));
EXPECT_FALSE(timer->IsRunning());
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GURL(kQuxWebAppUrl), install_callback_url());
ResetResults();
- // Queue through Install.
+ // Queue a different app through Install.
pending_app_manager->Install(
- GetQuxAppInfo(),
+ GetXyzAppInfo(),
base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
base::Unretained(this)));
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(timer->IsRunning());
// Fire the timer to simulate a failed load.
+ EXPECT_TRUE(timer->IsRunning());
timer->Fire();
EXPECT_FALSE(install_succeeded());
- EXPECT_EQ(GURL(kQuxWebAppUrl), install_callback_url());
+ EXPECT_EQ(GURL(kXyzWebAppUrl), install_callback_url());
ResetResults();
- // Queue through InstallApps.
+ // Queue two more apps, different from all those before, through InstallApps.
std::vector<web_app::PendingAppManager::AppInfo> apps_to_install;
apps_to_install.push_back(GetFooAppInfo());
apps_to_install.push_back(GetBarAppInfo());
@@ -666,18 +753,18 @@
base::Unretained(this)));
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(timer->IsRunning());
// Fire the timer to simulate a failed load.
+ EXPECT_TRUE(timer->IsRunning());
timer->Fire();
EXPECT_FALSE(install_succeeded());
EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
ResetResults();
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(timer->IsRunning());
// Fire the timer to simulate a failed load.
+ EXPECT_TRUE(timer->IsRunning());
timer->Fire();
EXPECT_FALSE(install_succeeded());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
@@ -689,14 +776,82 @@
base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
base::Unretained(this)));
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(timer->IsRunning());
// Verify that the timer is stopped after a successful load.
+ EXPECT_TRUE(timer->IsRunning());
SuccessfullyLoad(GURL(kBarWebAppUrl));
EXPECT_FALSE(timer->IsRunning());
+ EXPECT_EQ(1u, installation_task_run_count());
EXPECT_TRUE(install_succeeded());
EXPECT_EQ(GetBarAppInfo(), last_app_info());
EXPECT_EQ(GURL(kBarWebAppUrl), install_callback_url());
}
+TEST_F(PendingBookmarkAppManagerTest, ExtensionUninstalled) {
+ auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+
+ base::RunLoop().RunUntilIdle();
+ SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+ EXPECT_EQ(1u, installation_task_run_count());
+ EXPECT_TRUE(install_succeeded());
+
+ const std::string app_id = last_app_id();
+ ResetResults();
+
+ // Simulate the extension for the app getting uninstalled.
+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+ registry->RemoveEnabled(app_id);
+
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+
+ base::RunLoop().RunUntilIdle();
+ SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+ // The extension was uninstalled so a new installation task should run.
+ EXPECT_EQ(1u, installation_task_run_count());
+ EXPECT_TRUE(install_succeeded());
+}
+
+TEST_F(PendingBookmarkAppManagerTest, ExternalExtensionUninstalled) {
+ auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+
+ base::RunLoop().RunUntilIdle();
+ SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+ EXPECT_EQ(1u, installation_task_run_count());
+ EXPECT_TRUE(install_succeeded());
+
+ const std::string app_id = last_app_id();
+ ResetResults();
+
+ // Simulate external extension for the app getting uninstalled by the user.
+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+ registry->RemoveEnabled(app_id);
+ ExtensionPrefs::Get(profile())->OnExtensionUninstalled(
+ app_id, Manifest::EXTERNAL_POLICY, false /* external_uninstall */);
+
+ pending_app_manager->Install(
+ GetFooAppInfo(),
+ base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+ base::Unretained(this)));
+ base::RunLoop().RunUntilIdle();
+
+ // The extension was uninstalled by the user, we shouldn't try to install it
+ // again.
+ EXPECT_EQ(0u, installation_task_run_count());
+ EXPECT_FALSE(install_succeeded());
+}
+
} // namespace extensions
diff --git a/chrome/browser/web_applications/extensions/web_app_extension_ids_map.cc b/chrome/browser/web_applications/extensions/web_app_extension_ids_map.cc
new file mode 100644
index 0000000..50726c5
--- /dev/null
+++ b/chrome/browser/web_applications/extensions/web_app_extension_ids_map.cc
@@ -0,0 +1,61 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h"
+
+#include "base/values.h"
+#include "chrome/common/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
+#include "content/public/browser/browser_thread.h"
+#include "url/gurl.h"
+
+namespace web_app {
+
+// static
+void ExtensionIdsMap::RegisterProfilePrefs(
+ user_prefs::PrefRegistrySyncable* registry) {
+ registry->RegisterDictionaryPref(prefs::kWebAppsExtensionIDs);
+}
+
+// static
+bool ExtensionIdsMap::HasExtensionId(const PrefService* pref_service,
+ const std::string& extension_id) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ const base::DictionaryValue* dict =
+ pref_service->GetDictionary(prefs::kWebAppsExtensionIDs);
+ if (!dict) {
+ return false;
+ }
+ // Do a simple O(N) scan for extension_id being a value in the dictionary's
+ // key/value pairs. We expect both N and the number of times HasExtensionId
+ // is called to be relatively small in practice. If they turn out to be
+ // large, we can write a more sophisticated implementation.
+ for (const auto& it : dict->DictItems()) {
+ if (it.second.is_string() && it.second.GetString() == extension_id) {
+ return true;
+ }
+ }
+ return false;
+}
+
+ExtensionIdsMap::ExtensionIdsMap(PrefService* pref_service)
+ : pref_service_(pref_service) {}
+
+void ExtensionIdsMap::Insert(const GURL& url, const std::string& extension_id) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ DictionaryPrefUpdate dict_update(pref_service_, prefs::kWebAppsExtensionIDs);
+ dict_update->SetKey(url.spec(), base::Value(extension_id));
+}
+
+base::Optional<std::string> ExtensionIdsMap::Lookup(const GURL& url) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ const base::Value* value =
+ pref_service_->GetDictionary(prefs::kWebAppsExtensionIDs)
+ ->FindKeyOfType(url.spec(), base::Value::Type::STRING);
+ return value ? base::make_optional(value->GetString()) : base::nullopt;
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/extensions/web_app_extension_ids_map.h b/chrome/browser/web_applications/extensions/web_app_extension_ids_map.h
new file mode 100644
index 0000000..b08308c
--- /dev/null
+++ b/chrome/browser/web_applications/extensions/web_app_extension_ids_map.h
@@ -0,0 +1,46 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_WEB_APP_EXTENSION_IDS_MAP_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_WEB_APP_EXTENSION_IDS_MAP_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/optional.h"
+
+class GURL;
+class PrefService;
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
+namespace web_app {
+
+// A Prefs-backed map from web app URLs to Chrome extension IDs.
+//
+// This lets us determine, given a web app's URL, whether that web app is
+// already installed.
+class ExtensionIdsMap {
+ public:
+ static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+ static bool HasExtensionId(const PrefService* pref_service,
+ const std::string& extension_id);
+
+ explicit ExtensionIdsMap(PrefService* pref_service);
+
+ void Insert(const GURL& url, const std::string& extension_id);
+ base::Optional<std::string> Lookup(const GURL& url);
+
+ private:
+ PrefService* pref_service_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExtensionIdsMap);
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_WEB_APP_EXTENSION_IDS_MAP_H_
diff --git a/chrome/browser/web_applications/web_app_provider.cc b/chrome/browser/web_applications/web_app_provider.cc
index 61e6e60..f7d5dcb 100644
--- a/chrome/browser/web_applications/web_app_provider.cc
+++ b/chrome/browser/web_applications/web_app_provider.cc
@@ -11,6 +11,7 @@
#include "chrome/browser/web_applications/bookmark_apps/external_web_apps.h"
#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
#include "chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h"
+#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h"
#include "chrome/browser/web_applications/web_app_provider_factory.h"
namespace web_app {
@@ -38,6 +39,7 @@
// static
void WebAppProvider::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
+ ExtensionIdsMap::RegisterProfilePrefs(registry);
WebAppPolicyManager::RegisterProfilePrefs(registry);
}
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index ef5f7db..92db9225 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -1587,6 +1587,9 @@
// will be launched.
const char kWebAppInstallForceList[] = "profile.web_app.install.forcelist";
+// Dictionary that maps web app URLs to Chrome extension IDs.
+const char kWebAppsExtensionIDs[] = "web_apps.extension_ids";
+
// Dictionary that maps Geolocation network provider server URLs to
// corresponding access token.
const char kGeolocationAccessToken[] = "geolocation.access_token";
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index de89904..e9e1455 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -560,6 +560,8 @@
extern const char kWebAppInstallForceList[];
+extern const char kWebAppsExtensionIDs[];
+
extern const char kGeolocationAccessToken[];
extern const char kDefaultAudioCaptureDevice[];
