Revert 243344 "IndexedDB: Fix cursor prefetching edge cases"

> 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
> 
> Review URL: https://codereview.chromium.org/124323002

TBR=jsbell@chromium.org

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

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

content/browser/indexed_db/indexed_db_cursor.cc

@@ -108,7 +108,8 @@ void IndexedDBCursor::CursorPrefetchIterationOperation(
   std::vector<IndexedDBKey> found_primary_keys;
   std::vector<std::string> found_values;
 
-  saved_cursor_.reset();
+  if (cursor_)
+    saved_cursor_.reset(cursor_->Clone());
   const size_t max_size_estimate = 10 * 1024 * 1024;
   size_t size_estimate = 0;
 
@@ -118,12 +119,6 @@ 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());
 
@@ -157,8 +152,7 @@ void IndexedDBCursor::CursorPrefetchIterationOperation(
       found_keys, found_primary_keys, found_values);
 }
 
-void IndexedDBCursor::PrefetchReset(int used_prefetches,
-                                    int /* unused_prefetches */) {
+void IndexedDBCursor::PrefetchReset(int used_prefetches, int) {
   IDB_TRACE("IndexedDBCursor::PrefetchReset");
   cursor_.swap(saved_cursor_);
   saved_cursor_.reset();
@@ -166,9 +160,7 @@ void IndexedDBCursor::PrefetchReset(int used_prefetches,
   if (closed_)
     return;
   if (cursor_) {
-    // First prefetched result is always used.
-    DCHECK_GT(used_prefetches, 0);
-    for (int i = 0; i < used_prefetches - 1; ++i) {
+    for (int i = 0; i < used_prefetches; ++i) {
       bool ok = cursor_->Continue();
       DCHECK(ok);
     }

content/child/indexed_db/indexed_db_dispatcher.h

@@ -98,10 +98,9 @@ class CONTENT_EXPORT IndexedDBDispatcher
                                         blink::WebIDBCallbacks* callbacks_ptr,
                                         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 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,14 +161,6 @@ 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,7 +47,6 @@ 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,8 +30,6 @@ 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) {}
@@ -44,13 +42,6 @@ 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 {
@@ -71,18 +62,14 @@ 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_;
@@ -260,47 +247,4 @@ 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:
-  cursor.CachedContinue(new MockContinueCallbacks());
-
-  // 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