WebApp: Implement WebAppSyncBridge in full.

This code is disabled by default behind kDesktopPWAsWithoutExtensions and
kDesktopPWAsUSS base features.

- Implement WebAppSyncBridge::MergeSyncData and ApplySyncChanges.
- Implement "Local changes" propagation to change_processor.

Unit tests and browser tests will be added later in a follow up CL.
An MVP implementation of the bridge needed first.

This CL follows Chrome Sync's Model API doc
https://chromium.googlesource.com/chromium/src/+/HEAD/docs/sync/model_api.md

In next CLs:
Implement sync-initiated installs and uninstalls.

TBR=alancutter@chromium.org

Bug: 860583
Change-Id: I65d3cae6f47dd8fc8fdbe2c7cfb2ed13012f90f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830494Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704482}

chrome/browser/web_applications/BUILD.gn

@@ -45,6 +45,7 @@ source_set("web_applications") {
     "web_app_registry_update.h",
     "web_app_sync_bridge.cc",
     "web_app_sync_bridge.h",
+    "web_app_sync_install_delegate.h",
   ]
 
   deps = [

chrome/browser/web_applications/components/web_app_constants.h

@@ -19,6 +19,9 @@ enum Type {
   kSystem = kMinValue,
   kPolicy,
   kWebAppStore,
+  // We sync only regular user-installed apps from the open web. For
+  // user-installed apps without overlaps this is the only source that will be
+  // set.
   kSync,
   kDefault,
   kMaxValue

chrome/browser/web_applications/test/test_web_app_registry_controller.cc

@@ -7,6 +7,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/web_applications/test/test_web_app_database_factory.h"
+#include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registry_update.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
 
@@ -21,7 +22,7 @@ void TestWebAppRegistryController::SetUp(Profile* profile) {
   mutable_registrar_ = std::make_unique<WebAppRegistrarMutable>(profile);
 
   sync_bridge_ = std::make_unique<WebAppSyncBridge>(
-      profile, database_factory_.get(), mutable_registrar_.get(),
+      profile, database_factory_.get(), mutable_registrar_.get(), this,
       mock_processor_.CreateForwardingProcessor());
 
   ON_CALL(processor(), IsTrackingMetadata())
@@ -51,6 +52,12 @@ void TestWebAppRegistryController::UnregisterAll() {
     update->DeleteApp(app_id);
 }
 
+void TestWebAppRegistryController::InstallWebAppsAfterSync(
+    std::vector<WebApp*> web_apps) {}
+
+void TestWebAppRegistryController::UninstallWebAppsAfterSync(
+    std::vector<std::unique_ptr<WebApp>> web_apps) {}
+
 void TestWebAppRegistryController::DestroySubsystems() {
   mutable_registrar_.reset();
   sync_bridge_.reset();

chrome/browser/web_applications/test/test_web_app_registry_controller.h

@@ -9,6 +9,7 @@
 
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
+#include "chrome/browser/web_applications/web_app_sync_install_delegate.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -20,7 +21,7 @@ class TestWebAppDatabaseFactory;
 class WebAppSyncBridge;
 class WebApp;
 
-class TestWebAppRegistryController {
+class TestWebAppRegistryController : public SyncInstallDelegate {
  public:
   TestWebAppRegistryController();
   ~TestWebAppRegistryController();
@@ -35,6 +36,11 @@ class TestWebAppRegistryController {
   void UnregisterApp(const AppId& app_id);
   void UnregisterAll();
 
+  // SyncInstallDelegate:
+  void InstallWebAppsAfterSync(std::vector<WebApp*> web_apps) override;
+  void UninstallWebAppsAfterSync(
+      std::vector<std::unique_ptr<WebApp>> web_apps) override;
+
   void DestroySubsystems();
 
   TestWebAppDatabaseFactory& database_factory() { return *database_factory_; }

chrome/browser/web_applications/web_app.h

@@ -33,6 +33,9 @@ class WebApp {
   const GURL& scope() const { return scope_; }
   const base::Optional<SkColor>& theme_color() const { return theme_color_; }
   blink::mojom::DisplayMode display_mode() const { return display_mode_; }
+  // Locally installed apps have shortcuts installed on various UI surfaces.
+  // If app isn't locally installed, it is excluded from UIs and only listed as
+  // a part of user's app library.
   bool is_locally_installed() const { return is_locally_installed_; }
   // Sync-initiated installation produces a sync placeholder app awaiting for
   // full installation process. The sync placeholder app has only app_id,

chrome/browser/web_applications/web_app_database.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/web_applications/web_app_registry_update.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/model/metadata_batch.h"
+#include "components/sync/model/metadata_change_list.h"
 #include "components/sync/model/model_error.h"
 #include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
 
@@ -50,14 +51,18 @@ void WebAppDatabase::BeginTransaction() {
   write_batch_ = store_->CreateWriteBatch();
 }
 
-void WebAppDatabase::CommitTransaction(const RegistryUpdateData& update_data,
-                                       CompletionCallback callback) {
+void WebAppDatabase::CommitTransaction(
+    const RegistryUpdateData& update_data,
+    std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+    CompletionCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(opened_);
 
   DCHECK(write_batch_);
   DCHECK(!update_data.IsEmpty());
 
+  write_batch_->TakeMetadataChangesFrom(std::move(metadata_change_list));
+
   for (const std::unique_ptr<WebApp>& web_app : update_data.apps_to_create) {
     auto proto = CreateWebAppProto(*web_app);
     write_batch_->WriteData(web_app->app_id(), proto->SerializeAsString());

chrome/browser/web_applications/web_app_database.h

@@ -21,6 +21,7 @@
 namespace syncer {
 class ModelError;
 class MetadataBatch;
+class MetadataChangeList;
 }  // namespace syncer
 
 namespace web_app {
@@ -49,8 +50,10 @@ class WebAppDatabase {
   using CompletionCallback = base::OnceCallback<void(bool success)>;
   // There can be only 1 transaction at a time.
   void BeginTransaction();
-  void CommitTransaction(const RegistryUpdateData& update_data,
-                         CompletionCallback callback);
+  void CommitTransaction(
+      const RegistryUpdateData& update_data,
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      CompletionCallback callback);
   void CancelTransaction();
 
   // Exposed for testing.

chrome/browser/web_applications/web_app_install_manager.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_data_retriever.h"
 #include "chrome/browser/web_applications/components/web_app_utils.h"
+#include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_install_task.h"
 #include "chrome/common/web_application_info.h"
 #include "content/public/browser/web_contents.h"
@@ -154,6 +155,18 @@ void WebAppInstallManager::Shutdown() {
   web_contents_.reset();
 }
 
+void WebAppInstallManager::InstallWebAppsAfterSync(
+    std::vector<WebApp*> web_apps) {
+  // TODO(crbug.com/860583): Implement sync-initiated app installs.
+  NOTIMPLEMENTED();
+}
+
+void WebAppInstallManager::UninstallWebAppsAfterSync(
+    std::vector<std::unique_ptr<WebApp>> web_apps) {
+  // TODO(crbug.com/860583): Implement sync-initiated app uninstalls.
+  NOTIMPLEMENTED();
+}
+
 void WebAppInstallManager::SetUrlLoaderForTesting(
     std::unique_ptr<WebAppUrlLoader> url_loader) {
   url_loader_ = std::move(url_loader);

chrome/browser/web_applications/web_app_install_manager.h

@@ -16,6 +16,7 @@
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_url_loader.h"
+#include "chrome/browser/web_applications/web_app_sync_install_delegate.h"
 
 class Profile;
 
@@ -29,7 +30,8 @@ enum class InstallResultCode;
 class WebAppDataRetriever;
 class WebAppInstallTask;
 
-class WebAppInstallManager final : public InstallManager {
+class WebAppInstallManager final : public InstallManager,
+                                   public SyncInstallDelegate {
  public:
   explicit WebAppInstallManager(Profile* profile);
   ~WebAppInstallManager() override;
@@ -64,6 +66,11 @@ class WebAppInstallManager final : public InstallManager {
                                 OnceInstallCallback callback) override;
   void Shutdown() override;
 
+  // For the new USS-based system only. SyncInstallDelegate:
+  void InstallWebAppsAfterSync(std::vector<WebApp*> web_apps) override;
+  void UninstallWebAppsAfterSync(
+      std::vector<std::unique_ptr<WebApp>> web_apps) override;
+
   using DataRetrieverFactory =
       base::RepeatingCallback<std::unique_ptr<WebAppDataRetriever>()>;
   void SetDataRetrieverFactoryForTesting(

chrome/browser/web_applications/web_app_provider.cc

@@ -170,7 +170,8 @@ void WebAppProvider::CreateWebAppsSubsystems(Profile* profile) {
     auto mutable_registrar = std::make_unique<WebAppRegistrarMutable>(profile);
 
     sync_bridge = std::make_unique<WebAppSyncBridge>(
-        profile, database_factory_.get(), mutable_registrar.get());
+        profile, database_factory_.get(), mutable_registrar.get(),
+        install_manager_.get());
 
     // Upcast to read-only WebAppRegistrar.
     registrar = std::move(mutable_registrar);

chrome/browser/web_applications/web_app_sync_bridge.h

@@ -14,12 +14,14 @@
 #include "chrome/browser/web_applications/components/app_registry_controller.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
+#include "components/sync/model/entity_change.h"
 #include "components/sync/model/model_type_sync_bridge.h"
 
 class Profile;
 
 namespace syncer {
 class MetadataBatch;
+class MetadataChangeList;
 class ModelError;
 class ModelTypeChangeProcessor;
 }  // namespace syncer
@@ -27,6 +29,7 @@ class ModelTypeChangeProcessor;
 namespace web_app {
 
 class AbstractWebAppDatabaseFactory;
+class SyncInstallDelegate;
 class WebAppDatabase;
 class WebAppRegistryUpdate;
 struct RegistryUpdateData;
@@ -38,12 +41,14 @@ class WebAppSyncBridge : public AppRegistryController,
  public:
   WebAppSyncBridge(Profile* profile,
                    AbstractWebAppDatabaseFactory* database_factory,
-                   WebAppRegistrarMutable* registrar);
+                   WebAppRegistrarMutable* registrar,
+                   SyncInstallDelegate* install_delegate);
   // Tests may inject mocks using this ctor.
   WebAppSyncBridge(
       Profile* profile,
       AbstractWebAppDatabaseFactory* database_factory,
       WebAppRegistrarMutable* registrar,
+      SyncInstallDelegate* install_delegate,
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor);
   ~WebAppSyncBridge() override;
 
@@ -64,8 +69,15 @@ class WebAppSyncBridge : public AppRegistryController,
 
  private:
   void CheckRegistryUpdateData(const RegistryUpdateData& update_data) const;
-  // Update the in-memory model.
-  void UpdateRegistrar(std::unique_ptr<RegistryUpdateData> update_data);
+
+  // Update the in-memory model. Returns unregistered apps which may be
+  // disposed.
+  std::vector<std::unique_ptr<WebApp>> UpdateRegistrar(
+      std::unique_ptr<RegistryUpdateData> update_data);
+
+  // Update the remote sync server.
+  void UpdateSync(const RegistryUpdateData& update_data,
+                  syncer::MetadataChangeList* metadata_change_list);
 
   void OnDatabaseOpened(base::OnceClosure callback,
                         Registry registry,
@@ -74,6 +86,17 @@ class WebAppSyncBridge : public AppRegistryController,
 
   void ReportErrorToChangeProcessor(const syncer::ModelError& error);
 
+  // Any local entities that don’t exist remotely must be provided to sync.
+  void MergeLocalAppsToSync(const syncer::EntityChangeList& entity_data,
+                            syncer::MetadataChangeList* metadata_change_list);
+
+  void ApplySyncDataChange(const syncer::EntityChange& change,
+                           RegistryUpdateData* update_local_data);
+
+  // Update registrar and Install/Uninstall missing/excessive local apps.
+  void ApplySyncChangesToRegistrar(
+      std::unique_ptr<RegistryUpdateData> update_local_data);
+
   // syncer::ModelTypeSyncBridge:
   std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()
       override;
@@ -90,6 +113,7 @@ class WebAppSyncBridge : public AppRegistryController,
 
   std::unique_ptr<WebAppDatabase> database_;
   WebAppRegistrarMutable* const registrar_;
+  SyncInstallDelegate* const install_delegate_;
 
   bool is_in_update_ = false;
 

chrome/browser/web_applications/web_app_sync_install_delegate.h

+// Copyright 2019 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_SYNC_INSTALL_DELEGATE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_SYNC_INSTALL_DELEGATE_H_
+
+#include <memory>
+#include <vector>
+
+namespace web_app {
+
+class WebApp;
+
+// WebAppSyncBridge delegates sync-initiated installs and uninstalls using
+// this interface.
+class SyncInstallDelegate {
+ public:
+  virtual ~SyncInstallDelegate() = default;
+
+  // |web_apps| are already registered and owned by the registrar.
+  virtual void InstallWebAppsAfterSync(std::vector<WebApp*> web_apps) = 0;
+  // |web_apps| are already unregistered and not owned by the registrar.
+  virtual void UninstallWebAppsAfterSync(
+      std::vector<std::unique_ptr<WebApp>> web_apps) = 0;
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_SYNC_INSTALL_DELEGATE_H_