IndexedDB: Make IDBCursor.CursorContinue() use native Mojo callback

Similar to the IDBCursor.Advance() commit 9a58b89a,
IDBCursor.CursorContinue() previously took a separate IDBCallbacks
interface that had its own lifecycle and could have a number of
methods called on it.

This CL updates CursorContinue() to use Mojo's native callback
mechanism.

Bug: 717812
Change-Id: Icc7d8b0be70a078f31d658ac9c35d83d8af12b0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1480810
Commit-Queue: Chase Phillips <cmp@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638234}

content/browser/indexed_db/cursor_impl.cc

@@ -24,9 +24,10 @@ class CursorImpl::IDBSequenceHelper {
   void Advance(uint32_t count,
                base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
                blink::mojom::IDBCursor::AdvanceCallback callback);
-  void Continue(const IndexedDBKey& key,
+  void Continue(base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
+                const IndexedDBKey& key,
                 const IndexedDBKey& primary_key,
-                scoped_refptr<IndexedDBCallbacks> callbacks);
+                blink::mojom::IDBCursor::CursorContinueCallback callback);
   void Prefetch(int32_t count, scoped_refptr<IndexedDBCallbacks> callbacks);
   void PrefetchReset(int32_t used_prefetches, int32_t unused_prefetches);
 
@@ -66,12 +67,10 @@ void CursorImpl::Advance(uint32_t count,
 void CursorImpl::CursorContinue(
     const IndexedDBKey& key,
     const IndexedDBKey& primary_key,
-    blink::mojom::IDBCallbacksAssociatedPtrInfo callbacks_info) {
+    blink::mojom::IDBCursor::CursorContinueCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  scoped_refptr<IndexedDBCallbacks> callbacks(
-      new IndexedDBCallbacks(dispatcher_host_->AsWeakPtr(), origin_,
-                             std::move(callbacks_info), idb_runner_));
-  helper_->Continue(key, primary_key, std::move(callbacks));
+  helper_->Continue(dispatcher_host_->AsWeakPtr(), key, primary_key,
+                    std::move(callback));
 }
 
 void CursorImpl::Prefetch(
@@ -116,15 +115,17 @@ void CursorImpl::IDBSequenceHelper::Advance(
 }
 
 void CursorImpl::IDBSequenceHelper::Continue(
+    base::WeakPtr<content::IndexedDBDispatcherHost> dispatcher_host,
     const IndexedDBKey& key,
     const IndexedDBKey& primary_key,
-    scoped_refptr<IndexedDBCallbacks> callbacks) {
+    blink::mojom::IDBCursor::CursorContinueCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   cursor_->Continue(
+      std::move(dispatcher_host),
       key.IsValid() ? std::make_unique<IndexedDBKey>(key) : nullptr,
       primary_key.IsValid() ? std::make_unique<IndexedDBKey>(primary_key)
                             : nullptr,
-      std::move(callbacks));
+      std::move(callback));
 }
 
 void CursorImpl::IDBSequenceHelper::Prefetch(

content/browser/indexed_db/cursor_impl.h

@@ -36,7 +36,7 @@ class CursorImpl : public blink::mojom::IDBCursor {
   void CursorContinue(
       const blink::IndexedDBKey& key,
       const blink::IndexedDBKey& primary_key,
-      blink::mojom::IDBCallbacksAssociatedPtrInfo callbacks) override;
+      blink::mojom::IDBCursor::CursorContinueCallback callback) override;
   void Prefetch(int32_t count,
                 blink::mojom::IDBCallbacksAssociatedPtrInfo callbacks) override;
   void PrefetchReset(int32_t used_prefetches,

content/browser/indexed_db/indexed_db_cursor.cc

@@ -36,6 +36,14 @@ IndexedDBDatabaseError CreateCursorClosedError() {
                                 "The cursor has been closed.");
 }
 
+IndexedDBDatabaseError CreateError(uint16_t code,
+                                   const char* message,
+                                   IndexedDBTransaction* transaction) {
+  DCHECK(transaction);
+  transaction->IncrementNumErrorsSent();
+  return IndexedDBDatabaseError(code, message);
+}
+
 leveldb::Status InvokeOrSucceed(base::WeakPtr<IndexedDBCursor> weak_cursor,
                                 IndexedDBTransaction::Operation operation,
                                 IndexedDBTransaction* transaction) {
@@ -82,24 +90,6 @@ IndexedDBCursor::~IndexedDBCursor() {
   Close();
 }
 
-void IndexedDBCursor::Continue(std::unique_ptr<IndexedDBKey> key,
-                               std::unique_ptr<IndexedDBKey> primary_key,
-                               scoped_refptr<IndexedDBCallbacks> callbacks) {
-  IDB_TRACE("IndexedDBCursor::Continue");
-
-  if (closed_) {
-    callbacks->OnError(CreateCursorClosedError());
-
-    return;
-  }
-
-  transaction_->ScheduleTask(
-      task_type_,
-      BindWeakOperation(&IndexedDBCursor::CursorIterationOperation,
-                        ptr_factory_.GetWeakPtr(), base::Passed(&key),
-                        base::Passed(&primary_key), callbacks));
-}
-
 void IndexedDBCursor::Advance(
     uint32_t count,
     base::WeakPtr<content::IndexedDBDispatcherHost> dispatcher_host,
@@ -107,11 +97,9 @@ void IndexedDBCursor::Advance(
   IDB_TRACE("IndexedDBCursor::Advance");
 
   if (closed_) {
-    const IndexedDBDatabaseError closed_error(CreateCursorClosedError());
-    DCHECK_NE(closed_error.code(), 0);
+    const IndexedDBDatabaseError error(CreateCursorClosedError());
     std::move(callback).Run(
-        CreateIDBError(closed_error.code(),
-                       base::string16(closed_error.message()), transaction_),
+        blink::mojom::IDBError::New(error.code(), error.message()),
         blink::mojom::IDBCursorValuePtr());
     return;
   }
@@ -120,7 +108,7 @@ void IndexedDBCursor::Advance(
       task_type_,
       BindWeakOperation(
           &IndexedDBCursor::CursorAdvanceOperation, ptr_factory_.GetWeakPtr(),
-          count, dispatcher_host,
+          count, std::move(dispatcher_host),
           base::WrapRefCounted(dispatcher_host->context()->TaskRunner()),
           std::move(callback)));
 }
@@ -145,13 +133,13 @@ leveldb::Status IndexedDBCursor::CursorAdvanceOperation(
       return s;
     }
 
-    // CreateIDBError() needs to be called before calling Close() so
+    // CreateError() needs to be called before calling Close() so
     // |transaction_| is alive.
-    auto error = CreateIDBError(blink::kWebIDBDatabaseExceptionUnknownError,
-                                base::ASCIIToUTF16("Error advancing cursor"),
-                                transaction_);
+    auto error = CreateError(blink::kWebIDBDatabaseExceptionUnknownError,
+                             "Error advancing cursor", transaction_);
     Close();
-    std::move(callback).Run(std::move(error),
+    std::move(callback).Run(
+        blink::mojom::IDBError::New(error.code(), error.message()),
         blink::mojom::IDBCursorValuePtr());
     return s;
   }
@@ -166,8 +154,9 @@ leveldb::Status IndexedDBCursor::CursorAdvanceOperation(
     if (!IndexedDBCallbacks::CreateAllBlobs(
             dispatcher_host->blob_storage_context(), idb_runner,
             IndexedDBCallbacks::IndexedDBValueBlob::GetIndexedDBValueBlobs(
-                blob_info, &mojo_value->blob_or_file_info)))
+                blob_info, &mojo_value->blob_or_file_info))) {
       return s;
+    }
   } else {
     mojo_value = blink::mojom::IDBValue::New();
   }
@@ -179,13 +168,40 @@ leveldb::Status IndexedDBCursor::CursorAdvanceOperation(
   return s;
 }
 
-leveldb::Status IndexedDBCursor::CursorIterationOperation(
+void IndexedDBCursor::Continue(
+    base::WeakPtr<content::IndexedDBDispatcherHost> dispatcher_host,
     std::unique_ptr<IndexedDBKey> key,
     std::unique_ptr<IndexedDBKey> primary_key,
-    scoped_refptr<IndexedDBCallbacks> callbacks,
+    blink::mojom::IDBCursor::CursorContinueCallback callback) {
+  IDB_TRACE("IndexedDBCursor::Continue");
+  if (closed_) {
+    const IndexedDBDatabaseError error(CreateCursorClosedError());
+    std::move(callback).Run(
+        blink::mojom::IDBError::New(error.code(), error.message()),
+        blink::mojom::IDBCursorValuePtr());
+    return;
+  }
+
+  transaction_->ScheduleTask(
+      task_type_,
+      BindWeakOperation(
+          &IndexedDBCursor::CursorContinueOperation, ptr_factory_.GetWeakPtr(),
+          std::move(dispatcher_host),
+          base::WrapRefCounted(dispatcher_host->context()->TaskRunner()),
+          base::Passed(&key), base::Passed(&primary_key), std::move(callback)));
+}
+
+leveldb::Status IndexedDBCursor::CursorContinueOperation(
+    base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
+    scoped_refptr<base::SequencedTaskRunner> idb_runner,
+    std::unique_ptr<IndexedDBKey> key,
+    std::unique_ptr<IndexedDBKey> primary_key,
+    blink::mojom::IDBCursor::CursorContinueCallback callback,
     IndexedDBTransaction* /*transaction*/) {
-  IDB_TRACE("IndexedDBCursor::CursorIterationOperation");
+  IDB_TRACE("IndexedDBCursor::CursorContinueOperation");
   leveldb::Status s = leveldb::Status::OK();
+  if (!dispatcher_host)
+    return s;
 
   if (!cursor_ ||
       !cursor_->Continue(key.get(), primary_key.get(),
@@ -193,18 +209,44 @@ leveldb::Status IndexedDBCursor::CursorIterationOperation(
     cursor_.reset();
     if (s.ok()) {
       // This happens if we reach the end of the iterator and can't continue.
-      callbacks->OnSuccess(nullptr);
+      std::move(callback).Run(blink::mojom::IDBErrorPtr(),
+                              blink::mojom::IDBCursorValuePtr());
       return s;
     }
+
+    // |transaction_| must be valid for CreateError(), so we can't call
+    // Close() until after calling CreateError().
     IndexedDBDatabaseError error =
         CreateError(blink::kWebIDBDatabaseExceptionUnknownError,
                     "Error continuing cursor.", transaction_);
     Close();
-    callbacks->OnError(error);
+    std::move(callback).Run(
+        blink::mojom::IDBError::New(error.code(), error.message()),
+        blink::mojom::IDBCursorValuePtr());
+    return s;
+  }
+
+  blink::mojom::IDBValuePtr mojo_value;
+  std::vector<IndexedDBBlobInfo> blob_info;
+  IndexedDBValue* value = Value();
+  if (value) {
+    mojo_value = IndexedDBValue::ConvertAndEraseValue(value);
+    blob_info.swap(value->blob_info);
+
+    if (!IndexedDBCallbacks::CreateAllBlobs(
+            dispatcher_host->blob_storage_context(), idb_runner,
+            IndexedDBCallbacks::IndexedDBValueBlob::GetIndexedDBValueBlobs(
+                blob_info, &mojo_value->blob_or_file_info))) {
       return s;
     }
+  } else {
+    mojo_value = blink::mojom::IDBValue::New();
+  }
 
-  callbacks->OnSuccess(this->key(), this->primary_key(), Value());
+  blink::mojom::IDBCursorValuePtr cursor_value =
+      blink::mojom::IDBCursorValue::New(this->key(), this->primary_key(),
+                                        std::move(mojo_value));
+  std::move(callback).Run(blink::mojom::IDBErrorPtr(), std::move(cursor_value));
   return s;
 }
 
@@ -254,7 +296,7 @@ leveldb::Status IndexedDBCursor::CursorPrefetchIterationOperation(
           CreateError(blink::kWebIDBDatabaseExceptionUnknownError,
                       "Error continuing cursor.", transaction_);
       Close();
-      callbacks->OnError(error);
+      callbacks->OnError(std::move(error));
       return s;
     }
 
@@ -338,22 +380,4 @@ void IndexedDBCursor::Close() {
   transaction_ = nullptr;
 }
 
-IndexedDBDatabaseError IndexedDBCursor::CreateError(
-    uint16_t code,
-    const char* message,
-    IndexedDBTransaction* transaction) {
-  DCHECK(transaction);
-  transaction->IncrementNumErrorsSent();
-  return IndexedDBDatabaseError(code, message);
-}
-
-blink::mojom::IDBErrorPtr IndexedDBCursor::CreateIDBError(
-    uint16_t code,
-    const base::string16& message,
-    IndexedDBTransaction* transaction) {
-  DCHECK(transaction);
-  transaction->IncrementNumErrorsSent();
-  return blink::mojom::IDBError::New(code, message);
-}
-
 }  // namespace content

content/browser/indexed_db/indexed_db_cursor.h

@@ -32,9 +32,10 @@ class CONTENT_EXPORT IndexedDBCursor {
   void Advance(uint32_t count,
                base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
                blink::mojom::IDBCursor::AdvanceCallback callback);
-  void Continue(std::unique_ptr<blink::IndexedDBKey> key,
+  void Continue(base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
+                std::unique_ptr<blink::IndexedDBKey> key,
                 std::unique_ptr<blink::IndexedDBKey> primary_key,
-                scoped_refptr<IndexedDBCallbacks> callbacks);
+                blink::mojom::IDBCursor::CursorContinueCallback callback);
   void PrefetchContinue(int number_to_fetch,
                         scoped_refptr<IndexedDBCallbacks> callbacks);
   leveldb::Status PrefetchReset(int used_prefetches, int unused_prefetches);
@@ -55,10 +56,12 @@ class CONTENT_EXPORT IndexedDBCursor {
   void RemoveBinding();
   void Close();
 
-  leveldb::Status CursorIterationOperation(
+  leveldb::Status CursorContinueOperation(
+      base::WeakPtr<IndexedDBDispatcherHost> dispatcher_host,
+      scoped_refptr<base::SequencedTaskRunner> idb_runner,
       std::unique_ptr<blink::IndexedDBKey> key,
       std::unique_ptr<blink::IndexedDBKey> primary_key,
-      scoped_refptr<IndexedDBCallbacks> callbacks,
+      blink::mojom::IDBCursor::CursorContinueCallback callback,
       IndexedDBTransaction* transaction);
   leveldb::Status CursorAdvanceOperation(
       uint32_t count,
@@ -71,14 +74,6 @@ class CONTENT_EXPORT IndexedDBCursor {
       scoped_refptr<IndexedDBCallbacks> callbacks,
       IndexedDBTransaction* transaction);
 
-  static IndexedDBDatabaseError CreateError(uint16_t code,
-                                            const char* message,
-                                            IndexedDBTransaction* transaction);
-  static blink::mojom::IDBErrorPtr CreateIDBError(
-      uint16_t code,
-      const base::string16& message,
-      IndexedDBTransaction* transaction);
-
  private:
   blink::mojom::IDBTaskType task_type_;
   indexed_db::CursorType cursor_type_;

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

@@ -288,12 +288,14 @@ interface IDBCursor {
   //   of the source being iterated is reached
   Advance(uint32 count) => (IDBError? error, IDBCursorValue? value);
 
-  // Avoid calling the following function "Continue" since the Mojo
-  // Java bindings generate incorrect Java as a result.  We've named
-  // the following function "CursorContinue" as a workaround.
-  CursorContinue(IDBKey key,
-                 IDBKey primary_key,
-                 associated IDBCallbacks callbacks);
+  // CursorContinue() can call its return callback in one of 3 ways:
+  // * with |error| set and |value| unset, if an error occurs
+  // * with |error| unset and |value| set, under normal operation
+  // * with |error| unset and |value| unset, under normal operation when the end
+  //   of the source being iterated is reached
+  CursorContinue(IDBKey key, IDBKey primary_key)
+    => (IDBError? error, IDBCursorValue? value);
+
   Prefetch(int32 count, associated IDBCallbacks callbacks);
   PrefetchReset(int32 used_prefetches, int32 unused_prefetches);
 };

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

@@ -121,10 +121,33 @@ void WebIDBCursorImpl::CursorContinue(const IDBKey* key,
 
   // Reset all cursor prefetch caches except for this cursor.
   IndexedDBDispatcher::ResetCursorPrefetchCaches(transaction_id_, this);
-
   callbacks->SetState(weak_factory_.GetWeakPtr(), transaction_id_);
-  cursor_->CursorContinue(IDBKey::Clone(key), IDBKey::Clone(primary_key),
-                          GetCallbacksProxy(std::move(callbacks)));
+  cursor_->CursorContinue(
+      IDBKey::Clone(key), IDBKey::Clone(primary_key),
+      WTF::Bind(&WebIDBCursorImpl::CursorContinueCallback,
+                WTF::Unretained(this), std::move(callbacks)));
+}
+
+void WebIDBCursorImpl::CursorContinueCallback(
+    std::unique_ptr<WebIDBCallbacks> callbacks,
+    mojom::blink::IDBErrorPtr error,
+    mojom::blink::IDBCursorValuePtr value) {
+  if (error) {
+    callbacks->Error(error->error_code, error->error_message);
+    callbacks.reset();
+    return;
+  }
+
+  if (!value) {
+    callbacks->SuccessValue(nullptr);
+    callbacks.reset();
+    return;
+  }
+
+  callbacks->SuccessCursorContinue(std::move(value->key),
+                                   std::move(value->primary_key),
+                                   std::move(value->value));
+  callbacks.reset();
 }
 
 void WebIDBCursorImpl::PostSuccessHandlerCallback() {

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

@@ -30,6 +30,10 @@ class MODULES_EXPORT WebIDBCursorImpl : public WebIDBCursor {
   void CursorContinue(const IDBKey* key,
                       const IDBKey* primary_key,
                       WebIDBCallbacks* callback) override;
+  void CursorContinueCallback(std::unique_ptr<WebIDBCallbacks> callbacks,
+                              mojom::blink::IDBErrorPtr error,
+                              mojom::blink::IDBCursorValuePtr value);
+
   void PostSuccessHandlerCallback() override;
 
   void SetPrefetchData(Vector<std::unique_ptr<IDBKey>> keys,

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

@@ -54,8 +54,10 @@ class MockCursorImpl : public mojom::blink::IDBCursor {
   void CursorContinue(
       std::unique_ptr<IDBKey> key,
       std::unique_ptr<IDBKey> primary_key,
-      mojom::blink::IDBCallbacksAssociatedPtrInfo callbacks) override {
+      mojom::blink::IDBCursor::CursorContinueCallback callback) override {
     ++continue_calls_;
+    std::move(callback).Run(mojom::blink::IDBErrorPtr(),
+                            mojom::blink::IDBCursorValuePtr());
   }
 
   void CursorDestroyed() { destroyed_ = true; }