Factor WebAppProvider out of WebAppPolicyManager

That keyed service will, in the future, be responsible for more web app
things than just policy.

Also move c/b/wa/policy to c/b/wa/bookmark_apps/policy, where c/b/wa
means chrome/browser/web_applications. We (nigeltao@, ortuno@, loyso@,
dominickn@) want to organize c/b/wa into a 'root' directory and three
or four sub-directories as we de-tangle web apps from extensions:
- c/b/wa holds only the WebAppProvider and its Factory.
- c/b/wa/extensions is code that is tightly coupled to extensions.
- c/b/wa/bookmark_apps is code that currently depends on
  c/b/wa/extensions but, in the future, should not.
- c/b/wa/components is low level code that is unrelated to extensions.
- c/b/wa/new_bookmark_apps (name pending) is high level code that is
  unrelated to extensions.

Change-Id: Ie42bfc348be7166c710fc6232848b5e9eb91960e
Reviewed-on: https://chromium-review.googlesource.com/1142844
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576500}

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -144,7 +144,7 @@
 #include "chrome/browser/extensions/browser_context_keyed_service_factories.h"
 #include "chrome/browser/extensions/extension_management.h"
 #include "chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h"
-#include "chrome/browser/web_applications/policy/web_app_policy_manager_factory.h"
+#include "chrome/browser/web_applications/web_app_provider_factory.h"
 #include "extensions/browser/api/networking_private/networking_private_delegate_factory.h"
 #include "extensions/browser/browser_context_keyed_service_factories.h"
 #if defined(OS_CHROMEOS)
@@ -199,7 +199,6 @@ void ChromeBrowserMainExtraPartsProfiles::
   extensions::ExtensionManagementFactory::GetInstance();
   chrome_extensions::EnsureBrowserContextKeyedServiceFactoriesBuilt();
   chrome_apps::EnsureBrowserContextKeyedServiceFactoriesBuilt();
-  web_app::WebAppPolicyManagerFactory::GetInstance();
 #endif
 
 #if BUILDFLAG(ENABLE_APP_LIST)
@@ -381,6 +380,9 @@ void ChromeBrowserMainExtraPartsProfiles::
   UrlLanguageHistogramFactory::GetInstance();
 #if !defined(OS_ANDROID)
   UsbChooserContextFactory::GetInstance();
+#endif
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  web_app::WebAppProviderFactory::GetInstance();
 #endif
   WebDataServiceFactory::GetInstance();
 }

chrome/browser/web_applications/BUILD.gn

