IndexedDB: Optimize range deletion operations (e.g. clearing a store)

content/browser/indexed_db/leveldb/leveldb_transaction.h

Index: content/browser/indexed_db/leveldb/leveldb_transaction.h
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.h b/content/browser/indexed_db/leveldb/leveldb_transaction.h
index 33392f235336abd55f607ec406250a6c66014113..2c8ec4335702b7a2093a8101c9e97c0dd46add85 100644
--- a/content/browser/indexed_db/leveldb/leveldb_transaction.h
+++ b/content/browser/indexed_db/leveldb/leveldb_transaction.h
@@ -20,6 +20,7 @@
 
 namespace content {
 
+class LevelDBTransactionRangeTest;
 class LevelDBWriteBatch;
 

[dmurph, 2017/03/01 20:06:35]

Because this took me a while to understand:

Can you add documentation describing what the TransactionIterator vs
DataIterator is? To me it looks like DataIterator is for data that is
temporarily in the transaction, and TransactionIterator is actually reading
things from leveldb?

[jsbell, 2017/03/01 22:45:25]

Done - feedback welcome!

 class CONTENT_EXPORT LevelDBTransaction
@@ -27,9 +28,12 @@ class CONTENT_EXPORT LevelDBTransaction
  public:
   void Put(const base::StringPiece& key, std::string* value);
 
-  // Returns true if this operation performs a change, where the value wasn't
-  // already deleted.
-  bool Remove(const base::StringPiece& key);
+  void Remove(const base::StringPiece& key);
+
+  leveldb::Status RemoveRange(const base::StringPiece& begin,
+                              const base::StringPiece& end,
+                              bool upper_open);
+
   virtual leveldb::Status Get(const base::StringPiece& key,
                               std::string* value,
                               bool* found);
@@ -45,16 +49,17 @@ class CONTENT_EXPORT LevelDBTransaction
 
  private:
   friend class base::RefCounted<LevelDBTransaction>;
-  FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, Transaction);
-  FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, TransactionCommitTest);
-  FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, TransactionIterator);
+  friend class content::LevelDBTransactionRangeTest;
+  FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, GetAndPut);
+  FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, Commit);
+  FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, Iterator);
 
   struct Record {
     Record();
     ~Record();
     std::string key;
     std::string value;
-    bool deleted;
+    bool deleted = false;
   };
 
   class Comparator {
@@ -88,6 +93,9 @@ class CONTENT_EXPORT LevelDBTransaction
     base::StringPiece Value() const override;
     bool IsDeleted() const;
 
+    // Mark the current record as deleted.
+    void Delete();
+
    private:
     explicit DataIterator(LevelDBTransaction* transaction);
     DataType* data_;
@@ -111,6 +119,11 @@ class CONTENT_EXPORT LevelDBTransaction
     base::StringPiece Value() const override;
     void DataChanged();
 
+    // If the current iterator is the data iterator, mark the record as
+    // deleted and return true. Otherwise return false - the caller may want
+    // to insert a deletion record.
+    bool DeleteData();

[dmurph, 2017/03/01 20:06:35]

Better name? "MaybeDeleteOnDataIterator"?

[jsbell, 2017/03/01 22:45:25]

I realized I could pull the non-DataIterator case in here so I did that,
simplifying the logic here. PTAL?

+
    private:
     enum Direction { FORWARD, REVERSE };
 
@@ -126,15 +139,15 @@ class CONTENT_EXPORT LevelDBTransaction
     const LevelDBComparator* comparator_;
     mutable std::unique_ptr<DataIterator> data_iterator_;
     std::unique_ptr<LevelDBIterator> db_iterator_;
-    LevelDBIterator* current_;
+    LevelDBIterator* current_ = nullptr;
 
-    Direction direction_;
-    mutable bool data_changed_;
+    Direction direction_ = FORWARD;
+    mutable bool data_changed_ = false;
 
     DISALLOW_COPY_AND_ASSIGN(TransactionIterator);
   };
-  // Returns true if the key was originally marked deleted, false otherwise.
-  bool Set(const base::StringPiece& key, std::string* value, bool deleted);
+
+  void Set(const base::StringPiece& key, std::string* value, bool deleted);
   void RegisterIterator(TransactionIterator* iterator);
   void UnregisterIterator(TransactionIterator* iterator);
   void NotifyIterators();
@@ -144,7 +157,7 @@ class CONTENT_EXPORT LevelDBTransaction
   const LevelDBComparator* comparator_;
   Comparator data_comparator_;
   DataType data_;
-  bool finished_;
+  bool finished_ = false;
   std::set<TransactionIterator*> iterators_;
 
   DISALLOW_COPY_AND_ASSIGN(LevelDBTransaction);
@@ -169,7 +182,7 @@ class LevelDBDirectTransaction {
 
   LevelDBDatabase* db_;
   std::unique_ptr<LevelDBWriteBatch> write_batch_;
-  bool finished_;
+  bool finished_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(LevelDBDirectTransaction);
 };