IndexedDB - mojoify Success() callback

Remove Success() from IDBCallbacks and replace with mojo response
callbacks.

Bug: 843764,627484
Change-Id: I43cd47849f37e14ee962ddd4e1f9bc27077cd3cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4548318
Auto-Submit: Evan Stade <estade@chromium.org>
Reviewed-by: Nathan Memmott <memmott@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1151911}

content/browser/indexed_db/database_impl.cc

@@ -521,37 +521,38 @@ void DatabaseImpl::Count(
       std::move(callbacks)));
 }
 
-void DatabaseImpl::DeleteRange(
-    int64_t transaction_id,
-    int64_t object_store_id,
-    const IndexedDBKeyRange& key_range,
-    mojo::PendingAssociatedRemote<blink::mojom::IDBCallbacks>
-        pending_callbacks) {
+void DatabaseImpl::DeleteRange(int64_t transaction_id,
+                               int64_t object_store_id,
+                               const IndexedDBKeyRange& key_range,
+                               DeleteRangeCallback success_callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  auto callbacks = base::MakeRefCounted<IndexedDBCallbacks>(
-      dispatcher_host_->AsWeakPtr(), bucket_info_, std::move(pending_callbacks),
-      idb_runner_);
-  if (!connection_->IsConnected())
+  if (!connection_->IsConnected()) {
+    std::move(success_callback).Run(/*success=*/false);
     return;
+  }
 
   IndexedDBTransaction* transaction =
       connection_->GetTransaction(transaction_id);
-  if (!transaction)
+  if (!transaction) {
+    std::move(success_callback).Run(/*success=*/false);
     return;
+  }
 
   if (!transaction->IsAcceptingRequests()) {
     // TODO(https://crbug.com/1249908): If the transaction was already committed
     // (or is in the process of being committed) we should kill the renderer.
     // This branch however also includes cases where the browser process aborted
     // the transaction, as currently we don't distinguish that state from the
     // transaction having been committed. So for now simply ignore the request.
+    std::move(success_callback).Run(/*success=*/false);
     return;
   }
 
-  transaction->ScheduleTask(BindWeakOperation(
-      &IndexedDBDatabase::DeleteRangeOperation,
-      connection_->database()->AsWeakPtr(), object_store_id,
-      std::make_unique<IndexedDBKeyRange>(key_range), std::move(callbacks)));
+  transaction->ScheduleTask(
+      BindWeakOperation(&IndexedDBDatabase::DeleteRangeOperation,
+                        connection_->database()->AsWeakPtr(), object_store_id,
+                        std::make_unique<IndexedDBKeyRange>(key_range),
+                        std::move(success_callback)));
 }
 
 void DatabaseImpl::GetKeyGeneratorCurrentNumber(

content/browser/indexed_db/database_impl.h

@@ -105,8 +105,7 @@ class DatabaseImpl : public blink::mojom::IDBDatabase {
   void DeleteRange(int64_t transaction_id,
                    int64_t object_store_id,
                    const blink::IndexedDBKeyRange& key_range,
-                   mojo::PendingAssociatedRemote<blink::mojom::IDBCallbacks>
-                       pending_callbacks) override;
+                   DeleteRangeCallback success_callback) override;
   void GetKeyGeneratorCurrentNumber(
       int64_t transaction_id,
       int64_t object_store_id,

content/browser/indexed_db/indexed_db_callbacks.cc

@@ -223,22 +223,6 @@ void IndexedDBCallbacks::OnSuccess(int64_t value) {
   complete_ = true;
 }
 
-void IndexedDBCallbacks::OnSuccess() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(!complete_);
-
-  DCHECK_EQ(blink::mojom::IDBDataLoss::None, data_loss_);
-
-  if (!callbacks_)
-    return;
-  if (!dispatcher_host_) {
-    OnConnectionError();
-    return;
-  }
-  callbacks_->Success();
-  complete_ = true;
-}
-
 void IndexedDBCallbacks::OnConnectionError() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   callbacks_.reset();

content/browser/indexed_db/indexed_db_callbacks.h

@@ -34,7 +34,6 @@ struct IndexedDBDatabaseMetadata;
 
 namespace content {
 class IndexedDBConnection;
-class IndexedDBCursor;
 class IndexedDBDatabase;
 struct IndexedDBDataLossInfo;
 
@@ -69,9 +68,6 @@ class CONTENT_EXPORT IndexedDBCallbacks
   // IndexedDBDatabase::GetKeyGeneratorCurrentNumber
   virtual void OnSuccess(int64_t value);
 
-  // IndexedDBCursor::Continue / Advance (when complete)
-  virtual void OnSuccess();
-
   void OnConnectionError();
 
   bool is_complete() const { return complete_; }

content/browser/indexed_db/indexed_db_database.cc

@@ -1617,22 +1617,24 @@ Status IndexedDBDatabase::CountOperation(
 Status IndexedDBDatabase::DeleteRangeOperation(
     int64_t object_store_id,
     std::unique_ptr<IndexedDBKeyRange> key_range,
-    scoped_refptr<IndexedDBCallbacks> callbacks,
+    blink::mojom::IDBDatabase::DeleteRangeCallback success_callback,
     IndexedDBTransaction* transaction) {
   TRACE_EVENT1("IndexedDB", "IndexedDBDatabase::DeleteRangeOperation", "txn.id",
                transaction->id());
 
-  if (!IsObjectStoreIdInMetadata(object_store_id))
-    return leveldb::Status::InvalidArgument("Invalid object_store_id.");
-
-  Status s = backing_store_->DeleteRange(transaction->BackingStoreTransaction(),
-                                         id(), object_store_id, *key_range);
-  if (!s.ok())
-    return s;
-  callbacks->OnSuccess();
-  factory_->NotifyIndexedDBContentChanged(
-      bucket_locator(), metadata_.name,
-      metadata_.object_stores[object_store_id].name);
+  Status s;
+  if (IsObjectStoreIdInMetadata(object_store_id)) {
+    s = backing_store_->DeleteRange(transaction->BackingStoreTransaction(),
+                                    id(), object_store_id, *key_range);
+  } else {
+    s = leveldb::Status::InvalidArgument("Invalid object_store_id.");
+  }
+  if (s.ok()) {
+    factory_->NotifyIndexedDBContentChanged(
+        bucket_locator(), metadata_.name,
+        metadata_.object_stores[object_store_id].name);
+  }
+  std::move(success_callback).Run(s.ok());
   return s;
 }
 

content/browser/indexed_db/indexed_db_database.h

@@ -285,7 +285,7 @@ class CONTENT_EXPORT IndexedDBDatabase {
   leveldb::Status DeleteRangeOperation(
       int64_t object_store_id,
       std::unique_ptr<blink::IndexedDBKeyRange> key_range,
-      scoped_refptr<IndexedDBCallbacks> callbacks,
+      blink::mojom::IDBDatabase::DeleteRangeCallback success_callback,
       IndexedDBTransaction* transaction);
 
   leveldb::Status GetKeyGeneratorCurrentNumberOperation(

content/browser/indexed_db/indexed_db_unittest.cc

@@ -439,7 +439,6 @@ class ForceCloseDBCallbacks : public IndexedDBCallbacks {
   ForceCloseDBCallbacks(const ForceCloseDBCallbacks&) = delete;
   ForceCloseDBCallbacks& operator=(const ForceCloseDBCallbacks&) = delete;
 
-  void OnSuccess() override {}
   void OnSuccess(std::unique_ptr<IndexedDBConnection> connection,
                  const IndexedDBDatabaseMetadata& metadata) override {
     connection_ = std::move(connection);

content/browser/indexed_db/mock_indexed_db_callbacks.cc

@@ -36,8 +36,6 @@ void MockIndexedDBCallbacks::OnError(const IndexedDBDatabaseError& error) {
   error_called_ = true;
 }
 
-void MockIndexedDBCallbacks::OnSuccess() {}
-
 void MockIndexedDBCallbacks::OnSuccess(int64_t result) {}
 
 void MockIndexedDBCallbacks::OnSuccess(

content/browser/indexed_db/mock_indexed_db_callbacks.h

@@ -30,7 +30,6 @@ class MockIndexedDBCallbacks : public IndexedDBCallbacks {
 
   void OnError(const IndexedDBDatabaseError& error) override;
 
-  void OnSuccess() override;
   void OnSuccess(int64_t result) override;
   void OnSuccess(std::unique_ptr<IndexedDBConnection> connection,
                  const blink::IndexedDBDatabaseMetadata& metadata) override;

third_party/blink/public/mojom/indexeddb/indexeddb.mojom

@@ -249,12 +249,9 @@ interface IDBCallbacks {
   SuccessDatabase(pending_associated_remote<IDBDatabase>? pending_database,
                   IDBDatabaseMetadata metadata);
 
-  // Database::Count / DeleteRange
+  // Database::Count
   // Factory::DeleteDatabase
   SuccessInteger(int64 value);
-
-  // Cursor::Continue / Advance
-  Success();
 };
 
 // The IDBDatabaseCallbacks interface is used to notification of events out of
@@ -415,10 +412,10 @@ interface IDBDatabase {
         int64 index_id,
         IDBKeyRange key_range,
         pending_associated_remote<IDBCallbacks> pending_callbacks);
+  // Correlates to IDBObjectStore::delete() and also used by devtools.
   DeleteRange(int64 transaction_id,
               int64 object_store_id,
-              IDBKeyRange key_range,
-              pending_associated_remote<IDBCallbacks> pending_callbacks);
+              IDBKeyRange key_range) => (bool success);
   // Gets the current number of an IndexedDB ObjectStore's key generator. It
   // is implemented in the browser (more priviledged) and handles requests in
   // the renderer process: WebIDBDatabaseImpl::GetKeyGeneratorCurrentNumber()

third_party/blink/renderer/modules/indexeddb/idb_cursor.cc

@@ -47,6 +47,7 @@
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
 #include "third_party/blink/renderer/platform/wtf/casting.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
 
@@ -362,7 +363,7 @@ IDBRequest* IDBCursor::Delete(ScriptState* script_state,
       script_state, this, transaction_.Get(), std::move(metrics));
   transaction_->BackendDB()->Delete(
       transaction_->Id(), EffectiveObjectStore()->Id(), IdbPrimaryKey(),
-      request->CreateWebCallbacks().release());
+      WTF::BindOnce(&IDBRequest::OnDelete, WrapPersistent(request)));
   return request;
 }
 

third_party/blink/renderer/modules/indexeddb/idb_index.cc

@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/modules/indexeddb/indexed_db_blink_mojom_traits.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
 
@@ -344,7 +345,8 @@ IDBRequest* IDBIndex::GetInternal(ScriptState* script_state,
   IDBRequest* request = IDBRequest::Create(
       script_state, this, transaction_.Get(), std::move(metrics));
   BackendDB()->Get(transaction_->Id(), object_store_->Id(), Id(), key_range,
-                   key_only, request->CreateWebCallbacks().release());
+                   key_only,
+                   WTF::BindOnce(&IDBRequest::OnGet, WrapPersistent(request)));
   return request;
 }
 

third_party/blink/renderer/modules/indexeddb/idb_object_store.cc

@@ -168,7 +168,7 @@ IDBRequest* IDBObjectStore::get(ScriptState* script_state,
       script_state, this, transaction_.Get(), std::move(metrics));
   BackendDB()->Get(transaction_->Id(), Id(), IDBIndexMetadata::kInvalidId,
                    key_range, /*key_only=*/false,
-                   request->CreateWebCallbacks().release());
+                   WTF::BindOnce(&IDBRequest::OnGet, WrapPersistent(request)));
   return request;
 }
 
@@ -210,7 +210,7 @@ IDBRequest* IDBObjectStore::getKey(ScriptState* script_state,
       script_state, this, transaction_.Get(), std::move(metrics));
   BackendDB()->Get(transaction_->Id(), Id(), IDBIndexMetadata::kInvalidId,
                    key_range, /*key_only=*/true,
-                   request->CreateWebCallbacks().release());
+                   WTF::BindOnce(&IDBRequest::OnGet, WrapPersistent(request)));
   return request;
 }
 
@@ -726,9 +726,9 @@ IDBRequest* IDBObjectStore::deleteFunction(
     IDBRequest::AsyncTraceState metrics) {
   IDBRequest* request = IDBRequest::Create(
       script_state, this, transaction_.Get(), std::move(metrics));
-
-  BackendDB()->DeleteRange(transaction_->Id(), Id(), key_range,
-                           request->CreateWebCallbacks().release());
+  BackendDB()->DeleteRange(
+      transaction_->Id(), Id(), key_range,
+      WTF::BindOnce(&IDBRequest::OnDelete, WrapPersistent(request)));
   return request;
 }
 

third_party/blink/renderer/modules/indexeddb/idb_request.cc

@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/core/dom/events/event_queue.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
+#include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/modules/indexed_db_names.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_cursor_with_value.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_database.h"
@@ -146,7 +147,10 @@ IDBRequest::IDBRequest(ScriptState* script_state,
       source_(source),
       event_queue_(
           MakeGarbageCollected<EventQueue>(ExecutionContext::From(script_state),
-                                           TaskType::kDatabaseAccess)) {}
+                                           TaskType::kDatabaseAccess)) {
+  async_task_context_.Schedule(ExecutionContext::From(script_state),
+                               indexed_db_names::kIndexedDB);
+}
 
 IDBRequest::~IDBRequest() {
   if (!GetExecutionContext())
@@ -447,7 +451,49 @@ void IDBRequest::HandleResponse(
 
 void IDBRequest::OnClear(bool success) {
   if (success) {
+    probe::AsyncTask async_task(GetExecutionContext(), &async_task_context_,
+                                "clear");
+    HandleResponse();
+  }
+}
+
+void IDBRequest::OnDelete(bool success) {
+  if (success) {
+    probe::AsyncTask async_task(GetExecutionContext(), &async_task_context_,
+                                "delete");
+    HandleResponse();
+  }
+}
+
+void IDBRequest::OnGet(mojom::blink::IDBDatabaseGetResultPtr result) {
+  if (result->is_error_result()) {
+    mojom::blink::IDBException code = result->get_error_result()->error_code;
+    // In some cases, the backend clears the pending transaction task queue
+    // which destroys all pending tasks.  If our callback was queued with a task
+    // that gets cleared, we'll get a signal with an IgnorableAbortError as the
+    // task is torn down.  This means the error response can be safely ignored.
+    if (code == mojom::blink::IDBException::kIgnorableAbortError) {
+      return;
+    }
+    probe::AsyncTask async_task(GetExecutionContext(), &async_task_context_,
+                                "error");
+    HandleResponse(MakeGarbageCollected<DOMException>(
+        static_cast<DOMExceptionCode>(code),
+        std::move(result->get_error_result()->error_message)));
+    return;
+  }
+
+  probe::AsyncTask async_task(GetExecutionContext(), &async_task_context_,
+                              "get");
+  if (result->is_empty()) {
     HandleResponse();
+  } else if (result->is_key()) {
+    HandleResponse(std::move(result->get_key()));
+  } else if (result->is_value()) {
+    std::unique_ptr<IDBValue> value =
+        IDBValue::ConvertReturnValue(result->get_value());
+    value->SetIsolate(GetIsolate());
+    HandleResponse(std::move(value));
   }
 }
 

third_party/blink/renderer/modules/indexeddb/idb_request.h

@@ -44,6 +44,7 @@
 #include "third_party/blink/renderer/core/dom/events/event_queue.h"
 #include "third_party/blink/renderer/core/dom/events/event_target.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
+#include "third_party/blink/renderer/core/probe/async_task_context.h"
 #include "third_party/blink/renderer/modules/event_modules.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_any.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_transaction.h"
@@ -276,13 +277,14 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   void HandleResponse(Vector<std::unique_ptr<IDBValue>>);
   void HandleResponse(Vector<Vector<std::unique_ptr<IDBValue>>>);
   void HandleResponse(int64_t);
-  void HandleResponse();
   void HandleResponse(
       bool key_only,
       mojo::PendingReceiver<mojom::blink::IDBDatabaseGetAllResultSink>
           receiver);
 
   void OnClear(bool success);
+  void OnDelete(bool success);
+  void OnGet(mojom::blink::IDBDatabaseGetResultPtr result);
 
   // Only IDBOpenDBRequest instances should receive these:
   virtual void EnqueueBlocked(int64_t old_version) { NOTREACHED(); }
@@ -371,9 +373,14 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   AsyncTraceState metrics_;
 
  private:
+  friend class IDBRequestTest;
+
   // Calls EnqueueResponse().
   friend class IDBRequestQueueItem;
 
+  // See docs above for HandleResponse() variants.
+  void HandleResponse();
+
   void SetResultCursor(IDBCursor*,
                        std::unique_ptr<IDBKey>,
                        std::unique_ptr<IDBKey> primary_key,
@@ -429,6 +436,8 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   //
   // The IDBRequestQueueItem is owned by the result queue in IDBTransaction.
   IDBRequestQueueItem* queue_item_ = nullptr;
+
+  probe::AsyncTaskContext async_task_context_;
 };
 
 }  // namespace blink