@@ -6,6 +6,12 @@ import("//extensions/buildflags/buildflags.gni")
 
 source_set("web_applications") {
   sources = [
+    "web_app_provider.cc",
+    "web_app_provider.h",
+    "web_app_provider_factory.cc",
+    "web_app_provider_factory.h",
+
+    # TODO(loyso): move this code (and everything other than "KEEP") elsewhere.
     "web_app.cc",
     "web_app.h",
     "web_app_chromeos.cc",
@@ -26,11 +32,15 @@ source_set("web_applications") {
   allow_circular_includes_from = [ "//chrome/browser/extensions" ]
 
   deps = [
+    # TODO(loyso): KEEP.
+    "//chrome/browser/web_applications/bookmark_apps",
+    "//chrome/common",
+    "//skia",
+
+    # TODO(loyso): move this code elsewhere.
     "//base",
     "//chrome/browser/extensions",
-    "//chrome/browser/web_applications/bookmark_apps",
     "//chrome/browser/web_applications/components",
-    "//chrome/common",
     "//components/keyed_service/content",
     "//components/prefs",
     "//extensions/browser",

chrome/browser/web_applications/bookmark_apps/BUILD.gn

@@ -7,17 +7,30 @@ import("//extensions/buildflags/buildflags.gni")
 assert(enable_extensions)
 
 source_set("bookmark_apps") {
+  sources = [
+    "policy/web_app_policy_constants.cc",
+    "policy/web_app_policy_constants.h",
+    "policy/web_app_policy_manager.cc",
+    "policy/web_app_policy_manager.h",
+  ]
+
   deps = [
-    # TODO(ortuno): Move policy to bookmark_apps/policy
-    "//chrome/browser/web_applications/policy",
+    "//chrome/browser/extensions",
+    "//chrome/common",
+    "//skia",
   ]
 }
 
 source_set("unit_tests") {
   testonly = true
 
+  sources = [
+    "policy/web_app_policy_manager_unittest.cc",
+  ]
+
   deps = [
-    # TODO(ortuno): Move policy to bookmark_apps/policy
-    "//chrome/browser/web_applications/policy:unit_tests",
+    ":bookmark_apps",
+    "//skia",
+    "//testing/gmock",
   ]
 }

chrome/browser/web_applications/policy/web_app_policy_constants.cc → chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_constants.cc

@@ -2,7 +2,7 @@
 // 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/policy/web_app_policy_constants.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_constants.h"
 
 namespace web_app {
 

chrome/browser/web_applications/policy/web_app_policy_constants.h → chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_constants.h

@@ -2,8 +2,8 @@
 // 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_POLICY_WEB_APP_POLICY_CONSTANTS_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_CONSTANTS_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_CONSTANTS_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_CONSTANTS_H_
 
 namespace web_app {
 
@@ -15,4 +15,4 @@ extern const char kLaunchContainerTabValue[];
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_CONSTANTS_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_CONSTANTS_H_

chrome/browser/web_applications/policy/web_app_policy_manager.cc → chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.cc

@@ -2,14 +2,14 @@
 // 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/policy/web_app_policy_manager.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/values.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_constants.h"
 #include "chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h"
-#include "chrome/browser/web_applications/policy/web_app_policy_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 

chrome/browser/web_applications/policy/web_app_policy_manager.h → chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h

@@ -2,14 +2,13 @@
 // 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_POLICY_WEB_APP_POLICY_MANAGER_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_MANAGER_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_MANAGER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_MANAGER_H_
 
 #include <memory>
 #include <vector>
 
 #include "base/macros.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "url/gurl.h"
 
 class PrefService;
@@ -22,7 +21,7 @@ namespace web_app {
 // which apps need to be installed, uninstalled, and updated. It uses
 // WebAppPolicyManager::PendingAppManager to actually install, uninstall,
 // and update apps.
-class WebAppPolicyManager : public KeyedService {
+class WebAppPolicyManager {
  public:
   class PendingAppManager;
 
@@ -55,7 +54,7 @@ class WebAppPolicyManager : public KeyedService {
       PrefService* pref_service,
       std::unique_ptr<PendingAppManager> pending_app_manager);
 
-  ~WebAppPolicyManager() override;
+  ~WebAppPolicyManager();
 
   const PendingAppManager& pending_app_manager() {
     return *pending_app_manager_;
@@ -89,4 +88,4 @@ class WebAppPolicyManager::PendingAppManager {
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_MANAGER_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_BOOKMARK_APPS_POLICY_WEB_APP_POLICY_MANAGER_H_

chrome/browser/web_applications/policy/web_app_policy_manager_unittest.cc → chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager_unittest.cc

@@ -2,14 +2,14 @@
 // 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/policy/web_app_policy_manager.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/values.h"
 #include "chrome/browser/prefs/browser_prefs.h"
-#include "chrome/browser/web_applications/policy/web_app_policy_constants.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "testing/gmock/include/gmock/gmock.h"

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h

@@ -8,7 +8,7 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "chrome/browser/web_applications/policy/web_app_policy_manager.h"
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
 
 namespace extensions {
 

chrome/browser/web_applications/policy/BUILD.gn

-# 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.
-
-import("//extensions/buildflags/buildflags.gni")
-
-assert(enable_extensions)
-
-source_set("policy") {
-  sources = [
-    "web_app_policy_constants.cc",
-    "web_app_policy_constants.h",
-    "web_app_policy_manager.cc",
-    "web_app_policy_manager.h",
-    "web_app_policy_manager_factory.cc",
-    "web_app_policy_manager_factory.h",
-  ]
-
-  deps = [
-    "//chrome/browser/extensions",
-    "//chrome/common",
-    "//components/keyed_service/content",
-    "//skia",
-  ]
-}
-
-source_set("unit_tests") {
-  testonly = true
-
-  sources = [
-    "web_app_policy_manager_unittest.cc",
-  ]
-
-  deps = [
-    ":policy",
-    "//skia",
-    "//testing/gmock",
-  ]
-}

chrome/browser/web_applications/web_app_provider.cc

+// 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/web_app_provider.h"
+
+#include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
+#include "chrome/browser/web_applications/web_app_provider_factory.h"
+
+namespace web_app {
+
+// static
+WebAppProvider* WebAppProvider::Get(Profile* profile) {
+  return WebAppProviderFactory::GetForProfile(profile);
+}
+
+WebAppProvider::WebAppProvider(PrefService* pref_service)
+    : web_app_policy_manager_(
+          std::make_unique<WebAppPolicyManager>(pref_service)) {
+  // TODO(nigeltao): install default web apps as per http://crbug.com/855281
+}
+
+WebAppProvider::~WebAppProvider() = default;
+
+}  // namespace web_app

chrome/browser/web_applications/web_app_provider.h

+// 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_WEB_APP_PROVIDER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_PROVIDER_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/prefs/pref_service.h"
+
+class Profile;
+
+namespace web_app {
+
+class WebAppPolicyManager;
+
+// Connects Web App features, such as the installation of default and
+// policy-managed web apps, with Profiles (as WebAppProvider is a
+// Profile-linked KeyedService) and their associated PrefService.
+class WebAppProvider : public KeyedService {
+ public:
+  static WebAppProvider* Get(Profile* profile);
+
+  explicit WebAppProvider(PrefService* pref_service);
+
+  ~WebAppProvider() override;
+
+ private:
+  std::unique_ptr<WebAppPolicyManager> web_app_policy_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebAppProvider);
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_PROVIDER_H_

chrome/browser/web_applications/policy/web_app_policy_manager_factory.cc → chrome/browser/web_applications/web_app_provider_factory.cc

@@ -2,10 +2,10 @@
 // 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/policy/web_app_policy_manager_factory.h"
+#include "chrome/browser/web_applications/web_app_provider_factory.h"
 
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/web_applications/policy/web_app_policy_manager.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "extensions/browser/extension_system_provider.h"
 #include "extensions/browser/extensions_browser_client.h"
@@ -13,35 +13,34 @@
 namespace web_app {
 
 // static
-WebAppPolicyManager* WebAppPolicyManagerFactory::GetForProfile(
-    Profile* profile) {
-  return static_cast<WebAppPolicyManager*>(
-      WebAppPolicyManagerFactory::GetInstance()->GetServiceForBrowserContext(
+WebAppProvider* WebAppProviderFactory::GetForProfile(Profile* profile) {
+  return static_cast<WebAppProvider*>(
+      WebAppProviderFactory::GetInstance()->GetServiceForBrowserContext(
           profile, true /* create */));
 }
 
 // static
-WebAppPolicyManagerFactory* WebAppPolicyManagerFactory::GetInstance() {
-  return base::Singleton<WebAppPolicyManagerFactory>::get();
+WebAppProviderFactory* WebAppProviderFactory::GetInstance() {
+  return base::Singleton<WebAppProviderFactory>::get();
 }
 
-WebAppPolicyManagerFactory::WebAppPolicyManagerFactory()
+WebAppProviderFactory::WebAppProviderFactory()
     : BrowserContextKeyedServiceFactory(
-          "WebAppPolicyManager",
+          "WebAppProvider",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(
       extensions::ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());
 }
 
-WebAppPolicyManagerFactory::~WebAppPolicyManagerFactory() = default;
+WebAppProviderFactory::~WebAppProviderFactory() = default;
 
-KeyedService* WebAppPolicyManagerFactory::BuildServiceInstanceFor(
+KeyedService* WebAppProviderFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
-  return new WebAppPolicyManager(profile->GetPrefs());
+  return new WebAppProvider(profile->GetPrefs());
 }
 
-bool WebAppPolicyManagerFactory::ServiceIsCreatedWithBrowserContext() const {
+bool WebAppProviderFactory::ServiceIsCreatedWithBrowserContext() const {
   return true;
 }
 

chrome/browser/web_applications/policy/web_app_policy_manager_factory.h → chrome/browser/web_applications/web_app_provider_factory.h

@@ -2,8 +2,8 @@
 // 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_POLICY_WEB_APP_POLICY_MANAGER_FACTORY_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_MANAGER_FACTORY_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_PROVIDER_FACTORY_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_PROVIDER_FACTORY_H_
 
 #include "base/macros.h"
 #include "base/memory/singleton.h"
@@ -17,30 +17,30 @@ class Profile;
 
 namespace web_app {
 
-class WebAppPolicyManager;
+class WebAppProvider;
 
-// Singleton that owns all WebAppPolicyManagerFactories and associates them with
+// Singleton that owns all WebAppProviderFactories and associates them with
 // Profile.
-class WebAppPolicyManagerFactory : public BrowserContextKeyedServiceFactory {
+class WebAppProviderFactory : public BrowserContextKeyedServiceFactory {
  public:
-  static WebAppPolicyManager* GetForProfile(Profile* profile);
+  static WebAppProvider* GetForProfile(Profile* profile);
 
-  static WebAppPolicyManagerFactory* GetInstance();
+  static WebAppProviderFactory* GetInstance();
 
  private:
-  friend struct base::DefaultSingletonTraits<WebAppPolicyManagerFactory>;
+  friend struct base::DefaultSingletonTraits<WebAppProviderFactory>;
 
-  WebAppPolicyManagerFactory();
-  ~WebAppPolicyManagerFactory() override;
+  WebAppProviderFactory();
+  ~WebAppProviderFactory() override;
 
   // BrowserContextKeyedServiceFactory
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
   bool ServiceIsCreatedWithBrowserContext() const override;
 
-  DISALLOW_COPY_AND_ASSIGN(WebAppPolicyManagerFactory);
+  DISALLOW_COPY_AND_ASSIGN(WebAppProviderFactory);
 };
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_POLICY_WEB_APP_POLICY_MANAGER_FACTORY_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_PROVIDER_FACTORY_H_