[SessionStorage] Simplify database open callback graph

This changeset simplifies database callback graph in open with GetMany provided
by leveldb service.

Previously OnDatabaseOpened called OnGotDatabaseVersion, and before calling
OnConnectionFinished it was block waiting on a barrier of OnGotNamespaces and
OnGotNextMapId. This changeset gets values of database version, namespaces and
next map id in one call, and processes the result sequentially.

Bug: 965723

Test: autoninja -C out/Default content/test:content_unittests. Passed all content_unittests
Change-Id: I968d92d93414cc6918e48d870fea27dc1d74b3d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1714487
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684106}

content/browser/dom_storage/session_storage_context_mojo.cc

@@ -803,119 +803,165 @@ void SessionStorageContextMojo::OnDatabaseOpened(
     return;
   }
 
-  // Verify DB schema version.
-  if (database_) {
+  if (!database_) {
+    OnConnectionFinished();
+    return;
+  }
+
   database_.set_connection_error_handler(
       base::BindOnce(&SessionStorageContextMojo::OnMojoConnectionDestroyed,
                      weak_ptr_factory_.GetWeakPtr()));
-    database_->Get(
-        std::vector<uint8_t>(
+
+  std::vector<uint8_t> database_version(
       SessionStorageMetadata::kDatabaseVersionBytes,
-            std::end(SessionStorageMetadata::kDatabaseVersionBytes)),
-        base::BindOnce(&SessionStorageContextMojo::OnGotDatabaseVersion,
+      std::end(SessionStorageMetadata::kDatabaseVersionBytes));
+  std::vector<uint8_t> namespace_prefix(
+      SessionStorageMetadata::kNamespacePrefixBytes,
+      std::end(SessionStorageMetadata::kNamespacePrefixBytes));
+  std::vector<uint8_t> next_map_id_key(
+      SessionStorageMetadata::kNextMapIdKeyBytes,
+      std::end(SessionStorageMetadata::kNextMapIdKeyBytes));
+
+  std::vector<leveldb::mojom::GetManyRequestPtr> requests;
+  requests.emplace_back(
+      leveldb::mojom::GetManyRequest::NewKey(std::move(database_version)));
+  requests.emplace_back(leveldb::mojom::GetManyRequest::NewKeyPrefix(
+      std::move(namespace_prefix)));
+  requests.emplace_back(
+      leveldb::mojom::GetManyRequest::NewKey(std::move(next_map_id_key)));
+
+  database_->GetMany(
+      std::move(requests),
+      base::BindOnce(&SessionStorageContextMojo::OnGotDatabaseMetadata,
                      weak_ptr_factory_.GetWeakPtr()));
+}
+
+void SessionStorageContextMojo::OnGotDatabaseMetadata(
+    std::vector<leveldb::mojom::GetManyResultPtr> results) {
+  DCHECK_EQ(results.size(), 3U);
+
+  std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations;
+
+  OpenResult open_result;
+  const char* histogram_name;
+
+  std::tie(open_result, histogram_name) =
+      ParseDatabaseVersion(results[0], &migration_operations);
+  if (open_result != OpenResult::kSuccess) {
+    LogDatabaseOpenResult(open_result);
+    DeleteAndRecreateDatabase(histogram_name);
+    return;
+  }
+
+  std::tie(open_result, histogram_name) =
+      ParseNamespaces(results[1], std::move(migration_operations));
+  if (open_result != OpenResult::kSuccess) {
+    LogDatabaseOpenResult(open_result);
+    DeleteAndRecreateDatabase(histogram_name);
+    return;
+  }
+
+  std::tie(open_result, histogram_name) = ParseNextMapId(results[2]);
+  if (open_result != OpenResult::kSuccess) {
+    LogDatabaseOpenResult(open_result);
+    DeleteAndRecreateDatabase(histogram_name);
     return;
   }
 
   OnConnectionFinished();
 }
 
-void SessionStorageContextMojo::OnGotDatabaseVersion(
-    leveldb::mojom::DatabaseError status,
-    const std::vector<uint8_t>& value) {
-  std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations;
-  if (status == leveldb::mojom::DatabaseError::NOT_FOUND) {
-    // New database, or schema v0. We must treat this as a schema v0 database.
-    metadata_.ParseDatabaseVersion(base::nullopt, &migration_operations);
-  } else if (status == leveldb::mojom::DatabaseError::OK) {
-    if (!metadata_.ParseDatabaseVersion(value, &migration_operations)) {
-      LogDatabaseOpenResult(OpenResult::kInvalidVersion);
-      DeleteAndRecreateDatabase(
-          "SessionStorageContext.OpenResultAfterInvalidVersion");
-      return;
+SessionStorageContextMojo::OpenResultAndHistogramName
+SessionStorageContextMojo::ParseDatabaseVersion(
+    const leveldb::mojom::GetManyResultPtr& result,
+    std::vector<leveldb::mojom::BatchedOperationPtr>* migration_operations) {
+  if (result->is_key_value()) {
+    const std::vector<uint8_t>& value = result->get_key_value();
+
+    if (!metadata_.ParseDatabaseVersion(value, migration_operations)) {
+      return {OpenResult::kInvalidVersion,
+              "SessionStorageContext.OpenResultAfterInvalidVersion"};
     }
     database_initialized_ = true;
-  } else {
-    // Other read error. Possibly database corruption.
+    return {OpenResult::kSuccess, ""};
+  }
+
+  // Failed to get DatabaseVersion, |result| contains error status
+  leveldb::mojom::DatabaseError status = result->get_status();
+
+  if (status == leveldb::mojom::DatabaseError::NOT_FOUND) {
+    // treat as v0 or new database
+    metadata_.ParseDatabaseVersion(base::nullopt, migration_operations);
+    return {OpenResult::kSuccess, ""};
+  }
+
+  // Other read error, Possibly database corruption
   UMA_HISTOGRAM_ENUMERATION("SessionStorageContext.ReadVersionError",
                             leveldb::GetLevelDBStatusUMAValue(status),
                             leveldb_env::LEVELDB_STATUS_MAX);
-    LogDatabaseOpenResult(OpenResult::kVersionReadError);
-    DeleteAndRecreateDatabase(
-        "SessionStorageContext.OpenResultAfterReadVersionError");
-    return;
+  return {OpenResult::kVersionReadError,
+          "SessionStorageContext.OpenResultAfterReadVersionError"};
+}
+
+SessionStorageContextMojo::OpenResultAndHistogramName
+SessionStorageContextMojo::ParseNamespaces(
+    const leveldb::mojom::GetManyResultPtr& result,
+    std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations) {
+  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
+
+  if (result->is_status()) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "SessionStorageContext.ReadNamespacesError",
+        leveldb::GetLevelDBStatusUMAValue(result->get_status()),
+        leveldb_env::LEVELDB_STATUS_MAX);
+    return {OpenResult::kNamespacesReadError,
+            "SessionStorageContext.OpenResultAfterReadNamespacesError"};
   }
 
-  base::RepeatingClosure barrier = base::BarrierClosure(
-      2, base::BindOnce(&SessionStorageContextMojo::OnConnectionFinished,
-                        weak_ptr_factory_.GetWeakPtr()));
+  DCHECK(result->is_key_prefix_values());
 
-  std::vector<uint8_t> namespace_prefix(
-      SessionStorageMetadata::kNamespacePrefixBytes,
-      std::end(SessionStorageMetadata::kNamespacePrefixBytes));
-  std::vector<uint8_t> next_map_id_key(
-      SessionStorageMetadata::kNextMapIdKeyBytes,
-      std::end(SessionStorageMetadata::kNextMapIdKeyBytes));
-  database_->GetPrefixed(
-      namespace_prefix,
-      base::BindOnce(&SessionStorageContextMojo::OnGotNamespaces,
-                     weak_ptr_factory_.GetWeakPtr(), barrier,
-                     std::move(migration_operations)));
-  database_->Get(next_map_id_key,
-                 base::BindOnce(&SessionStorageContextMojo::OnGotNextMapId,
-                                weak_ptr_factory_.GetWeakPtr(), barrier));
-}
+  bool parsing_success = metadata_.ParseNamespaces(
+      std::move(result->get_key_prefix_values()), &migration_operations);
 
-void SessionStorageContextMojo::OnGotNamespaces(
-    base::OnceClosure done,
-    std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations,
-    leveldb::mojom::DatabaseError status,
-    std::vector<leveldb::mojom::KeyValuePtr> values) {
-  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
-  bool parsing_failure =
-      status == leveldb::mojom::DatabaseError::OK &&
-      !metadata_.ParseNamespaces(std::move(values), &migration_operations);
-  if (status != leveldb::mojom::DatabaseError::OK || parsing_failure) {
-    UMA_HISTOGRAM_ENUMERATION("SessionStorageContext.ReadNamespacesError",
-                              leveldb::GetLevelDBStatusUMAValue(status),
+  if (!parsing_success) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "SessionStorageContext.ReadNamespacesError",
+        leveldb::GetLevelDBStatusUMAValue(leveldb::mojom::DatabaseError::OK),
         leveldb_env::LEVELDB_STATUS_MAX);
-    LogDatabaseOpenResult(OpenResult::kNamespacesReadError);
-    DeleteAndRecreateDatabase(
-        "SessionStorageContext.OpenResultAfterReadNamespacesError");
-    return;
+    return {OpenResult::kNamespacesReadError,
+            "SessionStorageContext.OpenResultAfterReadNamespacesError"};
   }
 
-  // Write all of our migration operations if we have any.
   if (!migration_operations.empty()) {
     database_->Write(std::move(migration_operations),
                      base::BindOnce(&SessionStorageContextMojo::OnCommitResult,
                                     base::Unretained(this)));
   }
-  std::move(done).Run();
+
+  return {OpenResult::kSuccess, ""};
 }
 
-void SessionStorageContextMojo::OnGotNextMapId(
-    base::OnceClosure done,
-    leveldb::mojom::DatabaseError status,
-    const std::vector<uint8_t>& map_id) {
-  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
+SessionStorageContextMojo::OpenResultAndHistogramName
+SessionStorageContextMojo::ParseNextMapId(
+    const leveldb::mojom::GetManyResultPtr& result) {
+  if (result->is_status()) {
+    leveldb::mojom::DatabaseError status = result->get_status();
+
     if (status == leveldb::mojom::DatabaseError::NOT_FOUND) {
-    std::move(done).Run();
-    return;
-  }
-  if (status == leveldb::mojom::DatabaseError::OK) {
-    metadata_.ParseNextMapId(map_id);
-    std::move(done).Run();
-    return;
+      return {OpenResult::kSuccess, ""};
     }
 
     // Other read error. Possibly database corruption.
     UMA_HISTOGRAM_ENUMERATION("SessionStorageContext.ReadNextMapIdError",
                               leveldb::GetLevelDBStatusUMAValue(status),
                               leveldb_env::LEVELDB_STATUS_MAX);
-  LogDatabaseOpenResult(OpenResult::kNamespacesReadError);
-  DeleteAndRecreateDatabase(
-      "SessionStorageContext.OpenResultAfterReadNextMapIdError");
+    return {OpenResult::kNamespacesReadError,
+            "SessionStorageContext.OpenResultAfterReadNextMapIdError"};
+  }
+
+  DCHECK(result->is_key_value());
+  metadata_.ParseNextMapId(result->get_key_value());
+  return {OpenResult::kSuccess, ""};
 }
 
 void SessionStorageContextMojo::OnConnectionFinished() {

content/browser/dom_storage/session_storage_context_mojo.h

@@ -9,6 +9,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <tuple>
 #include <vector>
 
 #include "base/callback_forward.h"
@@ -149,6 +150,18 @@ class CONTENT_EXPORT SessionStorageContextMojo
   FRIEND_TEST_ALL_PREFIXES(SessionStorageContextMojoTest,
                            PurgeMemoryDoesNotCrashOrHang);
 
+  // These values are written to logs.  New enum values can be added, but
+  // existing enums must never be renumbered or deleted and reused.
+  enum class OpenResult {
+    kDirectoryOpenFailed = 0,
+    kDatabaseOpenFailed = 1,
+    kInvalidVersion = 2,
+    kVersionReadError = 3,
+    kNamespacesReadError = 4,
+    kSuccess = 6,
+    kMaxValue = kSuccess
+  };
+
   // Object deletion is done through |ShutdownAndDelete()|.
   ~SessionStorageContextMojo() override;
 
@@ -187,16 +200,20 @@ class CONTENT_EXPORT SessionStorageContextMojo
   void InitiateConnection(bool in_memory_only = false);
   void OnDirectoryOpened(base::File::Error err);
   void OnDatabaseOpened(bool in_memory, leveldb::mojom::DatabaseError status);
-  void OnGotDatabaseVersion(leveldb::mojom::DatabaseError status,
-                            const std::vector<uint8_t>& value);
-  void OnGotNamespaces(
-      base::OnceClosure done,
-      std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations,
-      leveldb::mojom::DatabaseError status,
-      std::vector<leveldb::mojom::KeyValuePtr> values);
-  void OnGotNextMapId(base::OnceClosure done,
-                      leveldb::mojom::DatabaseError status,
-                      const std::vector<uint8_t>& map_id);
+
+  void OnGotDatabaseMetadata(
+      std::vector<leveldb::mojom::GetManyResultPtr> results);
+
+  using OpenResultAndHistogramName = std::tuple<OpenResult, const char*>;
+  OpenResultAndHistogramName ParseDatabaseVersion(
+      const leveldb::mojom::GetManyResultPtr& result,
+      std::vector<leveldb::mojom::BatchedOperationPtr>* migration_operations);
+  OpenResultAndHistogramName ParseNamespaces(
+      const leveldb::mojom::GetManyResultPtr& result,
+      std::vector<leveldb::mojom::BatchedOperationPtr> migration_operations);
+  OpenResultAndHistogramName ParseNextMapId(
+      const leveldb::mojom::GetManyResultPtr& result);
+
   void OnConnectionFinished();
   void DeleteAndRecreateDatabase(const char* histogram_name);
   void OnDBDestroyed(bool recreate_in_memory,
@@ -211,17 +228,6 @@ class CONTENT_EXPORT SessionStorageContextMojo
 
   void GetStatistics(size_t* total_cache_size, size_t* unused_areas_count);
 
-  // These values are written to logs.  New enum values can be added, but
-  // existing enums must never be renumbered or deleted and reused.
-  enum class OpenResult {
-    kDirectoryOpenFailed = 0,
-    kDatabaseOpenFailed = 1,
-    kInvalidVersion = 2,
-    kVersionReadError = 3,
-    kNamespacesReadError = 4,
-    kSuccess = 6,
-    kMaxValue = kSuccess
-  };
 
   void LogDatabaseOpenResult(OpenResult result);