WebApp: Let WebAppSyncBridge privately owns WebAppDatabase.

This code is disabled by default behind
kDesktopPWAsWithoutExtensions base feature.

WebAppSyncBridge should encapsulate the sync change processor
and the unified store (LevelDB).

Effectively, WebAppSyncBridge becomes the storage controller
("cloud" storage + local LevelDb). WebAppSyncBridge should handle
and coordinate any changes.

As a consequence:
Rename AbstractWebAppDatabase to AbstractWebAppSyncBridge.

In next CLs:
1) WebAppRegistry should become read-only model (M in MVC)
for external subsystems other than WebAppSyncBridge.
2) The writeable batch updates API will be moved out
of WebAppRegistry to WebAppSyncBridge. WebAppSyncBridge is
C in MVC.
3) WebAppSyncBridge should use WebAppRegistry (the model) in
one way dependency: we will erase AbstractWebAppSyncBridge
interface. There will be no pointer from registry to sync bridge.

Illustrational diagram is in progress.

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

chrome/browser/web_applications/BUILD.gn

@@ -11,7 +11,7 @@ group("web_app_test_group") {
 
 source_set("web_applications") {
   sources = [
-    "abstract_web_app_database.h",
+    "abstract_web_app_sync_bridge.h",
     "external_web_app_manager.cc",
     "external_web_app_manager.h",
     "file_utils_wrapper.cc",
@@ -93,10 +93,10 @@ source_set("web_applications_test_support") {
     "test/test_pending_app_manager.h",
     "test/test_system_web_app_manager.cc",
     "test/test_system_web_app_manager.h",
-    "test/test_web_app_database.cc",
-    "test/test_web_app_database.h",
     "test/test_web_app_database_factory.cc",
     "test/test_web_app_database_factory.h",
+    "test/test_web_app_sync_bridge.cc",
+    "test/test_web_app_sync_bridge.h",
     "test/test_web_app_ui_manager.cc",
     "test/test_web_app_ui_manager.h",
     "test/test_web_app_url_loader.cc",

chrome/browser/web_applications/abstract_web_app_database.h → chrome/browser/web_applications/abstract_web_app_sync_bridge.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_ABSTRACT_WEB_APP_DATABASE_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_DATABASE_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_SYNC_BRIDGE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_SYNC_BRIDGE_H_
 
 #include <map>
 #include <vector>
@@ -18,21 +18,21 @@ class WebApp;
 
 using Registry = std::map<AppId, std::unique_ptr<WebApp>>;
 
-// An abstract database for the registry persistence.
+// An abstract sync bridge for registry persistence and data updates.
 // Exclusively used from the UI thread.
-class AbstractWebAppDatabase {
+class AbstractWebAppSyncBridge {
  public:
-  virtual ~AbstractWebAppDatabase() = default;
+  virtual ~AbstractWebAppSyncBridge() = default;
 
-  using OnceRegistryOpenedCallback =
-      base::OnceCallback<void(Registry registry)>;
-  // Open existing or create new DB. Read all data and return it via callback.
-  virtual void OpenDatabase(OnceRegistryOpenedCallback callback) = 0;
+  using RegistryOpenedCallback = base::OnceCallback<void(Registry registry)>;
 
   using CompletionCallback = base::OnceCallback<void(bool success)>;
 
   using AppsToWrite = base::flat_set<const WebApp*>;
 
+  // Open existing or create new DB. Read all data and return it via callback.
+  virtual void OpenDatabase(RegistryOpenedCallback callback) = 0;
+
   // |OpenDatabase| must have been called and completed before using any other
   // methods. Otherwise, it fails with DCHECK.
   virtual void WriteWebApps(AppsToWrite apps, CompletionCallback callback) = 0;
@@ -42,4 +42,4 @@ class AbstractWebAppDatabase {
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_DATABASE_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_SYNC_BRIDGE_H_

chrome/browser/web_applications/test/test_web_app_database.cc → chrome/browser/web_applications/test/test_web_app_sync_bridge.cc

@@ -2,41 +2,41 @@
 // 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/test/test_web_app_database.h"
+#include "chrome/browser/web_applications/test/test_web_app_sync_bridge.h"
 
 #include "chrome/browser/web_applications/web_app.h"
 
 namespace web_app {
 
-TestWebAppDatabase::TestWebAppDatabase() {}
+TestWebAppSyncBridge::TestWebAppSyncBridge() {}
 
-TestWebAppDatabase::~TestWebAppDatabase() {}
+TestWebAppSyncBridge::~TestWebAppSyncBridge() {}
 
-void TestWebAppDatabase::OpenDatabase(OnceRegistryOpenedCallback callback) {
+void TestWebAppSyncBridge::OpenDatabase(RegistryOpenedCallback callback) {
   open_database_callback_ = std::move(callback);
 }
 
-void TestWebAppDatabase::WriteWebApps(AppsToWrite apps,
-                                      CompletionCallback callback) {
+void TestWebAppSyncBridge::WriteWebApps(AppsToWrite apps,
+                                        CompletionCallback callback) {
   for (auto* app : apps)
     write_web_app_ids_.push_back(app->app_id());
 
   std::move(callback).Run(next_write_web_apps_result_);
 }
 
-void TestWebAppDatabase::DeleteWebApps(std::vector<AppId> app_ids,
-                                       CompletionCallback callback) {
+void TestWebAppSyncBridge::DeleteWebApps(AppIds app_ids,
+                                         CompletionCallback callback) {
   delete_web_app_ids_ = std::move(app_ids);
 
   std::move(callback).Run(next_delete_web_apps_result_);
 }
 
-void TestWebAppDatabase::SetNextWriteWebAppsResult(
+void TestWebAppSyncBridge::SetNextWriteWebAppsResult(
     bool next_write_web_apps_result) {
   next_write_web_apps_result_ = next_write_web_apps_result;
 }
 
-void TestWebAppDatabase::SetNextDeleteWebAppsResult(
+void TestWebAppSyncBridge::SetNextDeleteWebAppsResult(
     bool next_delete_web_apps_result) {
   next_delete_web_apps_result_ = next_delete_web_apps_result;
 }

chrome/browser/web_applications/test/test_web_app_database.h → chrome/browser/web_applications/test/test_web_app_sync_bridge.h

@@ -2,49 +2,48 @@
 // 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_TEST_TEST_WEB_APP_DATABASE_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_SYNC_BRIDGE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_SYNC_BRIDGE_H_
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "chrome/browser/web_applications/abstract_web_app_database.h"
+#include "chrome/browser/web_applications/abstract_web_app_sync_bridge.h"
 
 namespace web_app {
 
-class TestWebAppDatabase : public AbstractWebAppDatabase {
+class TestWebAppSyncBridge : public AbstractWebAppSyncBridge {
  public:
-  TestWebAppDatabase();
-  ~TestWebAppDatabase() override;
+  TestWebAppSyncBridge();
+  ~TestWebAppSyncBridge() override;
 
-  // AbstractWebAppDatabase:
-  void OpenDatabase(OnceRegistryOpenedCallback callback) override;
+  using AppIds = std::vector<AppId>;
+
+  // AbstractWebAppSyncBridge:
+  void OpenDatabase(RegistryOpenedCallback callback) override;
   void WriteWebApps(AppsToWrite apps, CompletionCallback callback) override;
-  void DeleteWebApps(std::vector<AppId> app_ids,
-                     CompletionCallback callback) override;
+  void DeleteWebApps(AppIds app_ids, CompletionCallback callback) override;
 
-  OnceRegistryOpenedCallback TakeOpenDatabaseCallback() {
+  RegistryOpenedCallback TakeOpenDatabaseCallback() {
     return std::move(open_database_callback_);
   }
 
   void SetNextWriteWebAppsResult(bool next_write_web_apps_result);
   void SetNextDeleteWebAppsResult(bool next_delete_web_apps_result);
 
-  using AppIds = std::vector<AppId>;
-
   const AppIds& write_web_app_ids() const { return write_web_app_ids_; }
   const AppIds& delete_web_app_ids() const { return delete_web_app_ids_; }
 
  private:
-  OnceRegistryOpenedCallback open_database_callback_;
+  RegistryOpenedCallback open_database_callback_;
   AppIds write_web_app_ids_;
   AppIds delete_web_app_ids_;
 
   bool next_write_web_apps_result_ = true;
   bool next_delete_web_apps_result_ = true;
 
-  DISALLOW_COPY_AND_ASSIGN(TestWebAppDatabase);
+  DISALLOW_COPY_AND_ASSIGN(TestWebAppSyncBridge);
 };
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_SYNC_BRIDGE_H_

chrome/browser/web_applications/web_app_database.cc

@@ -24,7 +24,7 @@ WebAppDatabase::~WebAppDatabase() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-void WebAppDatabase::OpenDatabase(OnceRegistryOpenedCallback callback) {
+void WebAppDatabase::OpenDatabase(RegistryOpenedCallback callback) {
   DCHECK(!store_);
 
   syncer::OnceModelTypeStoreFactory store_factory =
@@ -201,7 +201,7 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
   return web_app;
 }
 
-void WebAppDatabase::ReadRegistry(OnceRegistryOpenedCallback callback) {
+void WebAppDatabase::ReadRegistry(RegistryOpenedCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(store_);
   store_->ReadAllData(base::BindOnce(&WebAppDatabase::OnAllDataRead,
@@ -235,7 +235,7 @@ void WebAppDatabase::OnStoreCreated(
 }
 
 void WebAppDatabase::OnAllDataRead(
-    OnceRegistryOpenedCallback callback,
+    RegistryOpenedCallback callback,
     const base::Optional<syncer::ModelError>& error,
     std::unique_ptr<syncer::ModelTypeStore::RecordList> data_records) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chrome/browser/web_applications/web_app_database.h

@@ -12,7 +12,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/sequence_checker.h"
-#include "chrome/browser/web_applications/abstract_web_app_database.h"
+#include "chrome/browser/web_applications/abstract_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "components/sync/model/model_type_store.h"
 
@@ -27,23 +27,26 @@ class WebApp;
 class WebAppProto;
 
 // Exclusively used from the UI thread.
-class WebAppDatabase : public AbstractWebAppDatabase {
+class WebAppDatabase {
  public:
   explicit WebAppDatabase(AbstractWebAppDatabaseFactory* database_factory);
-  ~WebAppDatabase() override;
+  ~WebAppDatabase();
 
-  // AbstractWebAppDatabase:
-  void OpenDatabase(OnceRegistryOpenedCallback callback) override;
-  void WriteWebApps(AppsToWrite apps, CompletionCallback callback) override;
-  void DeleteWebApps(std::vector<AppId> app_ids,
-                     CompletionCallback callback) override;
+  using RegistryOpenedCallback =
+      AbstractWebAppSyncBridge::RegistryOpenedCallback;
+  using CompletionCallback = AbstractWebAppSyncBridge::CompletionCallback;
+  using AppsToWrite = AbstractWebAppSyncBridge::AppsToWrite;
+
+  void OpenDatabase(RegistryOpenedCallback callback);
+  void WriteWebApps(AppsToWrite apps, CompletionCallback callback);
+  void DeleteWebApps(std::vector<AppId> app_ids, CompletionCallback callback);
 
   // Exposed for testing.
   static std::unique_ptr<WebAppProto> CreateWebAppProto(const WebApp& web_app);
   // Exposed for testing.
   static std::unique_ptr<WebApp> CreateWebApp(const WebAppProto& proto);
   // Exposed for testing.
-  void ReadRegistry(OnceRegistryOpenedCallback callback);
+  void ReadRegistry(RegistryOpenedCallback callback);
 
  private:
   void CreateStore(syncer::OnceModelTypeStoreFactory store_factory,
@@ -53,7 +56,7 @@ class WebAppDatabase : public AbstractWebAppDatabase {
                       std::unique_ptr<syncer::ModelTypeStore> store);
 
   void OnAllDataRead(
-      OnceRegistryOpenedCallback callback,
+      RegistryOpenedCallback callback,
       const base::Optional<syncer::ModelError>& error,
       std::unique_ptr<syncer::ModelTypeStore::RecordList> data_records);
 

chrome/browser/web_applications/web_app_database_unittest.cc

@@ -17,6 +17,7 @@
 #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_registrar.h"
+#include "chrome/browser/web_applications/web_app_sync_bridge.h"
 #include "components/sync/model/model_type_store.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -45,8 +46,8 @@ class WebAppDatabaseTest : public testing::Test {
  public:
   WebAppDatabaseTest() {
     database_factory_ = std::make_unique<TestWebAppDatabaseFactory>();
-    database_ = std::make_unique<WebAppDatabase>(database_factory_.get());
-    registrar_ = std::make_unique<WebAppRegistrar>(nullptr, database_.get());
+    sync_bridge_ = std::make_unique<WebAppSyncBridge>(database_factory_.get());
+    registrar_ = std::make_unique<WebAppRegistrar>(nullptr, sync_bridge_.get());
   }
 
   void InitRegistrar() {
@@ -109,7 +110,7 @@ class WebAppDatabaseTest : public testing::Test {
     Registry registry;
     base::RunLoop run_loop;
 
-    database_->ReadRegistry(base::BindLambdaForTesting([&](Registry r) {
+    sync_bridge().ReadRegistry(base::BindLambdaForTesting([&](Registry r) {
       registry = std::move(r);
       run_loop.Quit();
     }));
@@ -159,7 +160,7 @@ class WebAppDatabaseTest : public testing::Test {
   }
 
  protected:
-  WebAppDatabase& database() { return *database_; }
+  WebAppSyncBridge& sync_bridge() { return *sync_bridge_; }
   WebAppRegistrar& registrar() { return *registrar_; }
 
  private:
@@ -167,7 +168,7 @@ class WebAppDatabaseTest : public testing::Test {
   base::test::SingleThreadTaskEnvironment task_environment_;
 
   std::unique_ptr<TestWebAppDatabaseFactory> database_factory_;
-  std::unique_ptr<WebAppDatabase> database_;
+  std::unique_ptr<WebAppSyncBridge> sync_bridge_;
   std::unique_ptr<WebAppRegistrar> registrar_;
 };
 
@@ -216,11 +217,11 @@ TEST_F(WebAppDatabaseTest, WriteAndDeleteAppsWithCallbacks) {
 
   {
     base::RunLoop run_loop;
-    database().WriteWebApps(std::move(apps_to_write),
-                            base::BindLambdaForTesting([&](bool success) {
-                              EXPECT_TRUE(success);
-                              run_loop.Quit();
-                            }));
+    sync_bridge().WriteWebApps(std::move(apps_to_write),
+                               base::BindLambdaForTesting([&](bool success) {
+                                 EXPECT_TRUE(success);
+                                 run_loop.Quit();
+                               }));
     run_loop.Run();
 
     Registry registry_written = ReadRegistry();
@@ -229,11 +230,11 @@ TEST_F(WebAppDatabaseTest, WriteAndDeleteAppsWithCallbacks) {
 
   {
     base::RunLoop run_loop;
-    database().DeleteWebApps(std::move(apps_to_delete),
-                             base::BindLambdaForTesting([&](bool success) {
-                               EXPECT_TRUE(success);
-                               run_loop.Quit();
-                             }));
+    sync_bridge().DeleteWebApps(std::move(apps_to_delete),
+                                base::BindLambdaForTesting([&](bool success) {
+                                  EXPECT_TRUE(success);
+                                  run_loop.Quit();
+                                }));
     run_loop.Run();
 
     Registry registry_deleted = ReadRegistry();

chrome/browser/web_applications/web_app_icon_manager_unittest.cc

@@ -9,7 +9,7 @@
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/web_applications/components/web_app_icon_generator.h"
 #include "chrome/browser/web_applications/test/test_file_utils.h"
-#include "chrome/browser/web_applications/test/test_web_app_database.h"
+#include "chrome/browser/web_applications/test/test_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/test/web_app_icon_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/browser/web_applications/web_app.h"
@@ -27,8 +27,8 @@ class WebAppIconManagerTest : public WebAppTest {
     auto file_utils = std::make_unique<TestFileUtils>();
     file_utils_ = file_utils.get();
 
-    database_ = std::make_unique<TestWebAppDatabase>();
-    registrar_ = std::make_unique<WebAppRegistrar>(nullptr, database_.get());
+    sync_bridge_ = std::make_unique<TestWebAppSyncBridge>();
+    registrar_ = std::make_unique<WebAppRegistrar>(nullptr, sync_bridge_.get());
     icon_manager_ = std::make_unique<WebAppIconManager>(profile(), *registrar_,
                                                         std::move(file_utils));
   }
@@ -69,7 +69,7 @@ class WebAppIconManagerTest : public WebAppTest {
     return icons;
   }
 
-  std::unique_ptr<TestWebAppDatabase> database_;
+  std::unique_ptr<TestWebAppSyncBridge> sync_bridge_;
   std::unique_ptr<WebAppRegistrar> registrar_;
   std::unique_ptr<WebAppIconManager> icon_manager_;
 

chrome/browser/web_applications/web_app_install_task_unittest.cc

@@ -25,7 +25,7 @@
 #include "chrome/browser/web_applications/test/test_data_retriever.h"
 #include "chrome/browser/web_applications/test/test_file_utils.h"
 #include "chrome/browser/web_applications/test/test_install_finalizer.h"
-#include "chrome/browser/web_applications/test/test_web_app_database.h"
+#include "chrome/browser/web_applications/test/test_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/test/test_web_app_ui_manager.h"
 #include "chrome/browser/web_applications/test/web_app_icon_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
@@ -116,8 +116,9 @@ class WebAppInstallTaskTest : public WebAppTest {
   void SetUp() override {
     WebAppTest::SetUp();
 
-    database_ = std::make_unique<TestWebAppDatabase>();
-    registrar_ = std::make_unique<WebAppRegistrar>(profile(), database_.get());
+    sync_bridge_ = std::make_unique<TestWebAppSyncBridge>();
+    registrar_ =
+        std::make_unique<WebAppRegistrar>(profile(), sync_bridge_.get());
 
     auto file_utils = std::make_unique<TestFileUtils>();
     file_utils_ = file_utils.get();
@@ -312,7 +313,7 @@ class WebAppInstallTaskTest : public WebAppTest {
   }
 
  protected:
-  std::unique_ptr<TestWebAppDatabase> database_;
+  std::unique_ptr<TestWebAppSyncBridge> sync_bridge_;
   std::unique_ptr<WebAppRegistrar> registrar_;
   std::unique_ptr<WebAppIconManager> icon_manager_;
   std::unique_ptr<WebAppInstallTask> install_task_;

chrome/browser/web_applications/web_app_provider.cc

@@ -24,7 +24,6 @@
 #include "chrome/browser/web_applications/file_utils_wrapper.h"
 #include "chrome/browser/web_applications/pending_app_manager_impl.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
-#include "chrome/browser/web_applications/web_app_database.h"
 #include "chrome/browser/web_applications/web_app_database_factory.h"
 #include "chrome/browser/web_applications/web_app_file_handler_manager.h"
 #include "chrome/browser/web_applications/web_app_icon_manager.h"
@@ -156,9 +155,8 @@ void WebAppProvider::CreateCommonSubsystems(Profile* profile) {
 
 void WebAppProvider::CreateWebAppsSubsystems(Profile* profile) {
   database_factory_ = std::make_unique<WebAppDatabaseFactory>(profile);
-  database_ = std::make_unique<WebAppDatabase>(database_factory_.get());
-  registrar_ = std::make_unique<WebAppRegistrar>(profile, database_.get());
-  sync_bridge_ = std::make_unique<WebAppSyncBridge>();
+  sync_bridge_ = std::make_unique<WebAppSyncBridge>(database_factory_.get());
+  registrar_ = std::make_unique<WebAppRegistrar>(profile, sync_bridge_.get());
   auto icon_manager = std::make_unique<WebAppIconManager>(
       profile, *registrar_->AsWebAppRegistrar(),
       std::make_unique<FileUtilsWrapper>());

chrome/browser/web_applications/web_app_provider.h

@@ -42,7 +42,6 @@ class WebAppUiManager;
 
 // Forward declarations for new extension-independent subsystems.
 class WebAppDatabaseFactory;
-class WebAppDatabase;
 class WebAppSyncBridge;
 
 // Connects Web App features, such as the installation of default and
@@ -78,7 +77,6 @@ class WebAppProvider : public WebAppProviderBase {
   FileHandlerManager& file_handler_manager() override;
   AppIconManager& icon_manager() override;
 
-  WebAppDatabaseFactory& database_factory() { return *database_factory_; }
   WebAppSyncBridge& sync_bridge() { return *sync_bridge_; }
   SystemWebAppManager& system_web_app_manager();
 
@@ -113,7 +111,6 @@ class WebAppProvider : public WebAppProviderBase {
 
   // New extension-independent subsystems:
   std::unique_ptr<WebAppDatabaseFactory> database_factory_;
-  std::unique_ptr<WebAppDatabase> database_;
   std::unique_ptr<WebAppSyncBridge> sync_bridge_;
 
   // Generalized subsystems:

chrome/browser/web_applications/web_app_registrar.cc

@@ -14,7 +14,7 @@
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
-#include "chrome/browser/web_applications/abstract_web_app_database.h"
+#include "chrome/browser/web_applications/abstract_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registry_update.h"
@@ -22,9 +22,9 @@
 namespace web_app {
 
 WebAppRegistrar::WebAppRegistrar(Profile* profile,
-                                 AbstractWebAppDatabase* database)
-    : AppRegistrar(profile), database_(database) {
-  DCHECK(database_);
+                                 AbstractWebAppSyncBridge* sync_bridge)
+    : AppRegistrar(profile), sync_bridge_(sync_bridge) {
+  DCHECK(sync_bridge_);
 }
 
 WebAppRegistrar::~WebAppRegistrar() = default;
@@ -37,7 +37,7 @@ void WebAppRegistrar::RegisterApp(std::unique_ptr<WebApp> web_app) {
   DCHECK(!GetAppById(app_id));
 
   // TODO(loyso): Expose CompletionCallback as RegisterApp argument.
-  database_->WriteWebApps({web_app.get()}, base::DoNothing());
+  sync_bridge_->WriteWebApps({web_app.get()}, base::DoNothing());
 
   registry_.emplace(app_id, std::move(web_app));
 }
@@ -48,7 +48,7 @@ std::unique_ptr<WebApp> WebAppRegistrar::UnregisterApp(const AppId& app_id) {
   DCHECK(!app_id.empty());
 
   // TODO(loyso): Expose CompletionCallback as UnregisterApp argument.
-  database_->DeleteWebApps({app_id}, base::DoNothing());
+  sync_bridge_->DeleteWebApps({app_id}, base::DoNothing());
 
   auto kv = registry_.find(app_id);
   DCHECK(kv != registry_.end());
@@ -62,7 +62,7 @@ void WebAppRegistrar::UnregisterAll() {
   CountMutation();
 
   // TODO(loyso): Expose CompletionCallback as UnregisterAll argument.
-  database_->DeleteWebApps(GetAppIds(), base::DoNothing());
+  sync_bridge_->DeleteWebApps(GetAppIds(), base::DoNothing());
   registry_.clear();
 }
 
@@ -92,16 +92,16 @@ void WebAppRegistrar::CommitUpdate(std::unique_ptr<WebAppRegistryUpdate> update,
     DCHECK(GetAppById(app->app_id()));
 #endif
 
-  database_->WriteWebApps(
+  sync_bridge_->WriteWebApps(
       std::move(update->apps_to_update_),
       base::BindOnce(&WebAppRegistrar::OnDataWritten,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void WebAppRegistrar::Init(base::OnceClosure callback) {
-  database_->OpenDatabase(base::BindOnce(&WebAppRegistrar::OnDatabaseOpened,
-                                         weak_ptr_factory_.GetWeakPtr(),
-                                         std::move(callback)));
+  sync_bridge_->OpenDatabase(base::BindOnce(&WebAppRegistrar::OnDatabaseOpened,
+                                            weak_ptr_factory_.GetWeakPtr(),
+                                            std::move(callback)));
 }
 
 void WebAppRegistrar::OnDatabaseOpened(base::OnceClosure callback,

chrome/browser/web_applications/web_app_registrar.h

@@ -14,7 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
-#include "chrome/browser/web_applications/abstract_web_app_database.h"
+#include "chrome/browser/web_applications/abstract_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 
@@ -26,7 +26,8 @@ class WebAppRegistryUpdate;
 // A registry. This is a read-only container, which owns WebApp objects.
 class WebAppRegistrar : public AppRegistrar {
  public:
-  explicit WebAppRegistrar(Profile* profile, AbstractWebAppDatabase* database);
+  explicit WebAppRegistrar(Profile* profile,
+                           AbstractWebAppSyncBridge* sync_bridge);
   ~WebAppRegistrar() override;
 
   void RegisterApp(std::unique_ptr<WebApp> web_app);
@@ -128,8 +129,8 @@ class WebAppRegistrar : public AppRegistrar {
 
   Registry registry_;
 
-  // An abstract database, not owned by this registrar.
-  AbstractWebAppDatabase* database_;
+  // An abstract sync bridge to handle all local changes and persistence.
+  AbstractWebAppSyncBridge* sync_bridge_;
 
   bool is_in_update_ = false;
 

chrome/browser/web_applications/web_app_registrar_unittest.cc

@@ -16,7 +16,7 @@
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
-#include "chrome/browser/web_applications/test/test_web_app_database.h"
+#include "chrome/browser/web_applications/test/test_web_app_sync_bridge.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registry_update.h"
@@ -52,12 +52,13 @@ class WebAppRegistrarTest : public WebAppTest {
   void SetUp() override {
     WebAppTest::SetUp();
 
-    database_ = std::make_unique<TestWebAppDatabase>();
-    registrar_ = std::make_unique<WebAppRegistrar>(profile(), database_.get());
+    sync_bridge_ = std::make_unique<TestWebAppSyncBridge>();
+    registrar_ =
+        std::make_unique<WebAppRegistrar>(profile(), sync_bridge_.get());
   }
 
  protected:
-  TestWebAppDatabase& database() { return *database_; }
+  TestWebAppSyncBridge& sync_bridge() { return *sync_bridge_; }
   WebAppRegistrar& registrar() { return *registrar_; }
 
   std::set<AppId> RegisterAppsForTesting(Registry registry) {
@@ -80,7 +81,7 @@ class WebAppRegistrarTest : public WebAppTest {
     Registry registry;
     registry.emplace(app_id, std::move(app));
 
-    database().TakeOpenDatabaseCallback().Run(std::move(registry));
+    sync_bridge().TakeOpenDatabaseCallback().Run(std::move(registry));
     return app_id;
   }
 
@@ -94,7 +95,7 @@ class WebAppRegistrarTest : public WebAppTest {
     for (auto& kv : registry)
       app_ids.insert(kv.second->app_id());
 
-    database().TakeOpenDatabaseCallback().Run(std::move(registry));
+    sync_bridge().TakeOpenDatabaseCallback().Run(std::move(registry));
     return app_ids;
   }
 
@@ -123,11 +124,11 @@ class WebAppRegistrarTest : public WebAppTest {
 
   void DestroySubsystems() {
     registrar_.reset();
-    database_.reset();
+    sync_bridge_.reset();
   }
 
  private:
-  std::unique_ptr<TestWebAppDatabase> database_;
+  std::unique_ptr<TestWebAppSyncBridge> sync_bridge_;
   std::unique_ptr<WebAppRegistrar> registrar_;
 };
 
@@ -246,7 +247,7 @@ TEST_F(WebAppRegistrarTest, DoForEachAndUnregisterAllApps) {
   EXPECT_TRUE(registrar().is_empty());
 }
 
-TEST_F(WebAppRegistrarTest, AbstractWebAppDatabase) {
+TEST_F(WebAppRegistrarTest, AbstractWebAppSyncBridge) {
   std::set<AppId> ids = InitRegistrarWithApps("https://example.com/path", 100);
 
   // Add 1 app after Init.
@@ -254,20 +255,20 @@ TEST_F(WebAppRegistrarTest, AbstractWebAppDatabase) {
   auto web_app = std::make_unique<WebApp>(app_id);
   registrar().RegisterApp(std::move(web_app));
 
-  EXPECT_EQ(1UL, database().write_web_app_ids().size());
-  EXPECT_EQ(app_id, database().write_web_app_ids()[0]);
+  EXPECT_EQ(1UL, sync_bridge().write_web_app_ids().size());
+  EXPECT_EQ(app_id, sync_bridge().write_web_app_ids()[0]);
 
   EXPECT_EQ(101UL, registrar().registry_for_testing().size());
 
   // Remove 1 app after Init.
   registrar().UnregisterApp(app_id);
   EXPECT_EQ(100UL, registrar().registry_for_testing().size());
-  EXPECT_EQ(1UL, database().delete_web_app_ids().size());
-  EXPECT_EQ(app_id, database().delete_web_app_ids()[0]);
+  EXPECT_EQ(1UL, sync_bridge().delete_web_app_ids().size());
+  EXPECT_EQ(app_id, sync_bridge().delete_web_app_ids()[0]);
 
   // Remove 100 apps after Init.
   registrar().UnregisterAll();
-  for (auto& app_id : database().delete_web_app_ids())
+  for (auto& app_id : sync_bridge().delete_web_app_ids())
     ids.erase(app_id);
 
   EXPECT_TRUE(ids.empty());
@@ -486,13 +487,13 @@ TEST_F(WebAppRegistrarTest, DatabaseWriteAndDeleteAppsFail) {
   auto app = CreateWebApp("https://example.com/path");
   auto app_id = app->app_id();
 
-  database().SetNextWriteWebAppsResult(false);
+  sync_bridge().SetNextWriteWebAppsResult(false);
   registrar().RegisterApp(std::move(app));
 
   // nothing crashes, the production database impl would DLOG an error.
   EXPECT_FALSE(registrar().is_empty());
 
-  database().SetNextDeleteWebAppsResult(false);
+  sync_bridge().SetNextDeleteWebAppsResult(false);
   registrar().UnregisterApp(app_id);
 
   // nothing crashes, the production database impl would DLOG an error.
@@ -520,9 +521,9 @@ TEST_F(WebAppRegistrarTest, BeginAndCommitUpdate) {
   RegistrarCommitUpdate(std::move(update));
 
   // Make sure that all app ids were written to the database.
-  EXPECT_EQ(ids.size(), database().write_web_app_ids().size());
+  EXPECT_EQ(ids.size(), sync_bridge().write_web_app_ids().size());
 
-  for (auto& written_app_id : database().write_web_app_ids())
+  for (auto& written_app_id : sync_bridge().write_web_app_ids())
     ids.erase(written_app_id);
 
   EXPECT_TRUE(ids.empty());
@@ -535,7 +536,7 @@ TEST_F(WebAppRegistrarTest, CommitEmptyUpdate) {
     std::unique_ptr<WebAppRegistryUpdate> update = registrar().BeginUpdate();
     RegistrarCommitUpdate(std::move(update));
 
-    EXPECT_TRUE(database().write_web_app_ids().empty());
+    EXPECT_TRUE(sync_bridge().write_web_app_ids().empty());
   }
 
   {
@@ -543,7 +544,7 @@ TEST_F(WebAppRegistrarTest, CommitEmptyUpdate) {
     update.reset();
     RegistrarCommitUpdate(std::move(update));
 
-    EXPECT_TRUE(database().write_web_app_ids().empty());
+    EXPECT_TRUE(sync_bridge().write_web_app_ids().empty());
   }
 
   {
@@ -554,14 +555,14 @@ TEST_F(WebAppRegistrarTest, CommitEmptyUpdate) {
 
     RegistrarCommitUpdate(std::move(update));
 
-    EXPECT_TRUE(database().write_web_app_ids().empty());
+    EXPECT_TRUE(sync_bridge().write_web_app_ids().empty());
   }
 }
 
 TEST_F(WebAppRegistrarTest, CommitUpdateFailed) {
   auto app = CreateWebApp("https://example.com/path");
   auto app_id = InitRegistrarWithApp(std::move(app));
-  EXPECT_TRUE(database().write_web_app_ids().empty());
+  EXPECT_TRUE(sync_bridge().write_web_app_ids().empty());
 
   std::unique_ptr<WebAppRegistryUpdate> update = registrar().BeginUpdate();
 
@@ -569,7 +570,7 @@ TEST_F(WebAppRegistrarTest, CommitUpdateFailed) {
   EXPECT_TRUE(update_app);
   update_app->SetName("New Name");
 
-  database().SetNextWriteWebAppsResult(false);
+  sync_bridge().SetNextWriteWebAppsResult(false);
 
   base::RunLoop run_loop;
   registrar().CommitUpdate(std::move(update),
@@ -586,7 +587,7 @@ TEST_F(WebAppRegistrarTest, ScopedRegistryUpdate) {
 
   // Test empty update first.
   { ScopedRegistryUpdate update(&registrar()); }
-  EXPECT_TRUE(database().write_web_app_ids().empty());
+  EXPECT_TRUE(sync_bridge().write_web_app_ids().empty());
 
   {
     ScopedRegistryUpdate update(&registrar());
@@ -599,9 +600,9 @@ TEST_F(WebAppRegistrarTest, ScopedRegistryUpdate) {
   }
 
   // Make sure that all app ids were written to the database.
-  EXPECT_EQ(ids.size(), database().write_web_app_ids().size());
+  EXPECT_EQ(ids.size(), sync_bridge().write_web_app_ids().size());
 
-  for (auto& written_app_id : database().write_web_app_ids())
+  for (auto& written_app_id : sync_bridge().write_web_app_ids())
     ids.erase(written_app_id);
 
   EXPECT_TRUE(ids.empty());

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -7,6 +7,8 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/optional.h"
+#include "chrome/browser/web_applications/web_app_database.h"
+#include "chrome/browser/web_applications/web_app_database_factory.h"
 #include "chrome/common/channel_info.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/report_unrecoverable_error.h"
@@ -15,19 +17,43 @@
 
 namespace web_app {
 
-WebAppSyncBridge::WebAppSyncBridge()
+WebAppSyncBridge::WebAppSyncBridge(
+    AbstractWebAppDatabaseFactory* database_factory)
     : WebAppSyncBridge(
+          database_factory,
           std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
               syncer::WEB_APPS,
               base::BindRepeating(&syncer::ReportUnrecoverableError,
                                   chrome::GetChannel()))) {}
 
 WebAppSyncBridge::WebAppSyncBridge(
+    AbstractWebAppDatabaseFactory* database_factory,
     std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor)
-    : syncer::ModelTypeSyncBridge(std::move(change_processor)) {}
+    : syncer::ModelTypeSyncBridge(std::move(change_processor)) {
+  DCHECK(database_factory);
+  database_ = std::make_unique<WebAppDatabase>(database_factory);
+}
 
 WebAppSyncBridge::~WebAppSyncBridge() = default;
 
+void WebAppSyncBridge::OpenDatabase(RegistryOpenedCallback callback) {
+  database_->OpenDatabase(std::move(callback));
+}
+
+void WebAppSyncBridge::WriteWebApps(AppsToWrite apps,
+                                    CompletionCallback callback) {
+  database_->WriteWebApps(std::move(apps), std::move(callback));
+}
+
+void WebAppSyncBridge::DeleteWebApps(std::vector<AppId> app_ids,
+                                     CompletionCallback callback) {
+  database_->DeleteWebApps(std::move(app_ids), std::move(callback));
+}
+
+void WebAppSyncBridge::ReadRegistry(RegistryOpenedCallback callback) {
+  database_->ReadRegistry(std::move(callback));
+}
+
 std::unique_ptr<syncer::MetadataChangeList>
 WebAppSyncBridge::CreateMetadataChangeList() {
   NOTIMPLEMENTED();

chrome/browser/web_applications/web_app_sync_bridge.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "chrome/browser/web_applications/abstract_web_app_sync_bridge.h"
 #include "components/sync/model/model_type_sync_bridge.h"
 
 namespace syncer {
@@ -17,14 +18,31 @@ class ModelTypeChangeProcessor;
 
 namespace web_app {
 
-class WebAppSyncBridge : public syncer::ModelTypeSyncBridge {
+class AbstractWebAppDatabaseFactory;
+class WebAppDatabase;
+
+// The sync bridge exclusively owns ModelTypeChangeProcessor and WebAppDatabase
+// (the storage).
+class WebAppSyncBridge : public AbstractWebAppSyncBridge,
+                         public syncer::ModelTypeSyncBridge {
  public:
-  WebAppSyncBridge();
+  explicit WebAppSyncBridge(AbstractWebAppDatabaseFactory* database_factory);
   // Tests may inject mock change_processor using this ctor.
-  explicit WebAppSyncBridge(
+  WebAppSyncBridge(
+      AbstractWebAppDatabaseFactory* database_factory,
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor);
   ~WebAppSyncBridge() override;
 
+  // AbstractWebAppSyncBridge:
+  void OpenDatabase(RegistryOpenedCallback callback) override;
+  void WriteWebApps(AppsToWrite apps, CompletionCallback callback) override;
+  void DeleteWebApps(std::vector<AppId> app_ids,
+                     CompletionCallback callback) override;
+
+  // Exposed for testing.
+  void ReadRegistry(RegistryOpenedCallback callback);
+
+ private:
   // syncer::ModelTypeSyncBridge:
   std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()
       override;
@@ -39,7 +57,8 @@ class WebAppSyncBridge : public syncer::ModelTypeSyncBridge {
   std::string GetClientTag(const syncer::EntityData& entity_data) override;
   std::string GetStorageKey(const syncer::EntityData& entity_data) override;
 
- private:
+  std::unique_ptr<WebAppDatabase> database_;
+
   DISALLOW_COPY_AND_ASSIGN(WebAppSyncBridge);
 };