WebApp: Erase WebAppDatabase::BeginTransaction method.

Erase WebAppDatabase::BeginTransaction and WebAppDatabase::CancelTransaction.

The contract "There can be only 1 update at a time" is tracked by
the WebAppSyncBridge::is_in_update_ flag anyway.
Get rid of this redundancy.

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

chrome/browser/web_applications/web_app_database.cc

@@ -43,49 +43,37 @@ void WebAppDatabase::OpenDatabase(RegistryOpenedCallback callback) {
                           weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void WebAppDatabase::BeginTransaction() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(opened_);
-
-  DCHECK(!write_batch_);
-  write_batch_ = store_->CreateWriteBatch();
-}
-
-void WebAppDatabase::CommitTransaction(
+void WebAppDatabase::Write(
     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));
+  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch =
+      store_->CreateWriteBatch();
+
+  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());
+    write_batch->WriteData(web_app->app_id(), proto->SerializeAsString());
   }
 
   for (const AppId& app_id : update_data.apps_to_delete)
-    write_batch_->DeleteData(app_id);
+    write_batch->DeleteData(app_id);
 
   for (const WebApp* web_app : update_data.apps_to_update) {
     auto proto = CreateWebAppProto(*web_app);
-    write_batch_->WriteData(web_app->app_id(), proto->SerializeAsString());
+    write_batch->WriteData(web_app->app_id(), proto->SerializeAsString());
   }
 
   store_->CommitWriteBatch(
-      std::move(write_batch_),
+      std::move(write_batch),
       base::BindOnce(&WebAppDatabase::OnDataWritten,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
-  write_batch_.reset();
-}
-
-void WebAppDatabase::CancelTransaction() {
-  DCHECK(write_batch_);
-  write_batch_.reset();
 }
 
 // static

chrome/browser/web_applications/web_app_database.h

@@ -48,13 +48,9 @@ class WebAppDatabase {
   void OpenDatabase(RegistryOpenedCallback callback);
 
   using CompletionCallback = base::OnceCallback<void(bool success)>;
-  // There can be only 1 transaction at a time.
-  void BeginTransaction();
-  void CommitTransaction(
-      const RegistryUpdateData& update_data,
+  void Write(const RegistryUpdateData& update_data,
              std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
              CompletionCallback callback);
-  void CancelTransaction();
 
   // Exposed for testing.
   static std::unique_ptr<WebAppProto> CreateWebAppProto(const WebApp& web_app);
@@ -83,7 +79,6 @@ class WebAppDatabase {
                      const base::Optional<syncer::ModelError>& error);
 
   std::unique_ptr<syncer::ModelTypeStore> store_;
-  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch_;
   AbstractWebAppDatabaseFactory* const database_factory_;
   ReportErrorCallback error_callback_;
 

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -133,9 +133,6 @@ WebAppSyncBridge::~WebAppSyncBridge() = default;
 std::unique_ptr<WebAppRegistryUpdate> WebAppSyncBridge::BeginUpdate() {
   DCHECK(!is_in_update_);
   is_in_update_ = true;
-
-  database_->BeginTransaction();
-
   return std::make_unique<WebAppRegistryUpdate>(registrar_);
 }
 
@@ -146,7 +143,6 @@ void WebAppSyncBridge::CommitUpdate(
   is_in_update_ = false;
 
   if (update == nullptr || update->update_data().IsEmpty()) {
-    database_->CancelTransaction();
     std::move(callback).Run(/*success*/ true);
     return;
   }
@@ -159,7 +155,7 @@ void WebAppSyncBridge::CommitUpdate(
 
   UpdateSync(*update_data, metadata_change_list.get());
 
-  database_->CommitTransaction(
+  database_->Write(
       *update_data, std::move(metadata_change_list),
       base::BindOnce(&WebAppSyncBridge::OnDataWritten,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
@@ -409,8 +405,6 @@ base::Optional<syncer::ModelError> WebAppSyncBridge::MergeSyncData(
 
   auto update_local_data = std::make_unique<RegistryUpdateData>();
 
-  database_->BeginTransaction();
-
   for (const auto& change : entity_data) {
     DCHECK_NE(change->type(), syncer::EntityChange::ACTION_DELETE);
     ApplySyncDataChange(*change, update_local_data.get());
@@ -418,8 +412,8 @@ base::Optional<syncer::ModelError> WebAppSyncBridge::MergeSyncData(
 
   MergeLocalAppsToSync(entity_data, metadata_change_list.get());
 
-  database_->CommitTransaction(
-      *update_local_data, std::move(metadata_change_list), base::DoNothing());
+  database_->Write(*update_local_data, std::move(metadata_change_list),
+                   base::DoNothing());
 
   ApplySyncChangesToRegistrar(std::move(update_local_data));
   return base::nullopt;
@@ -432,13 +426,11 @@ base::Optional<syncer::ModelError> WebAppSyncBridge::ApplySyncChanges(
 
   auto update_local_data = std::make_unique<RegistryUpdateData>();
 
-  database_->BeginTransaction();
-
   for (const auto& change : entity_changes)
     ApplySyncDataChange(*change, update_local_data.get());
 
-  database_->CommitTransaction(
-      *update_local_data, std::move(metadata_change_list), base::DoNothing());
+  database_->Write(*update_local_data, std::move(metadata_change_list),
+                   base::DoNothing());
 
   ApplySyncChangesToRegistrar(std::move(update_local_data));
   return base::nullopt;