[WebAppProvider] Create WebAppMover class, feature, params, and tests

This change creates the basic WebAppMover class, but the functionality
is currently a NOOP. However it is hooked in correctly, and the
implementation would start where marked.

Unittests are included to test the initial sanity checking of the
source and destination URLs.

Follow up patches will include the implementation and thorough
browsertests.

Bug: 1158066
Change-Id: I04b0a3971932f30c4bbb3b8605f84dc0ea64be99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587825
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Auto-Submit: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Chase Phillips <cmp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838147}

chrome/browser/web_applications/BUILD.gn

@@ -53,6 +53,8 @@ source_set("web_applications") {
     "web_app_installation_utils.cc",
     "web_app_installation_utils.h",
     "web_app_migration_manager.h",
+    "web_app_mover.cc",
+    "web_app_mover.h",
     "web_app_proto_utils.cc",
     "web_app_proto_utils.h",
     "web_app_protocol_handler_manager.cc",
@@ -203,6 +205,7 @@ source_set("web_applications_unit_tests") {
     "web_app_install_manager_unittest.cc",
     "web_app_install_task_unittest.cc",
     "web_app_installation_utils_unittest.cc",
+    "web_app_mover_unittest.cc",
     "web_app_proto_utils_unittest.cc",
     "web_app_registrar_unittest.cc",
     "web_app_sync_bridge_unittest.cc",

chrome/browser/web_applications/web_app_mover.cc

+// Copyright 2020 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_mover.h"
+
+#include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/no_destructor.h"
+#include "base/strings/string_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/common/chrome_features.h"
+
+namespace {
+
+bool g_disabled_for_testing = false;
+bool g_skip_wait_for_sync_for_testing = false;
+
+base::OnceClosure& GetCompletedCallbackForTesting() {
+  static base::NoDestructor<base::OnceClosure> callback;
+  return *callback;
+}
+
+}  // namespace
+
+namespace web_app {
+
+std::unique_ptr<WebAppMover> WebAppMover::CreateIfNeeded(Profile* profile) {
+  DCHECK(base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions));
+
+  if (g_disabled_for_testing)
+    return nullptr;
+
+  if (!base::FeatureList::IsEnabled(features::kMoveWebApp))
+    return nullptr;
+
+  std::string uninstall_url_prefix_str =
+      features::kMoveWebAppUninstallStartUrlPrefix.Get();
+  std::string install_url_str = features::kMoveWebAppInstallStartUrl.Get();
+  if (uninstall_url_prefix_str.empty() || install_url_str.empty())
+    return nullptr;
+
+  GURL uninstall_url_prefix = GURL(uninstall_url_prefix_str);
+  GURL install_url = GURL(install_url_str);
+  // The URLs have to be valid, and the installation URL cannot be contained in
+  // the uninstall prefix.
+  if (!uninstall_url_prefix.is_valid() || !install_url.is_valid() ||
+      base::StartsWith(install_url.spec(), uninstall_url_prefix.spec())) {
+    return nullptr;
+  }
+
+  return std::make_unique<WebAppMover>(profile, uninstall_url_prefix,
+                                       install_url);
+}
+
+void WebAppMover::DisableForTesting() {
+  g_disabled_for_testing = true;
+}
+
+void WebAppMover::SkipWaitForSyncForTesting() {
+  g_skip_wait_for_sync_for_testing = true;
+}
+
+void WebAppMover::SetCompletedCallbackForTesting(base::OnceClosure callback) {
+  GetCompletedCallbackForTesting() = std::move(callback);
+}
+
+WebAppMover::WebAppMover(Profile* profile,
+                         const GURL& uninstall_url_prefix,
+                         const GURL& install_url)
+    : profile_(profile),
+      uninstall_url_prefix_(uninstall_url_prefix),
+      install_url_(install_url) {}
+
+WebAppMover::~WebAppMover() = default;
+
+void WebAppMover::Start() {
+  // We cannot grab the SyncService in the constructor without creating a
+  // circular KeyedService dependency.
+  sync_service_ = ProfileSyncServiceFactory::GetForProfile(profile_);
+  // This can be a nullptr if the --disable-sync switch is specified.
+  if (sync_service_)
+    sync_observer_.Observe(sync_service_);
+  // We must wait for sync to complete at least one cycle (if it is turned on).
+  // This avoids our local updates accidentally re-installing any web apps that
+  // were uninstalled on other devices. Installing the replacement app will send
+  // that record to sync servers, and if the user had uninstalled the 'source'
+  // app on another computer, we could miss that message and accidentally end up
+  // with the 'destination' app installed when it shouldn't have been installed
+  // in the first place (as the user uninstalled the 'source' app).
+  WaitForFirstSyncCycle(base::BindOnce(&WebAppMover::OnFirstSyncCycleComplete,
+                                       weak_ptr_factory_.GetWeakPtr()));
+}
+
+void WebAppMover::Shutdown() {
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  sync_observer_.Reset();
+}
+
+void WebAppMover::OnSyncCycleCompleted(syncer::SyncService* sync_service) {
+  DCHECK_EQ(sync_service_, sync_service);
+  if (sync_ready_callback_)
+    std::move(sync_ready_callback_).Run();
+}
+
+void WebAppMover::OnSyncShutdown(syncer::SyncService* sync_service) {
+  DCHECK_EQ(sync_service_, sync_service);
+  sync_observer_.Reset();
+  sync_service_ = nullptr;
+}
+
+void WebAppMover::WaitForFirstSyncCycle(base::OnceClosure callback) {
+  DCHECK(!sync_ready_callback_);
+  if (g_skip_wait_for_sync_for_testing || !sync_service_ ||
+      sync_service_->HasCompletedSyncCycle() ||
+      !sync_service_->IsSyncFeatureEnabled()) {
+    std::move(callback).Run();
+    return;
+  }
+  sync_ready_callback_ = std::move(callback);
+}
+
+void WebAppMover::OnFirstSyncCycleComplete() {
+  // TODO(dmurph): Implement migration here.
+
+  if (GetCompletedCallbackForTesting())
+    std::move(GetCompletedCallbackForTesting()).Run();
+}
+
+}  // namespace web_app
\ No newline at end of file

chrome/browser/web_applications/web_app_mover.h

+// Copyright 2020 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_MOVER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_MOVER_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "base/scoped_observation.h"
+#include "components/sync/driver/sync_service.h"
+#include "components/sync/driver/sync_service_observer.h"
+#include "url/gurl.h"
+
+class Profile;
+
+namespace web_app {
+
+// WebAppMover is designed to facilitate a one-off migration for a webapp, from
+// one start_url to another.
+// TODO(dmurph): Finish implementing.
+class WebAppMover final : public syncer::SyncServiceObserver {
+ public:
+  static std::unique_ptr<WebAppMover> CreateIfNeeded(Profile* profile);
+
+  static void DisableForTesting();
+  static void SkipWaitForSyncForTesting();
+  static void SetCompletedCallbackForTesting(base::OnceClosure callback);
+
+  WebAppMover(Profile* profile,
+              const GURL& uninstall_url_prefix,
+              const GURL& install_url);
+  WebAppMover(const WebAppMover&) = delete;
+  WebAppMover& operator=(const WebAppMover&) = delete;
+  ~WebAppMover() final;
+
+  void Start();
+  void Shutdown();
+
+  // syncer::SyncServiceObserver:
+  void OnSyncCycleCompleted(syncer::SyncService* sync_service) final;
+  void OnSyncShutdown(syncer::SyncService* sync_service) final;
+
+ private:
+  void WaitForFirstSyncCycle(base::OnceClosure callback);
+  void OnFirstSyncCycleComplete();
+
+  Profile* profile_;
+  GURL uninstall_url_prefix_;
+  GURL install_url_;
+  syncer::SyncService* sync_service_ = nullptr;
+  base::OnceClosure sync_ready_callback_;
+
+  base::ScopedObservation<syncer::SyncService, syncer::SyncServiceObserver>
+      sync_observer_{this};
+
+  base::WeakPtrFactory<WebAppMover> weak_ptr_factory_{this};
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_MOVER_H_
\ No newline at end of file

chrome/browser/web_applications/web_app_mover_unittest.cc

+// Copyright 2020 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_mover.h"
+
+#include "base/metrics/field_trial_params.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/common/chrome_features.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace web_app {
+namespace {
+
+class WebAppMoverTestWithParams : public ::testing::Test,
+                                  public ::testing::WithParamInterface<
+                                      std::pair<const char*, const char*>> {
+ public:
+  WebAppMoverTestWithParams() {
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kMoveWebApp,
+          {{features::kMoveWebAppUninstallStartUrlPrefix.name,
+            GetParam().first},
+           {features::kMoveWebAppInstallStartUrl.name, GetParam().second}}}},
+        {});
+  }
+  ~WebAppMoverTestWithParams() override = default;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+using WebAppMoverTestWithInvalidParams = WebAppMoverTestWithParams;
+
+TEST_P(WebAppMoverTestWithInvalidParams, VerifyInvalidParams) {
+  std::unique_ptr<WebAppMover> mover = WebAppMover::CreateIfNeeded(nullptr);
+  EXPECT_FALSE(mover);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    InvalidInputs,
+    WebAppMoverTestWithInvalidParams,
+    ::testing::Values(
+        std::make_pair("", ""),
+        std::make_pair("test", "test"),
+        std::make_pair("www.google.com/a", "www.google.com/b"),
+        std::make_pair("https://www.google.com/a", "https://www.google.com/a"),
+        std::make_pair("https://www.google.com/", "https://www.google.com/a"),
+        std::make_pair("https://www.google.com/foo",
+                       "https://www.google.com/foobar")));
+
+using WebAppMoverTestWithValidParams = WebAppMoverTestWithParams;
+
+TEST_P(WebAppMoverTestWithValidParams, VerifyValidParams) {
+  std::unique_ptr<WebAppMover> mover = WebAppMover::CreateIfNeeded(nullptr);
+  EXPECT_TRUE(mover);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    ValidInputs,
+    WebAppMoverTestWithValidParams,
+    ::testing::Values(std::make_pair("https://www.google.com/a",
+                                     "https://www.google.com/b")));
+
+}  // namespace
+}  // namespace web_app

