desktop-pwas: Move PendingAppManager to be owned by WebAppProvider

Moves the ownership of PendingAppManager up to WebAppProvider.
PendingAppManager will be used by any client that wishes to install a
WebApp. For now this is only WebAppPolicyManager and
ScanForExternalApps.

Bug: 864904
Change-Id: I0599c896accdd74ceabfc16bf403d4eec501c97e
Reviewed-on: https://chromium-review.googlesource.com/1154744
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579277}

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

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
 
-#include <utility>
 #include <vector>
 
 #include "base/values.h"
@@ -15,19 +14,12 @@
 
 namespace web_app {
 
-WebAppPolicyManager::WebAppPolicyManager(
-    PrefService* pref_service,
-    std::unique_ptr<PendingAppManager> pending_app_manager)
-    : pref_service_(pref_service),
-      pending_app_manager_(std::move(pending_app_manager)) {
+WebAppPolicyManager::WebAppPolicyManager(PrefService* pref_service,
+                                         PendingAppManager* pending_app_manager)
+    : pref_service_(pref_service), pending_app_manager_(pending_app_manager) {
   pending_app_manager_->ProcessAppOperations(GetAppsToInstall());
 }
 
-WebAppPolicyManager::WebAppPolicyManager(PrefService* pref_service)
-    : WebAppPolicyManager(
-          pref_service,
-          std::make_unique<extensions::PendingBookmarkAppManager>()) {}
-
 WebAppPolicyManager::~WebAppPolicyManager() = default;
 
 std::vector<PendingAppManager::AppInfo>

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

@@ -5,7 +5,6 @@
 #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"
@@ -22,26 +21,19 @@ namespace web_app {
 class WebAppPolicyManager {
  public:
   // Constructs a WebAppPolicyManager instance that uses
-  // extensions::PendingBookmarkAppManager to manage apps.
-  explicit WebAppPolicyManager(PrefService* pref_service);
-
-  // Constructs a WebAppPolicyManager instance that uses |pending_app_manager|
-  // to manage apps.
-  explicit WebAppPolicyManager(
-      PrefService* pref_service,
-      std::unique_ptr<PendingAppManager> pending_app_manager);
-
+  // |pending_app_manager| to manage apps. |pending_app_manager| should outlive
+  // this class.
+  explicit WebAppPolicyManager(PrefService* pref_service,
+                               PendingAppManager* pending_app_manager);
   ~WebAppPolicyManager();
 
-  const PendingAppManager& pending_app_manager() {
-    return *pending_app_manager_;
-  }
-
  private:
   std::vector<PendingAppManager::AppInfo> GetAppsToInstall();
 
   PrefService* pref_service_;
-  std::unique_ptr<PendingAppManager> pending_app_manager_;
+
+  // Used to install, uninstall, and update apps. Should outlive this class.
+  PendingAppManager* pending_app_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(WebAppPolicyManager);
 };

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

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h"
 
+#include <memory>
 #include <utility>
 #include <vector>
 
@@ -57,11 +58,9 @@ TEST_F(WebAppPolicyManagerTest, NoForceInstalledAppsPrefValue) {
   RegisterUserProfilePrefs(prefs->registry());
 
   auto pending_app_manager = std::make_unique<TestPendingAppManager>();
-  auto* pending_app_manager_ptr = pending_app_manager.get();
-
   WebAppPolicyManager web_app_policy_manager(prefs.get(),
-                                             std::move(pending_app_manager));
-  const auto& apps_to_install = pending_app_manager_ptr->last_apps_to_install();
+                                             pending_app_manager.get());
+  const auto& apps_to_install = pending_app_manager->last_apps_to_install();
 
   EXPECT_TRUE(apps_to_install.empty());
 }
@@ -74,11 +73,9 @@ TEST_F(WebAppPolicyManagerTest, NoForceInstalledApps) {
              base::Value(base::Value::Type::LIST));
 
   auto pending_app_manager = std::make_unique<TestPendingAppManager>();
-  auto* pending_app_manager_ptr = pending_app_manager.get();
-
   WebAppPolicyManager web_app_policy_manager(prefs.get(),
-                                             std::move(pending_app_manager));
-  const auto& apps_to_install = pending_app_manager_ptr->last_apps_to_install();
+                                             pending_app_manager.get());
+  const auto& apps_to_install = pending_app_manager->last_apps_to_install();
 
   EXPECT_TRUE(apps_to_install.empty());
 }
@@ -105,11 +102,9 @@ TEST_F(WebAppPolicyManagerTest, TwoForceInstalledApps) {
   }
 
   auto pending_app_manager = std::make_unique<TestPendingAppManager>();
-  auto* pending_app_manager_ptr = pending_app_manager.get();
-
   WebAppPolicyManager web_app_policy_manager(prefs.get(),
-                                             std::move(pending_app_manager));
-  const auto& apps_to_install = pending_app_manager_ptr->last_apps_to_install();
+                                             pending_app_manager.get());
+  const auto& apps_to_install = pending_app_manager->last_apps_to_install();
 
   std::vector<PendingAppManager::AppInfo> expected_apps_to_install;
   expected_apps_to_install.emplace_back(

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h

@@ -15,7 +15,7 @@ namespace extensions {
 // Implementation of web_app::PendingAppManager that manages the set of
 // Bookmark Apps which are being installed, uninstalled, and updated.
 //
-// WebAppPolicyManager creates an instance of this class and manages its
+// WebAppProvider creates an instance of this class and manages its
 // lifetime. This class should only be used from the UI thread.
 class PendingBookmarkAppManager final : public web_app::PendingAppManager {
  public:

chrome/browser/web_applications/web_app_provider.cc

@@ -4,7 +4,10 @@
 
 #include "chrome/browser/web_applications/web_app_provider.h"
 
+#include <utility>
+
 #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/web_app_provider_factory.h"
 
 namespace web_app {
@@ -15,8 +18,11 @@ WebAppProvider* WebAppProvider::Get(Profile* profile) {
 }
 
 WebAppProvider::WebAppProvider(PrefService* pref_service)
-    : web_app_policy_manager_(
-          std::make_unique<WebAppPolicyManager>(pref_service)) {
+    : pending_app_manager_(
+          std::make_unique<extensions::PendingBookmarkAppManager>()),
+      web_app_policy_manager_(
+          std::make_unique<WebAppPolicyManager>(pref_service,
+                                                pending_app_manager_.get())) {
   // TODO(nigeltao): install default web apps as per http://crbug.com/855281
 }
 

chrome/browser/web_applications/web_app_provider.h

@@ -15,6 +15,7 @@ class Profile;
 
 namespace web_app {
 
+class PendingAppManager;
 class WebAppPolicyManager;
 
 // Connects Web App features, such as the installation of default and
@@ -26,9 +27,14 @@ class WebAppProvider : public KeyedService {
 
   explicit WebAppProvider(PrefService* pref_service);
 
+  // Clients can use PendingAppManager to install, uninstall, and update
+  // Web Apps.
+  PendingAppManager& pending_app_manager() { return *pending_app_manager_; }
+
   ~WebAppProvider() override;
 
  private:
+  std::unique_ptr<PendingAppManager> pending_app_manager_;
   std::unique_ptr<WebAppPolicyManager> web_app_policy_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(WebAppProvider);