IndexedDB: Fix cursor prefetching edge cases

Cursor prefetch caches must be discarded when other
requests are made to ensure proper request sequencing.
Two edge cases were handled improperly if new records
was written just ahead of the cursor.

* A reset occurring before the prefetch results were
received would be ignored; since the newly records
weren't in the prefetch data, the cursor wouldn't see
them.

* A reset occurring after the results are received
would back up the cursor to before the new records,
even though the prefetch itself is a "continue"
and advanced past them already.

The fix is to reset the cache on receipt if necessary,
and to ensure the reset state accounts for the implicit
advance.

BUG=331570

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243344

Review URL: https://codereview.chromium.org/124323002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243421 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 83 insertions, 8 deletions

diff --git a/content/browser/indexed_db/indexed_db_cursor.cc b/content/browser/indexed_db/indexed_db_cursor.cc
index 6d7bc1c..0853b81 100644
--- a/content/browser/indexed_db/indexed_db_cursor.cc
+++ b/content/browser/indexed_db/indexed_db_cursor.cc
@@ -108,8 +108,7 @@ void IndexedDBCursor::CursorPrefetchIterationOperation(
   std::vector<IndexedDBKey> found_primary_keys;
   std::vector<std::string> found_values;
 
-  if (cursor_)
-    saved_cursor_.reset(cursor_->Clone());
+  saved_cursor_.reset();
   const size_t max_size_estimate = 10 * 1024 * 1024;
   size_t size_estimate = 0;
 
@@ -119,6 +118,12 @@ void IndexedDBCursor::CursorPrefetchIterationOperation(
       break;
     }
 
+    if (i == 0) {
+      // First prefetched result is always used, so that's the position
+      // a cursor should be reset to if the prefetch is invalidated.
+      saved_cursor_.reset(cursor_->Clone());
+    }
+
     found_keys.push_back(cursor_->key());
     found_primary_keys.push_back(cursor_->primary_key());
 
@@ -152,7 +157,8 @@ void IndexedDBCursor::CursorPrefetchIterationOperation(
       found_keys, found_primary_keys, found_values);
 }
 
