indexeddb: add blob write failure reason to message

This is a little bit awkward for native file system handles, which
aren't related to blob writing failures.  However, these are already
returned as "failure to write blobs", so it's not too far afield.

Bug: 1144131
Change-Id: I800445b39a93e7fda9d36480092d2cff55760f94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538155
Commit-Queue: enne <enne@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827874}

content/browser/indexed_db/indexed_db_backing_store.cc

@@ -3289,7 +3289,8 @@ Status IndexedDBBackingStore::Transaction::CommitPhaseOne(
     return WriteNewBlobs(std::move(callback));
   } else {
     return std::move(callback).Run(
-        BlobWriteResult::kRunPhaseTwoAndReturnResult);
+        BlobWriteResult::kRunPhaseTwoAndReturnResult,
+        storage::mojom::WriteBlobToFileResult::kSuccess);
   }
 }
 
@@ -3455,7 +3456,8 @@ leveldb::Status IndexedDBBackingStore::Transaction::WriteNewBlobs(
   }
   if (num_objects_to_write == 0) {
     return std::move(callback).Run(
-        BlobWriteResult::kRunPhaseTwoAndReturnResult);
+        BlobWriteResult::kRunPhaseTwoAndReturnResult,
+        storage::mojom::WriteBlobToFileResult::kSuccess);
   }
 
   write_state_.emplace(num_objects_to_write, std::move(callback));