chrome/browser/web_applications/web_app_provider.cc

@@ -29,6 +29,7 @@
 #include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_install_manager.h"
 #include "chrome/browser/web_applications/web_app_migration_manager.h"
+#include "chrome/browser/web_applications/web_app_mover.h"
 #include "chrome/browser/web_applications/web_app_protocol_handler_manager.h"
 #include "chrome/browser/web_applications/web_app_provider_factory.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
@@ -161,6 +162,8 @@ void WebAppProvider::Shutdown() {
   icon_manager_->Shutdown();
   install_finalizer_->Shutdown();
   registrar_->Shutdown();
+  if (web_app_mover_)
+    web_app_mover_->Shutdown();
 }
 
 void WebAppProvider::StartImpl() {
@@ -235,6 +238,7 @@ void WebAppProvider::CreateWebAppsSubsystems(Profile* profile) {
   migration_manager_ = std::make_unique<WebAppMigrationManager>(
       profile, database_factory_.get(), icon_manager.get(),
       os_integration_manager_.get());
+  web_app_mover_ = WebAppMover::CreateIfNeeded(profile);
 
   // Upcast to unified subsystem types:
   registrar_ = std::move(registrar);
@@ -290,6 +294,8 @@ void WebAppProvider::OnRegistryControllerReady() {
   manifest_update_manager_->Start();
   os_integration_manager_->Start();
   ui_manager_->Start();
+  if (web_app_mover_)
+    web_app_mover_->Start();
 
   on_registry_ready_.Signal();
 }

chrome/browser/web_applications/web_app_provider.h

@@ -43,6 +43,7 @@ class OsIntegrationManager;
 // Forward declarations for new extension-independent subsystems.
 class WebAppDatabaseFactory;
 class WebAppMigrationManager;
+class WebAppMover;
 
 // Connects Web App features, such as the installation of default and
 // policy-managed web apps, with Profiles (as WebAppProvider is a
@@ -127,6 +128,7 @@ class WebAppProvider : public WebAppProviderBase {
   std::unique_ptr<WebAppDatabaseFactory> database_factory_;
   // migration_manager_ can be nullptr if no migration needed.
   std::unique_ptr<WebAppMigrationManager> migration_manager_;
+  std::unique_ptr<WebAppMover> web_app_mover_;
 
   // Generalized subsystems:
   std::unique_ptr<AppRegistrar> registrar_;

chrome/common/chrome_features.cc

@@ -616,6 +616,15 @@ const base::Feature kMetricsSettingsAndroid{"MetricsSettingsAndroid",
                                             base::FEATURE_DISABLED_BY_DEFAULT};
 #endif
 
+const base::Feature kMoveWebApp{
+    "MoveWebApp", base::FeatureState::FEATURE_DISABLED_BY_DEFAULT};
+const base::FeatureParam<std::string> kMoveWebAppUninstallStartUrlPrefix(
+    &kMoveWebApp,
+    "uninstallStartUrlPrefix",
+    "");
+const base::FeatureParam<std::string>
+    kMoveWebAppInstallStartUrl(&kMoveWebApp, "installStartUrl", "");
+
 // Enables the use of native notification centers instead of using the Message
 // Center for displaying the toasts. The feature is hardcoded to enabled for
 // Chrome OS.

chrome/common/chrome_features.h

@@ -407,6 +407,13 @@ COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kMetricsSettingsAndroid;
 #endif
 
+COMPONENT_EXPORT(CHROME_FEATURES)
+extern const base::Feature kMoveWebApp;
+COMPONENT_EXPORT(CHROME_FEATURES)
+extern const base::FeatureParam<std::string> kMoveWebAppUninstallStartUrlPrefix;
+COMPONENT_EXPORT(CHROME_FEATURES)
+extern const base::FeatureParam<std::string> kMoveWebAppInstallStartUrl;
+
 #if BUILDFLAG(ENABLE_NATIVE_NOTIFICATIONS)
 COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kNativeNotifications;