-void IndexedDBCursor::PrefetchReset(int used_prefetches, int) {
+void IndexedDBCursor::PrefetchReset(int used_prefetches,
+                                    int /* unused_prefetches */) {
   IDB_TRACE("IndexedDBCursor::PrefetchReset");
   cursor_.swap(saved_cursor_);
   saved_cursor_.reset();
@@ -160,7 +166,9 @@ void IndexedDBCursor::PrefetchReset(int used_prefetches, int) {
   if (closed_)
     return;
   if (cursor_) {
-    for (int i = 0; i < used_prefetches; ++i) {
+    // First prefetched result is always used.
+    DCHECK_GT(used_prefetches, 0);
+    for (int i = 0; i < used_prefetches - 1; ++i) {
       bool ok = cursor_->Continue();
       DCHECK(ok);
     }
diff --git a/content/child/indexed_db/indexed_db_dispatcher.h b/content/child/indexed_db/indexed_db_dispatcher.h
index 8df8407..564bcd7 100644
--- a/content/child/indexed_db/indexed_db_dispatcher.h
+++ b/content/child/indexed_db/indexed_db_dispatcher.h
@@ -98,9 +98,10 @@ class CONTENT_EXPORT IndexedDBDispatcher
                                         blink::WebIDBCallbacks* callbacks_ptr,
                                         int32 ipc_cursor_id);
 
-  void RequestIDBCursorPrefetchReset(int used_prefetches,
-                                     int unused_prefetches,
-                                     int32 ipc_cursor_id);
+  // This method is virtual so it can be overridden in unit tests.
+  virtual void RequestIDBCursorPrefetchReset(int used_prefetches,
+                                             int unused_prefetches,
+                                             int32 ipc_cursor_id);
 
   void RequestIDBDatabaseClose(int32 ipc_database_id,
                                int32 ipc_database_callbacks_id);
diff --git a/content/child/indexed_db/webidbcursor_impl.cc b/content/child/indexed_db/webidbcursor_impl.cc
index 3642635..50f7ebf 100644
--- a/content/child/indexed_db/webidbcursor_impl.cc
+++ b/content/child/indexed_db/webidbcursor_impl.cc
@@ -161,6 +161,14 @@ void WebIDBCursorImpl::CachedContinue(WebIDBCallbacks* callbacks) {
 
   ++pending_onsuccess_callbacks_;
 
+  if (!continue_count_) {
+    // The cache was invalidated by a call to ResetPrefetchCache()
+    // after the RequestIDBCursorPrefetch() was made. Now that the
+    // initiating continue() call has been satisfied, discard
+    // the rest of the cache.
+    ResetPrefetchCache();
+  }
+
   callbacks->onSuccess(WebIDBKeyBuilder::Build(key),
                        WebIDBKeyBuilder::Build(primary_key),
                        value);
diff --git a/content/child/indexed_db/webidbcursor_impl.h b/content/child/indexed_db/webidbcursor_impl.h
index 187c7a5..5552022 100644
--- a/content/child/indexed_db/webidbcursor_impl.h
+++ b/content/child/indexed_db/webidbcursor_impl.h
@@ -47,6 +47,7 @@ class CONTENT_EXPORT WebIDBCursorImpl
  private:
   FRIEND_TEST_ALL_PREFIXES(WebIDBCursorImplTest, PrefetchTest);
   FRIEND_TEST_ALL_PREFIXES(WebIDBCursorImplTest, AdvancePrefetchTest);
+  FRIEND_TEST_ALL_PREFIXES(WebIDBCursorImplTest, PrefetchReset);
 
   int32 ipc_cursor_id_;
 
diff --git a/content/child/indexed_db/webidbcursor_impl_unittest.cc b/content/child/indexed_db/webidbcursor_impl_unittest.cc
index 14842df..1115950 100644
--- a/content/child/indexed_db/webidbcursor_impl_unittest.cc
+++ b/content/child/indexed_db/webidbcursor_impl_unittest.cc
@@ -30,6 +30,8 @@ class MockDispatcher : public IndexedDBDispatcher {
       : IndexedDBDispatcher(thread_safe_sender),
         prefetch_calls_(0),
         last_prefetch_count_(0),
+        reset_calls_(0),
+        last_used_count_(0),
         advance_calls_(0),
         continue_calls_(0),
         destroyed_cursor_id_(0) {}
@@ -42,6 +44,13 @@ class MockDispatcher : public IndexedDBDispatcher {
     callbacks_.reset(callbacks);
   }
 
+  virtual void RequestIDBCursorPrefetchReset(int used_prefetches,
+                                             int unused_prefetches,
+                                             int32 ipc_cursor_id) OVERRIDE {
+    ++reset_calls_;
+    last_used_count_ = used_prefetches;
+  }
+
   virtual void RequestIDBCursorAdvance(unsigned long count,
                                        WebIDBCallbacks* callbacks,
                                        int32 ipc_cursor_id) OVERRIDE {
@@ -62,14 +71,18 @@ class MockDispatcher : public IndexedDBDispatcher {
   }
 
   int prefetch_calls() { return prefetch_calls_; }
+  int last_prefetch_count() { return last_prefetch_count_; }
+  int reset_calls() { return reset_calls_; }
+  int last_used_count() { return last_used_count_; }
   int advance_calls() { return advance_calls_; }
   int continue_calls() { return continue_calls_; }
-  int last_prefetch_count() { return last_prefetch_count_; }
   int32 destroyed_cursor_id() { return destroyed_cursor_id_; }
 
  private:
   int prefetch_calls_;
   int last_prefetch_count_;
+  int reset_calls_;
+  int last_used_count_;
   int advance_calls_;
   int continue_calls_;
   int32 destroyed_cursor_id_;
@@ -247,4 +260,48 @@ TEST_F(WebIDBCursorImplTest, AdvancePrefetchTest) {
             dispatcher_->continue_calls());
 }
 
+TEST_F(WebIDBCursorImplTest, PrefetchReset) {
+  WebIDBCursorImpl cursor(WebIDBCursorImpl::kInvalidCursorId,
+                          thread_safe_sender_.get());
+
+  // Call continue() until prefetching should kick in.
+  int continue_calls = 0;
+  EXPECT_EQ(dispatcher_->continue_calls(), 0);
+  for (int i = 0; i < WebIDBCursorImpl::kPrefetchContinueThreshold; ++i) {
+    cursor.continueFunction(null_key_, new MockContinueCallbacks());
+    EXPECT_EQ(++continue_calls, dispatcher_->continue_calls());
+    EXPECT_EQ(0, dispatcher_->prefetch_calls());
+  }
+
+  // Initiate the prefetch
+  cursor.continueFunction(null_key_, new MockContinueCallbacks());
+  EXPECT_EQ(continue_calls, dispatcher_->continue_calls());
+  EXPECT_EQ(1, dispatcher_->prefetch_calls());
+  EXPECT_EQ(0, dispatcher_->reset_calls());
+
+  // Now invalidate it
+  cursor.ResetPrefetchCache();
+
+  // No reset should have been sent since nothing has been received yet.
+  EXPECT_EQ(0, dispatcher_->reset_calls());
+
+  // Fill the prefetch cache as requested.
+  int prefetch_count = dispatcher_->last_prefetch_count();
+  std::vector<IndexedDBKey> keys(prefetch_count);
+  std::vector<IndexedDBKey> primary_keys(prefetch_count);
+  std::vector<WebData> values(prefetch_count);
+  cursor.SetPrefetchData(keys, primary_keys, values);
+
+  // No reset should have been sent since prefetch data hasn't been used.
+  EXPECT_EQ(0, dispatcher_->reset_calls());
+
+  // The real dispatcher would call cursor->CachedContinue(), so do that:
+  scoped_ptr<WebIDBCallbacks> callbacks(new MockContinueCallbacks());
+  cursor.CachedContinue(callbacks.get());
+
+  // Now the cursor should have reset the rest of the cache.
+  EXPECT_EQ(1, dispatcher_->reset_calls());
+  EXPECT_EQ(1, dispatcher_->last_used_count());
+}
+
 }  // namespace content