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

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);
     }

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);

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);

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_;
 

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