Migrate extensions::BookmarkOrHostedAppInstalled() to AppRegistrar.

This CL deletes the extensions::BookmarkOrHostedAppInstalled() method, and adds an abstract IsInstalled(const GURL& start_url) method to the AppRegistrar to replace it. The new method's concrete implementation lives in BookmarkAppRegistrar (migrated from its old home). The existing call sites for the method are migrated as well. BUG=891172 Change-Id: Ide93d8f6e799ad325ae8b8902b43787c89636601 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1585281 Commit-Queue: Dominick Ng <dominickn@chromium.org> Reviewed-by: Alexey Baskakov <loyso@chromium.org> Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/heads/master@{#654777}
Denger-Network · Apr 29, 2019 · fdaf095 · fdaf095
1 parent 010beef
commit fdaf095
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 36 deletions.
diff --git a/chrome/browser/banners/app_banner_manager_desktop.cc b/chrome/browser/banners/app_banner_manager_desktop.cc
@@ -13,8 +13,10 @@
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
+#include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
 #include "extensions/common/constants.h"
 
@@ -93,8 +95,10 @@ bool AppBannerManagerDesktop::IsWebAppConsideredInstalled(
     const GURL& validated_url,
     const GURL& start_url,
     const GURL& manifest_url) {
-  return extensions::BookmarkOrHostedAppInstalled(
-      web_contents->GetBrowserContext(), start_url);
+  return web_app::WebAppProvider::Get(
+             Profile::FromBrowserContext(web_contents->GetBrowserContext()))
+      ->registrar()
+      .IsInstalled(start_url);
 }
 
 void AppBannerManagerDesktop::ShowBannerUi(WebappInstallSource install_source) {

diff --git a/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc b/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc
@@ -29,11 +29,11 @@
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
+#include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/components/web_app_utils.h"
-#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
 #include "chrome/common/extensions/extension_metrics.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/web_application_info.h"
@@ -368,7 +368,10 @@ ChromeManagementAPIDelegate::GenerateAppForLinkFunctionDelegate(
 bool ChromeManagementAPIDelegate::IsWebAppInstalled(
     content::BrowserContext* context,
     const GURL& web_app_url) const {
-  return extensions::BookmarkOrHostedAppInstalled(context, web_app_url);
+  auto* provider = web_app::WebAppProviderBase::GetProviderBase(
+      Profile::FromBrowserContext(context));
+  DCHECK(provider);
+  return provider->registrar().IsInstalled(web_app_url);
 }
 
 bool ChromeManagementAPIDelegate::CanContextInstallWebApps(

diff --git a/chrome/browser/extensions/api/management/management_apitest.cc b/chrome/browser/extensions/api/management/management_apitest.cc
@@ -16,7 +16,8 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
+#include "chrome/browser/web_applications/components/app_registrar.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "extensions/browser/api/management/management_api.h"
@@ -258,12 +259,13 @@ IN_PROC_BROWSER_TEST_F(InstallReplacementWebAppApiTest, InstallableWebApp) {
 
   chrome::SetAutoAcceptPWAInstallConfirmationForTesting(true);
   const GURL good_web_app_url = https_test_server_.GetURL(kGoodWebAppURL);
-  EXPECT_FALSE(extensions::BookmarkOrHostedAppInstalled(browser()->profile(),
-                                                        good_web_app_url));
+
+  auto* provider =
+      web_app::WebAppProviderBase::GetProviderBase(browser()->profile());
+  EXPECT_FALSE(provider->registrar().IsInstalled(good_web_app_url));
 
   RunTest(kGoodWebAppURL, kBackground, true /* from_webstore */);
-  EXPECT_TRUE(extensions::BookmarkOrHostedAppInstalled(browser()->profile(),
-                                                       good_web_app_url));
+  EXPECT_TRUE(provider->registrar().IsInstalled(good_web_app_url));
   chrome::SetAutoAcceptPWAInstallConfirmationForTesting(false);
 }
 

diff --git a/chrome/browser/web_applications/components/app_registrar.h b/chrome/browser/web_applications/components/app_registrar.h
@@ -8,6 +8,8 @@
 #include "base/callback_forward.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 
+class GURL;
+
 namespace web_app {
 
 class AppRegistrar {
@@ -16,6 +18,12 @@ class AppRegistrar {
 
   virtual void Init(base::OnceClosure callback) = 0;
 
+  // Returns true if the app with the specified |start_url| is currently fully
+  // locally installed. The provided |start_url| must exactly match the launch
+  // URL for the app; this method does not consult the app scope or match URLs
+  // that fall within the scope.
+  virtual bool IsInstalled(const GURL& start_url) const = 0;
+
   // Returns true if the app with |app_id| is currently installed.
   virtual bool IsInstalled(const AppId& app_id) const = 0;
 

diff --git a/chrome/browser/web_applications/extensions/bookmark_app_registrar.cc b/chrome/browser/web_applications/extensions/bookmark_app_registrar.cc
@@ -9,10 +9,14 @@
 #include "base/one_shot_event.h"
 #include "chrome/browser/extensions/convert_web_app.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
 #include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
+#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/common/extension.h"
+#include "url/gurl.h"
 
 namespace extensions {
 
@@ -25,6 +29,27 @@ void BookmarkAppRegistrar::Init(base::OnceClosure callback) {
   ExtensionSystem::Get(profile_)->ready().Post(FROM_HERE, std::move(callback));
 }
 
+bool BookmarkAppRegistrar::IsInstalled(const GURL& start_url) const {
+  ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
+  const ExtensionSet& extensions = registry->enabled_extensions();
+
+  // Iterate through the extensions and extract the LaunchWebUrl (bookmark apps)
+  // or check the web extent (hosted apps).
+  for (const scoped_refptr<const Extension>& extension : extensions) {
+    if (!extension->is_hosted_app())
+      continue;
+
+    if (!BookmarkAppIsLocallyInstalled(profile_, extension.get()))
+      continue;
+
+    if (extension->web_extent().MatchesURL(start_url) ||
+        AppLaunchInfo::GetLaunchWebURL(extension.get()) == start_url) {
+      return true;
+    }
+  }
+  return false;
+}
+
 bool BookmarkAppRegistrar::IsInstalled(const web_app::AppId& app_id) const {
   return GetExtension(app_id) != nullptr;
 }

diff --git a/chrome/browser/web_applications/extensions/bookmark_app_registrar.h b/chrome/browser/web_applications/extensions/bookmark_app_registrar.h
@@ -21,6 +21,7 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar {
 
   // AppRegistrar
   void Init(base::OnceClosure callback) override;
+  bool IsInstalled(const GURL& start_url) const override;
   bool IsInstalled(const web_app::AppId& app_id) const override;
   bool WasExternalAppUninstalledByUser(
       const web_app::AppId& app_id) const override;

diff --git a/chrome/browser/web_applications/extensions/bookmark_app_util.cc b/chrome/browser/web_applications/extensions/bookmark_app_util.cc
@@ -55,28 +55,6 @@ bool BookmarkAppIsLocallyInstalled(const ExtensionPrefs* prefs,
   return true;
 }
 
-bool BookmarkOrHostedAppInstalled(content::BrowserContext* browser_context,
-                                  const GURL& url) {
-  ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context);
-  const ExtensionSet& extensions = registry->enabled_extensions();
-
-  // Iterate through the extensions and extract the LaunchWebUrl (bookmark apps)
-  // or check the web extent (hosted apps).
-  for (const scoped_refptr<const Extension>& extension : extensions) {
-    if (!extension->is_hosted_app())
-      continue;
-
-    if (!BookmarkAppIsLocallyInstalled(browser_context, extension.get()))
-      continue;
-
-    if (extension->web_extent().MatchesURL(url) ||
-        AppLaunchInfo::GetLaunchWebURL(extension.get()) == url) {
-      return true;
-    }
-  }
-  return false;
-}
-
 bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url) {
   // Drop any "suffix" components after the path (Resolve "."):
   const GURL nav_scope = launch_url.GetWithoutFilename();

diff --git a/chrome/browser/web_applications/extensions/bookmark_app_util.h b/chrome/browser/web_applications/extensions/bookmark_app_util.h
@@ -34,11 +34,6 @@ bool BookmarkAppIsLocallyInstalled(content::BrowserContext* context,
 bool BookmarkAppIsLocallyInstalled(const ExtensionPrefs* prefs,
                                    const Extension* extension);
 
-// Returns true if a bookmark or hosted app from a given URL is already
-// installed and enabled.
-bool BookmarkOrHostedAppInstalled(content::BrowserContext* browser_context,
-                                  const GURL& url);
-
 // Generates a scope based on |launch_url| and checks if the |url| falls under
 // it. https://www.w3.org/TR/appmanifest/#navigation-scope
 bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url);

diff --git a/chrome/browser/web_applications/test/test_app_registrar.cc b/chrome/browser/web_applications/test/test_app_registrar.cc
@@ -30,6 +30,11 @@ void TestAppRegistrar::AddAsExternalAppUninstalledByUser(const AppId& app_id) {
 
 void TestAppRegistrar::Init(base::OnceClosure callback) {}
 
+bool TestAppRegistrar::IsInstalled(const GURL& start_url) const {
+  NOTIMPLEMENTED();
+  return false;
+}
+
 bool TestAppRegistrar::IsInstalled(const AppId& app_id) const {
   return base::ContainsKey(installed_apps_, app_id);
 }

diff --git a/chrome/browser/web_applications/test/test_app_registrar.h b/chrome/browser/web_applications/test/test_app_registrar.h
@@ -28,6 +28,7 @@ class TestAppRegistrar : public AppRegistrar {
 
   // AppRegistrar
   void Init(base::OnceClosure callback) override;
+  bool IsInstalled(const GURL& start_url) const override;
   bool IsInstalled(const AppId& app_id) const override;
   bool WasExternalAppUninstalledByUser(const AppId& app_id) const override;
   bool HasScopeUrl(const AppId& app_id) const override;

diff --git a/chrome/browser/web_applications/web_app_registrar.cc b/chrome/browser/web_applications/web_app_registrar.cc
@@ -74,6 +74,11 @@ void WebAppRegistrar::OnDatabaseOpened(base::OnceClosure callback,
   std::move(callback).Run();
 }
 
+bool WebAppRegistrar::IsInstalled(const GURL& start_url) const {
+  NOTIMPLEMENTED();
+  return false;
+}
+
 bool WebAppRegistrar::IsInstalled(const AppId& app_id) const {
   return GetAppById(app_id) != nullptr;
 }

diff --git a/chrome/browser/web_applications/web_app_registrar.h b/chrome/browser/web_applications/web_app_registrar.h
@@ -37,6 +37,7 @@ class WebAppRegistrar : public AppRegistrar {
 
   // AppRegistrar
   void Init(base::OnceClosure callback) override;
+  bool IsInstalled(const GURL& start_url) const override;
   bool IsInstalled(const AppId& app_id) const override;
   bool WasExternalAppUninstalledByUser(const AppId& app_id) const override;
   bool HasScopeUrl(const AppId& app_id) const override;