WebApp: Introduce WebAppDatabase.
For now we try to reuse as much USS code as possible.
It has limitations and drawbacks (see the code).
We will rethink our persistence code later when we start to design
WebAppSyncBridge
(an implementation for syncer::ModelTypeSyncBridge interface).
Bug: 896150
Change-Id: Ia898b905f39869a6b1c3f79751358425449a617a
Reviewed-on: https://chromium-review.googlesource.com/c/1295529
Reviewed-by: calamity <[email protected]>
Reviewed-by: Mikel Astiz <[email protected]>
Reviewed-by: Ben Wells <[email protected]>
Commit-Queue: Alexey Baskakov <[email protected]>
Cr-Commit-Position: refs/heads/master@{#605974}diff --git a/chrome/browser/web_applications/BUILD.gn b/chrome/browser/web_applications/BUILD.gn
index c6de8a2..c7b41bf 100644
--- a/chrome/browser/web_applications/BUILD.gn
+++ b/chrome/browser/web_applications/BUILD.gn
@@ -8,8 +8,13 @@
source_set("web_applications") {
sources = [
+ "abstract_web_app_database.h",
"web_app.cc",
"web_app.h",
+ "web_app_database.cc",
+ "web_app_database.h",
+ "web_app_database_factory.cc",
+ "web_app_database_factory.h",
"web_app_install_manager.cc",
"web_app_install_manager.h",
"web_app_registrar.cc",
@@ -22,9 +27,14 @@
":web_app_group",
"//chrome/browser/web_applications/components",
"//chrome/common",
+ "//components/sync",
"//content/public/browser",
"//skia",
]
+
+ public_deps = [
+ "//chrome/browser/web_applications/proto",
+ ]
}
source_set("web_applications_test_support") {
@@ -33,6 +43,10 @@
sources = [
"test/test_data_retriever.cc",
"test/test_data_retriever.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/web_app_test.cc",
"test/web_app_test.h",
]
@@ -42,6 +56,7 @@
":web_applications",
"//chrome/browser/web_applications/components",
"//chrome/test:test_support",
+ "//components/sync:test_support_model",
"//testing/gtest",
]
}
@@ -50,6 +65,7 @@
testonly = true
sources = [
+ "web_app_database_unittest.cc",
"web_app_install_manager_unittest.cc",
"web_app_registrar_unittest.cc",
]
diff --git a/chrome/browser/web_applications/abstract_web_app_database.h b/chrome/browser/web_applications/abstract_web_app_database.h
new file mode 100644
index 0000000..a7228db
--- /dev/null
+++ b/chrome/browser/web_applications/abstract_web_app_database.h
@@ -0,0 +1,39 @@
+// 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_ABSTRACT_WEB_APP_DATABASE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_DATABASE_H_
+
+#include <map>
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+
+namespace web_app {
+
+class WebApp;
+
+using Registry = std::map<AppId, std::unique_ptr<WebApp>>;
+
+// An abstract database for the registry persistence.
+// Exclusively used from the UI thread.
+class AbstractWebAppDatabase {
+ public:
+ virtual ~AbstractWebAppDatabase() = 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;
+
+ // |OpenDatabase| must have been called and completed before using any other
+ // methods. Otherwise, it fails with DCHECK.
+ virtual void WriteWebApp(const WebApp& web_app) = 0;
+ virtual void DeleteWebApps(std::vector<AppId> app_ids) = 0;
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_ABSTRACT_WEB_APP_DATABASE_H_
diff --git a/chrome/browser/web_applications/proto/BUILD.gn b/chrome/browser/web_applications/proto/BUILD.gn
new file mode 100644
index 0000000..58fc6fd
--- /dev/null
+++ b/chrome/browser/web_applications/proto/BUILD.gn
@@ -0,0 +1,14 @@
+# 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("//third_party/protobuf/proto_library.gni")
+
+proto_library("proto") {
+ sources = [
+ "web_app.proto",
+ ]
+ deps = [
+ "//components/sync/protocol",
+ ]
+}
diff --git a/chrome/browser/web_applications/proto/web_app.proto b/chrome/browser/web_applications/proto/web_app.proto
new file mode 100644
index 0000000..77dd32d
--- /dev/null
+++ b/chrome/browser/web_applications/proto/web_app.proto
@@ -0,0 +1,20 @@
+// 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.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package web_app;
+
+// WebApp class data.
+// TODO(loyso): Consider moving this proto to components/sync/protocol/
+// crbug.com/902214.
+message WebAppProto {
+ // app_id is the client tag for sync system.
+ optional string app_id = 1;
+ optional string name = 2;
+ optional string description = 3;
+ optional string launch_url = 4;
+}
diff --git a/chrome/browser/web_applications/test/test_web_app_database.cc b/chrome/browser/web_applications/test/test_web_app_database.cc
new file mode 100644
index 0000000..731bef6e
--- /dev/null
+++ b/chrome/browser/web_applications/test/test_web_app_database.cc
@@ -0,0 +1,27 @@
+// 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/test/test_web_app_database.h"
+
+#include "chrome/browser/web_applications/web_app.h"
+
+namespace web_app {
+
+TestWebAppDatabase::TestWebAppDatabase() {}
+
+TestWebAppDatabase::~TestWebAppDatabase() {}
+
+void TestWebAppDatabase::OpenDatabase(OnceRegistryOpenedCallback callback) {
+ open_database_callback_ = std::move(callback);
+}
+
+void TestWebAppDatabase::WriteWebApp(const WebApp& web_app) {
+ write_web_app_id_ = web_app.app_id();
+}
+
+void TestWebAppDatabase::DeleteWebApps(std::vector<AppId> app_ids) {
+ delete_web_app_ids_ = std::move(app_ids);
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/test/test_web_app_database.h b/chrome/browser/web_applications/test/test_web_app_database.h
new file mode 100644
index 0000000..19d53fa
--- /dev/null
+++ b/chrome/browser/web_applications/test/test_web_app_database.h
@@ -0,0 +1,42 @@
+// 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_TEST_TEST_WEB_APP_DATABASE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_H_
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "chrome/browser/web_applications/abstract_web_app_database.h"
+
+namespace web_app {
+
+class TestWebAppDatabase : public AbstractWebAppDatabase {
+ public:
+ TestWebAppDatabase();
+ ~TestWebAppDatabase() override;
+
+ // AbstractWebAppDatabase:
+ void OpenDatabase(OnceRegistryOpenedCallback callback) override;
+ void WriteWebApp(const WebApp& web_app) override;
+ void DeleteWebApps(std::vector<AppId> app_ids) override;
+
+ OnceRegistryOpenedCallback TakeOpenDatabaseCallback() {
+ return std::move(open_database_callback_);
+ }
+ const AppId& write_web_app_id() const { return write_web_app_id_; }
+ const std::vector<AppId>& delete_web_app_ids() const {
+ return delete_web_app_ids_;
+ }
+
+ private:
+ OnceRegistryOpenedCallback open_database_callback_;
+ AppId write_web_app_id_;
+ std::vector<AppId> delete_web_app_ids_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestWebAppDatabase);
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_H_
diff --git a/chrome/browser/web_applications/test/test_web_app_database_factory.cc b/chrome/browser/web_applications/test/test_web_app_database_factory.cc
new file mode 100644
index 0000000..e8b101f
--- /dev/null
+++ b/chrome/browser/web_applications/test/test_web_app_database_factory.cc
@@ -0,0 +1,23 @@
+// 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/test/test_web_app_database_factory.h"
+
+#include "components/sync/model/model_type_store_test_util.h"
+
+namespace web_app {
+
+TestWebAppDatabaseFactory::TestWebAppDatabaseFactory() {
+ // InMemoryStore must be created after message_loop_.
+ store_ = syncer::ModelTypeStoreTestUtil::CreateInMemoryStoreForTest();
+}
+
+TestWebAppDatabaseFactory::~TestWebAppDatabaseFactory() {}
+
+syncer::OnceModelTypeStoreFactory TestWebAppDatabaseFactory::GetStoreFactory() {
+ return syncer::ModelTypeStoreTestUtil::FactoryForForwardingStore(
+ store_.get());
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/test/test_web_app_database_factory.h b/chrome/browser/web_applications/test/test_web_app_database_factory.h
new file mode 100644
index 0000000..ddefbf3
--- /dev/null
+++ b/chrome/browser/web_applications/test/test_web_app_database_factory.h
@@ -0,0 +1,38 @@
+// 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_TEST_TEST_WEB_APP_DATABASE_FACTORY_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_FACTORY_H_
+
+#include "base/macros.h"
+#include "chrome/browser/web_applications/web_app_database_factory.h"
+
+namespace syncer {
+class ModelTypeStore;
+} // namespace syncer
+
+namespace web_app {
+
+// Requires base::MessageLoop message_loop_ in test fixture. Reason:
+// InMemoryStore needs a SequencedTaskRunner.
+// MessageLoop ctor calls MessageLoop::SetThreadTaskRunnerHandle().
+class TestWebAppDatabaseFactory : public AbstractWebAppDatabaseFactory {
+ public:
+ TestWebAppDatabaseFactory();
+ ~TestWebAppDatabaseFactory() override;
+
+ // AbstractWebAppDatabaseFactory interface implementation.
+ syncer::OnceModelTypeStoreFactory GetStoreFactory() override;
+
+ syncer::ModelTypeStore* store() { return store_.get(); }
+
+ private:
+ std::unique_ptr<syncer::ModelTypeStore> store_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestWebAppDatabaseFactory);
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_WEB_APP_DATABASE_FACTORY_H_
diff --git a/chrome/browser/web_applications/web_app_database.cc b/chrome/browser/web_applications/web_app_database.cc
new file mode 100644
index 0000000..f232ec5
--- /dev/null
+++ b/chrome/browser/web_applications/web_app_database.cc
@@ -0,0 +1,179 @@
+// 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_database.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "chrome/browser/web_applications/proto/web_app.pb.h"
+#include "chrome/browser/web_applications/web_app.h"
+#include "chrome/browser/web_applications/web_app_database_factory.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/model/model_error.h"
+
+namespace web_app {
+
+WebAppDatabase::WebAppDatabase(AbstractWebAppDatabaseFactory* database_factory)
+ : database_factory_(database_factory) {
+ DCHECK(database_factory_);
+}
+
+WebAppDatabase::~WebAppDatabase() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+void WebAppDatabase::OpenDatabase(OnceRegistryOpenedCallback callback) {
+ DCHECK(!store_);
+
+ syncer::OnceModelTypeStoreFactory store_factory =
+ database_factory_->GetStoreFactory();
+
+ // CreateStore then ReadRegistry.
+ CreateStore(
+ std::move(store_factory),
+ base::BindOnce(&WebAppDatabase::ReadRegistry,
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void WebAppDatabase::WriteWebApp(const WebApp& web_app) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(opened_);
+
+ BeginTransaction();
+
+ auto proto = CreateWebAppProto(web_app);
+ write_batch_->WriteData(proto->app_id(), proto->SerializeAsString());
+
+ CommitTransaction();
+}
+
+void WebAppDatabase::DeleteWebApps(std::vector<AppId> app_ids) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(opened_);
+
+ BeginTransaction();
+ for (auto& app_id : app_ids)
+ write_batch_->DeleteData(app_id);
+ CommitTransaction();
+}
+
+// static
+std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
+ const WebApp& web_app) {
+ auto proto = std::make_unique<WebAppProto>();
+
+ proto->set_app_id(web_app.app_id());
+ proto->set_name(web_app.name());
+ proto->set_description(web_app.description());
+ proto->set_launch_url(web_app.launch_url());
+
+ return proto;
+}
+
+// static
+std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
+ auto web_app = std::make_unique<WebApp>(proto.app_id());
+
+ web_app->SetName(proto.name());
+ web_app->SetDescription(proto.description());
+ web_app->SetLaunchUrl(proto.launch_url());
+
+ return web_app;
+}
+
+void WebAppDatabase::ReadRegistry(OnceRegistryOpenedCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(store_);
+ store_->ReadAllData(base::BindOnce(&WebAppDatabase::OnAllDataRead,
+ weak_ptr_factory_.GetWeakPtr(),
+ std::move(callback)));
+}
+
+void WebAppDatabase::CreateStore(
+ syncer::OnceModelTypeStoreFactory store_factory,
+ base::OnceClosure closure) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+ // For now we use syncer:APPS prefix within local isolated LevelDB, no sync.
+ // TODO(loyso): Create separate ModelType::WEB_APPS before implementing sync.
+ // Otherwise it may interfere with existing APPS data.
+ std::move(store_factory)
+ .Run(syncer::APPS,
+ base::BindOnce(&WebAppDatabase::OnStoreCreated,
+ weak_ptr_factory_.GetWeakPtr(), std::move(closure)));
+}
+
+void WebAppDatabase::OnStoreCreated(
+ base::OnceClosure closure,
+ const base::Optional<syncer::ModelError>& error,
+ std::unique_ptr<syncer::ModelTypeStore> store) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (error) {
+ LOG(ERROR) << "WebApps LevelDB opening error: " << error->ToString();
+ return;
+ }
+
+ store_ = std::move(store);
+ std::move(closure).Run();
+}
+
+void WebAppDatabase::OnAllDataRead(
+ OnceRegistryOpenedCallback callback,
+ const base::Optional<syncer::ModelError>& error,
+ std::unique_ptr<syncer::ModelTypeStore::RecordList> data_records) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (error) {
+ LOG(ERROR) << "WebApps LevelDB read error: " << error->ToString();
+ return;
+ }
+
+ Registry registry;
+ for (const syncer::ModelTypeStore::Record& record : *data_records) {
+ const AppId app_id = record.id;
+ std::unique_ptr<WebApp> web_app = ParseWebApp(app_id, record.value);
+ if (web_app)
+ registry.emplace(app_id, std::move(web_app));
+ }
+
+ std::move(callback).Run(std::move(registry));
+ opened_ = true;
+}
+
+void WebAppDatabase::OnDataWritten(
+ const base::Optional<syncer::ModelError>& error) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (error)
+ LOG(ERROR) << "WebApps LevelDB write error: " << error->ToString();
+}
+
+// static
+std::unique_ptr<WebApp> WebAppDatabase::ParseWebApp(const AppId& app_id,
+ const std::string& value) {
+ WebAppProto proto;
+ const bool parsed = proto.ParseFromString(value);
+ if (!parsed || proto.app_id() != app_id) {
+ LOG(ERROR) << "WebApps LevelDB parse error (unknown).";
+ return nullptr;
+ }
+
+ return CreateWebApp(proto);
+}
+
+void WebAppDatabase::BeginTransaction() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(!write_batch_);
+ write_batch_ = store_->CreateWriteBatch();
+}
+
+void WebAppDatabase::CommitTransaction() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(write_batch_);
+
+ store_->CommitWriteBatch(std::move(write_batch_),
+ base::BindOnce(&WebAppDatabase::OnDataWritten,
+ weak_ptr_factory_.GetWeakPtr()));
+ write_batch_.reset();
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_database.h b/chrome/browser/web_applications/web_app_database.h
new file mode 100644
index 0000000..8ef472b
--- /dev/null
+++ b/chrome/browser/web_applications/web_app_database.h
@@ -0,0 +1,83 @@
+// 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_DATABASE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_DATABASE_H_
+
+#include <memory>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#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/components/web_app_helpers.h"
+#include "components/sync/model/model_type_store.h"
+
+namespace syncer {
+class ModelError;
+} // namespace syncer
+
+namespace web_app {
+
+class AbstractWebAppDatabaseFactory;
+class WebApp;
+class WebAppProto;
+
+// Exclusively used from the UI thread.
+class WebAppDatabase : public AbstractWebAppDatabase {
+ public:
+ explicit WebAppDatabase(AbstractWebAppDatabaseFactory* database_factory);
+ ~WebAppDatabase() override;
+
+ // AbstractWebAppDatabase:
+ void OpenDatabase(OnceRegistryOpenedCallback callback) override;
+ void WriteWebApp(const WebApp& web_app) override;
+ void DeleteWebApps(std::vector<AppId> app_ids) override;
+
+ // 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);
+
+ private:
+ void CreateStore(syncer::OnceModelTypeStoreFactory store_factory,
+ base::OnceClosure closure);
+ void OnStoreCreated(base::OnceClosure closure,
+ const base::Optional<syncer::ModelError>& error,
+ std::unique_ptr<syncer::ModelTypeStore> store);
+
+ void OnAllDataRead(
+ OnceRegistryOpenedCallback callback,
+ const base::Optional<syncer::ModelError>& error,
+ std::unique_ptr<syncer::ModelTypeStore::RecordList> data_records);
+
+ void OnDataWritten(const base::Optional<syncer::ModelError>& error);
+
+ static std::unique_ptr<WebApp> ParseWebApp(const AppId& app_id,
+ const std::string& value);
+
+ void BeginTransaction();
+ void CommitTransaction();
+
+ std::unique_ptr<syncer::ModelTypeStore> store_;
+ std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch_;
+ AbstractWebAppDatabaseFactory* database_factory_;
+
+ // Database is opened if store is created and all data read.
+ bool opened_ = false;
+
+ SEQUENCE_CHECKER(sequence_checker_);
+
+ base::WeakPtrFactory<WebAppDatabase> weak_ptr_factory_{this};
+
+ DISALLOW_COPY_AND_ASSIGN(WebAppDatabase);
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_DATABASE_H_
diff --git a/chrome/browser/web_applications/web_app_database_factory.cc b/chrome/browser/web_applications/web_app_database_factory.cc
new file mode 100644
index 0000000..6254a80
--- /dev/null
+++ b/chrome/browser/web_applications/web_app_database_factory.cc
@@ -0,0 +1,27 @@
+// 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_database_factory.h"
+
+#include "base/files/file_path.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/sync/model_impl/model_type_store_service_impl.h"
+
+namespace web_app {
+
+constexpr base::FilePath::CharType kWebAppsFolderName[] =
+ FILE_PATH_LITERAL("WebApps");
+
+WebAppDatabaseFactory::WebAppDatabaseFactory(Profile* profile)
+ : model_type_store_service_(
+ std::make_unique<syncer::ModelTypeStoreServiceImpl>(
+ profile->GetPath().Append(base::FilePath(kWebAppsFolderName)))) {}
+
+WebAppDatabaseFactory::~WebAppDatabaseFactory() {}
+
+syncer::OnceModelTypeStoreFactory WebAppDatabaseFactory::GetStoreFactory() {
+ return model_type_store_service_->GetStoreFactory();
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_database_factory.h b/chrome/browser/web_applications/web_app_database_factory.h
new file mode 100644
index 0000000..33ac124
--- /dev/null
+++ b/chrome/browser/web_applications/web_app_database_factory.h
@@ -0,0 +1,45 @@
+// 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_DATABASE_FACTORY_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_DATABASE_FACTORY_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "components/sync/model/model_type_store.h"
+
+class Profile;
+
+namespace syncer {
+class ModelTypeStoreService;
+} // namespace syncer
+
+namespace web_app {
+
+class AbstractWebAppDatabaseFactory {
+ public:
+ virtual ~AbstractWebAppDatabaseFactory() = default;
+ virtual syncer::OnceModelTypeStoreFactory GetStoreFactory() = 0;
+};
+
+class WebAppDatabaseFactory : public AbstractWebAppDatabaseFactory {
+ public:
+ explicit WebAppDatabaseFactory(Profile* profile);
+ ~WebAppDatabaseFactory() override;
+
+ // AbstractWebAppDatabaseFactory implementation.
+ syncer::OnceModelTypeStoreFactory GetStoreFactory() override;
+
+ private:
+ // TODO(loyso): Consider using shared ModelTypeStoreService from profile.
+ // crbug.com/902214.
+ std::unique_ptr<syncer::ModelTypeStoreService> model_type_store_service_;
+
+ DISALLOW_COPY_AND_ASSIGN(WebAppDatabaseFactory);
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_DATABASE_FACTORY_H_
diff --git a/chrome/browser/web_applications/web_app_database_unittest.cc b/chrome/browser/web_applications/web_app_database_unittest.cc
new file mode 100644
index 0000000..8955cd4
--- /dev/null
+++ b/chrome/browser/web_applications/web_app_database_unittest.cc
@@ -0,0 +1,181 @@
+// 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_database.h"
+
+#include <memory>
+#include <tuple>
+
+#include "base/bind_helpers.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/proto/web_app.pb.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_registrar.h"
+#include "components/sync/model/model_type_store.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace web_app {
+
+namespace {
+
+bool operator==(const WebApp& web_app, const WebApp& web_app2) {
+ return std::tie(web_app.app_id(), web_app.name(), web_app.launch_url(),
+ web_app.description()) ==
+ std::tie(web_app2.app_id(), web_app2.name(), web_app2.launch_url(),
+ web_app2.description());
+}
+
+bool operator!=(const WebApp& web_app, const WebApp& web_app2) {
+ return !operator==(web_app, web_app2);
+}
+
+bool IsRegistryEqual(const Registry& registry, const Registry& registry2) {
+ if (registry.size() != registry2.size())
+ return false;
+
+ for (auto& kv : registry) {
+ const WebApp* web_app = kv.second.get();
+ const WebApp* web_app2 = registry2.at(web_app->app_id()).get();
+ if (*web_app != *web_app2)
+ return false;
+ }
+
+ return true;
+}
+
+} // namespace
+
+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>(database_.get());
+ }
+
+ void InitRegistrar() {
+ base::RunLoop run_loop;
+
+ registrar_->Init(base::BindLambdaForTesting([&]() { run_loop.Quit(); }));
+
+ run_loop.Run();
+ }
+
+ static std::unique_ptr<WebApp> CreateWebApp(const std::string& base_url,
+ int suffix) {
+ const auto launch_url = base_url + base::IntToString(suffix);
+ const AppId app_id = GenerateAppIdFromURL(GURL(launch_url));
+ const std::string name = "Name" + base::IntToString(suffix);
+ const std::string description = "Description" + base::IntToString(suffix);
+
+ auto app = std::make_unique<WebApp>(app_id);
+
+ app->SetName(name);
+ app->SetDescription(description);
+ app->SetLaunchUrl(launch_url);
+
+ return app;
+ }
+
+ Registry ReadRegistry() {
+ Registry registry;
+ base::RunLoop run_loop;
+
+ database_->ReadRegistry(base::BindLambdaForTesting([&](Registry r) {
+ registry = std::move(r);
+ run_loop.Quit();
+ }));
+
+ run_loop.Run();
+ return registry;
+ }
+
+ bool IsDatabaseRegistryEqualToRegistrar() {
+ Registry registry = ReadRegistry();
+ return IsRegistryEqual(registrar_->registry(), registry);
+ }
+
+ void WriteBatch(
+ std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch) {
+ base::RunLoop run_loop;
+
+ database_factory_->store()->CommitWriteBatch(
+ std::move(write_batch),
+ base::BindLambdaForTesting(
+ [&](const base::Optional<syncer::ModelError>& error) {
+ EXPECT_FALSE(error);
+ run_loop.Quit();
+ }));
+
+ run_loop.Run();
+ }
+
+ Registry WriteWebApps(const std::string& base_url, int num_apps) {
+ Registry registry;
+
+ auto write_batch = database_factory_->store()->CreateWriteBatch();
+
+ for (int i = 0; i < num_apps; ++i) {
+ auto app = CreateWebApp(base_url, i);
+ auto proto = WebAppDatabase::CreateWebAppProto(*app);
+ const auto app_id = app->app_id();
+
+ write_batch->WriteData(app_id, proto->SerializeAsString());
+
+ registry.emplace(app_id, std::move(app));
+ }
+
+ WriteBatch(std::move(write_batch));
+
+ return registry;
+ }
+
+ protected:
+ // Must be created before TestWebAppDatabaseFactory.
+ base::MessageLoop message_loop_;
+
+ std::unique_ptr<TestWebAppDatabaseFactory> database_factory_;
+ std::unique_ptr<WebAppDatabase> database_;
+ std::unique_ptr<WebAppRegistrar> registrar_;
+};
+
+TEST_F(WebAppDatabaseTest, WriteAndReadRegistry) {
+ InitRegistrar();
+ EXPECT_TRUE(registrar_->is_empty());
+
+ const int num_apps = 100;
+ const std::string base_url = "https://example.com/path";
+
+ auto app = CreateWebApp(base_url, 0);
+ auto app_id = app->app_id();
+ registrar_->RegisterApp(std::move(app));
+ EXPECT_TRUE(IsDatabaseRegistryEqualToRegistrar());
+
+ for (int i = 1; i <= num_apps; ++i) {
+ auto extra_app = CreateWebApp(base_url, i);
+ registrar_->RegisterApp(std::move(extra_app));
+ }
+ EXPECT_TRUE(IsDatabaseRegistryEqualToRegistrar());
+
+ registrar_->UnregisterApp(app_id);
+ EXPECT_TRUE(IsDatabaseRegistryEqualToRegistrar());
+
+ registrar_->UnregisterAll();
+ EXPECT_TRUE(IsDatabaseRegistryEqualToRegistrar());
+}
+
+TEST_F(WebAppDatabaseTest, OpenDatabaseAndReadRegistry) {
+ Registry registry = WriteWebApps("https://example.com/path", 100);
+
+ InitRegistrar();
+ EXPECT_TRUE(IsRegistryEqual(registrar_->registry(), registry));
+}
+
+} // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_install_manager_unittest.cc b/chrome/browser/web_applications/web_app_install_manager_unittest.cc
index aa71f1c..0cb7922e 100644
--- a/chrome/browser/web_applications/web_app_install_manager_unittest.cc
+++ b/chrome/browser/web_applications/web_app_install_manager_unittest.cc
@@ -16,6 +16,7 @@
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/test/test_data_retriever.h"
+#include "chrome/browser/web_applications/test/test_web_app_database.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_registrar.h"
@@ -56,7 +57,8 @@
void SetUp() override {
WebAppTest::SetUp();
- registrar_ = std::make_unique<WebAppRegistrar>();
+ database_ = std::make_unique<TestWebAppDatabase>();
+ registrar_ = std::make_unique<WebAppRegistrar>(database_.get());
install_manager_ =
std::make_unique<WebAppInstallManager>(profile(), registrar_.get());
}
@@ -105,6 +107,7 @@
}
protected:
+ std::unique_ptr<TestWebAppDatabase> database_;
std::unique_ptr<WebAppRegistrar> registrar_;
std::unique_ptr<WebAppInstallManager> install_manager_;
};
diff --git a/chrome/browser/web_applications/web_app_provider.cc b/chrome/browser/web_applications/web_app_provider.cc
index d74b049..9188038 100644
--- a/chrome/browser/web_applications/web_app_provider.cc
+++ b/chrome/browser/web_applications/web_app_provider.cc
@@ -19,6 +19,8 @@
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h"
#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.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_install_manager.h"
#include "chrome/browser/web_applications/web_app_provider_factory.h"
#include "chrome/browser/web_applications/web_app_registrar.h"
@@ -56,9 +58,13 @@
if (!AllowWebAppInstallation(profile))
return;
- registrar_ = std::make_unique<WebAppRegistrar>();
+ database_factory_ = std::make_unique<WebAppDatabaseFactory>(profile);
+ database_ = std::make_unique<WebAppDatabase>(database_factory_.get());
+ registrar_ = std::make_unique<WebAppRegistrar>(database_.get());
install_manager_ =
std::make_unique<WebAppInstallManager>(profile, registrar_.get());
+
+ registrar_->Init(base::DoNothing());
}
void WebAppProvider::CreateBookmarkAppsSubsystems(Profile* profile) {
@@ -123,6 +129,8 @@
install_manager_.reset();
registrar_.reset();
+ database_.reset();
+ database_factory_.reset();
}
void WebAppProvider::Observe(int type,
diff --git a/chrome/browser/web_applications/web_app_provider.h b/chrome/browser/web_applications/web_app_provider.h
index 9c5ef7d..65ed8c4 100644
--- a/chrome/browser/web_applications/web_app_provider.h
+++ b/chrome/browser/web_applications/web_app_provider.h
@@ -32,6 +32,8 @@
class InstallManager;
// Forward declarations for new extension-independent subsystems.
+class WebAppDatabase;
+class WebAppDatabaseFactory;
class WebAppRegistrar;
// Forward declarations for legacy extension-based subsystems.
@@ -82,6 +84,8 @@
std::vector<web_app::PendingAppManager::AppInfo>);
// New extension-independent subsystems:
+ std::unique_ptr<WebAppDatabaseFactory> database_factory_;
+ std::unique_ptr<WebAppDatabase> database_;
std::unique_ptr<WebAppRegistrar> registrar_;
// New generalized subsystems:
diff --git a/chrome/browser/web_applications/web_app_registrar.cc b/chrome/browser/web_applications/web_app_registrar.cc
index 61a0c49..cd1832a 100644
--- a/chrome/browser/web_applications/web_app_registrar.cc
+++ b/chrome/browser/web_applications/web_app_registrar.cc
@@ -4,12 +4,18 @@
#include "chrome/browser/web_applications/web_app_registrar.h"
+#include "base/bind.h"
+#include "base/callback.h"
#include "base/logging.h"
+#include "chrome/browser/web_applications/abstract_web_app_database.h"
#include "chrome/browser/web_applications/web_app.h"
namespace web_app {
-WebAppRegistrar::WebAppRegistrar() {}
+WebAppRegistrar::WebAppRegistrar(AbstractWebAppDatabase* database)
+ : database_(database) {
+ DCHECK(database_);
+}
WebAppRegistrar::~WebAppRegistrar() = default;
@@ -18,12 +24,16 @@
DCHECK(!app_id.empty());
DCHECK(!GetAppById(app_id));
- registry_.emplace(std::make_pair(app_id, std::move(web_app)));
+ database_->WriteWebApp(*web_app);
+
+ registry_.emplace(app_id, std::move(web_app));
}
std::unique_ptr<WebApp> WebAppRegistrar::UnregisterApp(const AppId& app_id) {
DCHECK(!app_id.empty());
+ database_->DeleteWebApps({app_id});
+
auto kv = registry_.find(app_id);
DCHECK(kv != registry_.end());
@@ -38,7 +48,27 @@
}
void WebAppRegistrar::UnregisterAll() {
+ std::vector<AppId> app_ids;
+ for (auto& kv : registry()) {
+ const AppId& app_id = kv.first;
+ app_ids.push_back(app_id);
+ }
+ database_->DeleteWebApps(std::move(app_ids));
+
registry_.clear();
}
+void WebAppRegistrar::Init(base::OnceClosure closure) {
+ database_->OpenDatabase(base::BindOnce(&WebAppRegistrar::OnDatabaseOpened,
+ weak_ptr_factory_.GetWeakPtr(),
+ std::move(closure)));
+}
+
+void WebAppRegistrar::OnDatabaseOpened(base::OnceClosure closure,
+ Registry registry) {
+ DCHECK(is_empty());
+ registry_ = std::move(registry);
+ std::move(closure).Run();
+}
+
} // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_registrar.h b/chrome/browser/web_applications/web_app_registrar.h
index cf26c9d9..d76e046 100644
--- a/chrome/browser/web_applications/web_app_registrar.h
+++ b/chrome/browser/web_applications/web_app_registrar.h
@@ -5,10 +5,12 @@
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_REGISTRAR_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_REGISTRAR_H_
-#include <map>
#include <memory>
+#include "base/callback_forward.h"
#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/web_applications/abstract_web_app_database.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
namespace web_app {
@@ -17,7 +19,7 @@
class WebAppRegistrar {
public:
- WebAppRegistrar();
+ explicit WebAppRegistrar(AbstractWebAppDatabase* database);
~WebAppRegistrar();
void RegisterApp(std::unique_ptr<WebApp> web_app);
@@ -25,7 +27,6 @@
WebApp* GetAppById(const AppId& app_id);
- using Registry = std::map<AppId, std::unique_ptr<WebApp>>;
const Registry& registry() const { return registry_; }
bool is_empty() const { return registry_.empty(); }
@@ -33,9 +34,18 @@
// Clears registry.
void UnregisterAll();
+ void Init(base::OnceClosure closure);
+
private:
+ void OnDatabaseOpened(base::OnceClosure closure, Registry registry);
+
Registry registry_;
+ // An abstract database, not owned by this registrar.
+ AbstractWebAppDatabase* database_;
+
+ base::WeakPtrFactory<WebAppRegistrar> weak_ptr_factory_{this};
+
DISALLOW_COPY_AND_ASSIGN(WebAppRegistrar);
};
diff --git a/chrome/browser/web_applications/web_app_registrar_unittest.cc b/chrome/browser/web_applications/web_app_registrar_unittest.cc
index f296f5b..390ff342 100644
--- a/chrome/browser/web_applications/web_app_registrar_unittest.cc
+++ b/chrome/browser/web_applications/web_app_registrar_unittest.cc
@@ -6,8 +6,10 @@
#include <set>
+#include "base/bind_helpers.h"
#include "base/strings/string_number_conversions.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/web_app.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
@@ -16,18 +18,26 @@
namespace {
-std::set<AppId> RegisterAppsForTesting(const std::string& base_url,
- int num_apps,
- WebAppRegistrar* registrar) {
- std::set<AppId> ids;
+Registry CreateRegistryForTesting(const std::string& base_url, int num_apps) {
+ Registry registry;
for (int i = 0; i < num_apps; ++i) {
const auto url = base_url + base::IntToString(i);
const AppId app_id = GenerateAppIdFromURL(GURL(url));
auto web_app = std::make_unique<WebApp>(app_id);
- registrar->RegisterApp(std::move(web_app));
+ registry.emplace(app_id, std::move(web_app));
+ }
- ids.insert(app_id);
+ return registry;
+}
+
+std::set<AppId> RegisterAppsForTesting(WebAppRegistrar* registrar,
+ Registry&& registry) {
+ std::set<AppId> ids;
+
+ for (auto& kv : registry) {
+ ids.insert(kv.first);
+ registrar->RegisterApp(std::move(kv.second));
}
return ids;
@@ -36,7 +46,9 @@
} // namespace
TEST(WebAppRegistrar, CreateRegisterUnregister) {
- auto registrar = std::make_unique<WebAppRegistrar>();
+ auto database = std::make_unique<TestWebAppDatabase>();
+ auto registrar = std::make_unique<WebAppRegistrar>(database.get());
+
EXPECT_EQ(nullptr, registrar->GetAppById(AppId()));
const GURL launch_url = GURL("https://example.com/path");
@@ -88,7 +100,8 @@
}
TEST(WebAppRegistrar, DestroyRegistrarOwningRegisteredApps) {
- auto registrar = std::make_unique<WebAppRegistrar>();
+ auto database = std::make_unique<TestWebAppDatabase>();
+ auto registrar = std::make_unique<WebAppRegistrar>(database.get());
const AppId app_id = GenerateAppIdFromURL(GURL("https://example.com/path"));
const AppId app_id2 = GenerateAppIdFromURL(GURL("https://example.com/path2"));
@@ -103,10 +116,11 @@
}
TEST(WebAppRegistrar, ForEachAndUnregisterAll) {
- auto registrar = std::make_unique<WebAppRegistrar>();
+ auto database = std::make_unique<TestWebAppDatabase>();
+ auto registrar = std::make_unique<WebAppRegistrar>(database.get());
- auto ids =
- RegisterAppsForTesting("https://example.com/path", 100, registrar.get());
+ Registry registry = CreateRegistryForTesting("https://example.com/path", 100);
+ auto ids = RegisterAppsForTesting(registrar.get(), std::move(registry));
EXPECT_EQ(100UL, ids.size());
for (auto& kv : registrar->registry()) {
@@ -121,4 +135,43 @@
EXPECT_TRUE(registrar->is_empty());
}
+TEST(WebAppRegistrar, AbstractWebAppDatabase) {
+ auto database = std::make_unique<TestWebAppDatabase>();
+ auto registrar = std::make_unique<WebAppRegistrar>(database.get());
+
+ registrar->Init(base::DoNothing());
+ EXPECT_TRUE(registrar->is_empty());
+
+ // Load 100 apps.
+ Registry registry = CreateRegistryForTesting("https://example.com/path", 100);
+ // Copy their ids.
+ std::set<AppId> ids;
+ for (auto& kv : registry)
+ ids.insert(kv.first);
+ EXPECT_EQ(100UL, ids.size());
+
+ database->TakeOpenDatabaseCallback().Run(std::move(registry));
+
+ // Add 1 app after opening.
+ const AppId app_id = GenerateAppIdFromURL(GURL("https://example.com/path"));
+ auto web_app = std::make_unique<WebApp>(app_id);
+ registrar->RegisterApp(std::move(web_app));
+ EXPECT_EQ(app_id, database->write_web_app_id());
+ EXPECT_EQ(101UL, registrar->registry().size());
+
+ // Remove 1 app after opening.
+ registrar->UnregisterApp(app_id);
+ EXPECT_EQ(100UL, registrar->registry().size());
+ EXPECT_EQ(1UL, database->delete_web_app_ids().size());
+ EXPECT_EQ(app_id, database->delete_web_app_ids()[0]);
+
+ // Remove 100 apps after opening.
+ registrar->UnregisterAll();
+ for (auto& app_id : database->delete_web_app_ids())
+ ids.erase(app_id);
+
+ EXPECT_TRUE(ids.empty());
+ EXPECT_TRUE(registrar->is_empty());
+}
+
} // namespace web_app