@@ -3464,7 +3466,8 @@ leveldb::Status IndexedDBBackingStore::Transaction::WriteNewBlobs(
       backing_store_->blob_storage_context_;
 
   auto write_result_callback = base::BindRepeating(
-      [](base::WeakPtr<Transaction> transaction, bool success) {
+      [](base::WeakPtr<Transaction> transaction,
+         storage::mojom::WriteBlobToFileResult result) {
         if (!transaction)
           return;
         // This can be null if Rollback() is called.
@@ -3472,28 +3475,22 @@ leveldb::Status IndexedDBBackingStore::Transaction::WriteNewBlobs(
           return;
         auto& write_state = transaction->write_state_.value();
         DCHECK(!write_state.on_complete.is_null());
-        if (!success) {
+        if (result != storage::mojom::WriteBlobToFileResult::kSuccess) {
           auto on_complete = std::move(write_state.on_complete);
           transaction->write_state_.reset();
-          std::move(on_complete).Run(BlobWriteResult::kFailure);
+          std::move(on_complete).Run(BlobWriteResult::kFailure, result);
           return;
         }
         --(write_state.calls_left);
         if (write_state.calls_left == 0) {
           auto on_complete = std::move(write_state.on_complete);
           transaction->write_state_.reset();
-          std::move(on_complete).Run(BlobWriteResult::kRunPhaseTwoAsync);
+          std::move(on_complete)
+              .Run(BlobWriteResult::kRunPhaseTwoAsync, result);
         }
       },
       ptr_factory_.GetWeakPtr());
 
-  auto blob_write_result_callback = base::BindRepeating(
-      [](const base::RepeatingCallback<void(bool success)>& callback,
-         storage::mojom::WriteBlobToFileResult result) {
-        callback.Run(result == storage::mojom::WriteBlobToFileResult::kSuccess);
-      },
-      write_result_callback);
-
   for (auto& iter : external_object_change_map_) {
     for (auto& entry : iter.second->mutable_external_objects()) {
       switch (entry.object_type()) {
@@ -3526,7 +3523,7 @@ leveldb::Status IndexedDBBackingStore::Transaction::WriteNewBlobs(
               backing_store_->GetBlobFileName(database_id_,
                                               entry.blob_number()),
               IndexedDBBackingStore::ShouldSyncOnCommit(durability_),
-              last_modified, blob_write_result_callback);
+              last_modified, write_result_callback);
           break;
         }
         case IndexedDBExternalObject::ObjectType::kNativeFileSystemHandle: {
@@ -3545,18 +3542,21 @@ leveldb::Status IndexedDBBackingStore::Transaction::WriteNewBlobs(
               base::BindOnce(
                   [](base::WeakPtr<Transaction> transaction,
                      IndexedDBExternalObject* object,
-                     base::OnceCallback<void(bool success)> callback,
+                     base::OnceCallback<void(
+                         storage::mojom::WriteBlobToFileResult)> callback,
                      const std::vector<uint8_t>& serialized_token) {
                     // |object| is owned by |transaction|, so make sure
                     // |transaction| is still valid before doing anything else.
                     if (!transaction)
                       return;
                     if (serialized_token.empty()) {
-                      std::move(callback).Run(/*success=*/false);
+                      std::move(callback).Run(
+                          storage::mojom::WriteBlobToFileResult::kError);
                       return;
                     }
                     object->set_native_file_system_token(serialized_token);
-                    std::move(callback).Run(/*success=*/true);
+                    std::move(callback).Run(
+                        storage::mojom::WriteBlobToFileResult::kSuccess);
                   },
                   ptr_factory_.GetWeakPtr(), &entry, write_result_callback));
           break;

content/browser/indexed_db/indexed_db_backing_store_unittest.cc

@@ -770,13 +770,15 @@ BlobWriteCallback CreateBlobWriteCallback(
     base::OnceClosure on_done = base::OnceClosure()) {
   *succeeded = false;
   return base::BindOnce(
-      [](bool* succeeded, base::OnceClosure on_done, BlobWriteResult result) {
+      [](bool* succeeded, base::OnceClosure on_done, BlobWriteResult result,
+         storage::mojom::WriteBlobToFileResult error) {
         switch (result) {
           case BlobWriteResult::kFailure:
             NOTREACHED();
             break;
           case BlobWriteResult::kRunPhaseTwoAsync:
           case BlobWriteResult::kRunPhaseTwoAndReturnResult:
+            DCHECK_EQ(error, storage::mojom::WriteBlobToFileResult::kSuccess);
             *succeeded = true;
             break;
         }

content/browser/indexed_db/indexed_db_external_object_storage.h

@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
 #include "base/time/time.h"
+#include "components/services/storage/public/mojom/blob_storage_context.mojom.h"
 #include "content/browser/indexed_db/indexed_db_external_object.h"
 #include "content/browser/indexed_db/indexed_db_leveldb_coding.h"
 #include "storage/common/file_system/file_system_mount_option.h"
@@ -40,8 +41,11 @@ enum class BlobWriteResult {
 // BlobWriteResult signifies if the operation succeeded or not, and the returned
 // status is used to handle errors in the next part of the transcation commit
 // lifecycle. Note: The returned status can only be used when the result is
-// |kRunPhaseTwoAndReturnResult|.
-using BlobWriteCallback = base::OnceCallback<leveldb::Status(BlobWriteResult)>;
+// |kRunPhaseTwoAndReturnResult|.  The WriteBlobToFileResult is a more granular
+// error in the case something goes wrong.
+using BlobWriteCallback =
+    base::OnceCallback<leveldb::Status(BlobWriteResult,
+                                       storage::mojom::WriteBlobToFileResult)>;
 
 // This object represents a change in the database involving adding or removing
 // external objects. if external_objects() is empty, then objects are to be

content/browser/indexed_db/indexed_db_fake_backing_store.cc

@@ -193,7 +193,9 @@ void IndexedDBFakeBackingStore::FakeTransaction::Begin(
     std::vector<ScopeLock> locks) {}
 leveldb::Status IndexedDBFakeBackingStore::FakeTransaction::CommitPhaseOne(
     BlobWriteCallback callback) {
-  return std::move(callback).Run(BlobWriteResult::kRunPhaseTwoAndReturnResult);
+  return std::move(callback).Run(
+      BlobWriteResult::kRunPhaseTwoAndReturnResult,
+      storage::mojom::WriteBlobToFileResult::kSuccess);
 }
 leveldb::Status IndexedDBFakeBackingStore::FakeTransaction::CommitPhaseTwo() {
   return result_;

content/browser/indexed_db/indexed_db_transaction.cc

@@ -28,6 +28,26 @@ namespace content {
 
 namespace {
 
+std::string WriteBlobToFileResultToString(
+    storage::mojom::WriteBlobToFileResult result) {
+  switch (result) {
+    case storage::mojom::WriteBlobToFileResult::kError:
+      return "Error";
+    case storage::mojom::WriteBlobToFileResult::kBadPath:
+      return "BadPath";
+    case storage::mojom::WriteBlobToFileResult::kInvalidBlob:
+      return "InvalidBlob";
+    case storage::mojom::WriteBlobToFileResult::kIOError:
+      return "IOError";
+    case storage::mojom::WriteBlobToFileResult::kTimestampError:
+      return "TimestampError";
+    case storage::mojom::WriteBlobToFileResult::kSuccess:
+      return "Success";
+  }
+  NOTREACHED();
+  return "";
+}
+
 const int64_t kInactivityTimeoutPeriodSeconds = 60;
 
 // Used for UMA metrics - do not change values.
@@ -266,7 +286,8 @@ void IndexedDBTransaction::EnsureBackingStoreTransactionBegun() {
 }
 
 leveldb::Status IndexedDBTransaction::BlobWriteComplete(
-    BlobWriteResult result) {
+    BlobWriteResult result,
+    storage::mojom::WriteBlobToFileResult error) {
   IDB_TRACE("IndexedDBTransaction::BlobWriteComplete");
   if (state_ == FINISHED)  // aborted
     return leveldb::Status::OK();
@@ -275,7 +296,10 @@ leveldb::Status IndexedDBTransaction::BlobWriteComplete(
   switch (result) {
     case BlobWriteResult::kFailure: {
       leveldb::Status status = Abort(IndexedDBDatabaseError(
-          blink::mojom::IDBException::kDataError, "Failed to write blobs."));
+          blink::mojom::IDBException::kDataError,
+          base::ASCIIToUTF16(base::StringPrintf(
+              "Failed to write blobs (%s)",
+              WriteBlobToFileResultToString(error).c_str()))));
       if (!status.ok())
         tear_down_callback_.Run(status);
       // The result is ignored.
@@ -338,10 +362,11 @@ leveldb::Status IndexedDBTransaction::Commit() {
     // to write.
     s = transaction_->CommitPhaseOne(base::BindOnce(
         [](base::WeakPtr<IndexedDBTransaction> transaction,
-           BlobWriteResult result) {
+           BlobWriteResult result,
+           storage::mojom::WriteBlobToFileResult error) {
           if (!transaction)
             return leveldb::Status::OK();
-          return transaction->BlobWriteComplete(result);
+          return transaction->BlobWriteComplete(result, error);
         },
         ptr_factory_.GetWeakPtr()));
   }

content/browser/indexed_db/indexed_db_transaction.h

@@ -211,7 +211,9 @@ class CONTENT_EXPORT IndexedDBTransaction {
   bool IsTaskQueueEmpty() const;
   bool HasPendingTasks() const;
 
-  leveldb::Status BlobWriteComplete(BlobWriteResult result);
+  leveldb::Status BlobWriteComplete(
+      BlobWriteResult result,
+      storage::mojom::WriteBlobToFileResult error);
   void CloseOpenCursorBindings();
   void CloseOpenCursors();
   leveldb::Status CommitPhaseTwo();