DomStorageArea: Allow multiple simultaneous in flight commit batches.

Currently, only one commit batch will be in flight at given time. This change is
needed to be able to support shallow copies of file-backed sessionStorage.

BUG=104292
TEST=Existing DomStorageAreaTest.* tests.


Review URL: https://chromiumcodereview.appspot.com/10389061

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

3 files changed, 24 insertions, 25 deletions

diff --git a/webkit/dom_storage/dom_storage_area.cc b/webkit/dom_storage/dom_storage_area.cc
index f589d88..43f22f1 100644
--- a/webkit/dom_storage/dom_storage_area.cc
+++ b/webkit/dom_storage/dom_storage_area.cc
@@ -60,7 +60,8 @@ DomStorageArea::DomStorageArea(
       task_runner_(task_runner),
       map_(new DomStorageMap(kPerAreaQuota)),
       is_initial_import_done_(true),
-      is_shutdown_(false) {
+      is_shutdown_(false),
+      commit_batches_in_flight_(0) {
   if (namespace_id == kLocalStorageNamespaceId && !directory.empty()) {
     FilePath path = directory.Append(DatabaseFileNameFromOrigin(origin_));
     backing_.reset(new DomStorageDatabase(path));
@@ -161,7 +162,7 @@ DomStorageArea* DomStorageArea::ShallowCopy(int64 destination_namespace_id) {
 
 bool DomStorageArea::HasUncommittedChanges() const {
   DCHECK(!is_shutdown_);
-  return commit_batch_.get() || in_flight_commit_batch_.get();
+  return commit_batch_.get() || commit_batches_in_flight_;
 }
 
 void DomStorageArea::DeleteOrigin() {
@@ -233,10 +234,10 @@ DomStorageArea::CommitBatch* DomStorageArea::CreateCommitBatchIfNeeded() {
   if (!commit_batch_.get()) {
     commit_batch_.reset(new CommitBatch());
 
-    // Start a timer to commit any changes that accrue in the batch,
-    // but only if a commit is not currently in flight. In that case
-    // the timer will be started after the current commit has happened.
-    if (!in_flight_commit_batch_.get()) {
+    // Start a timer to commit any changes that accrue in the batch, but only if
+    // no commits are currently in flight. In that case the timer will be
+    // started after the commits have happened.
+    if (!commit_batches_in_flight_) {
       task_runner_->PostDelayedTask(
           FROM_HERE,
           base::Bind(&DomStorageArea::OnCommitTimer, this),
@@ -253,26 +254,25 @@ void DomStorageArea::OnCommitTimer() {
 
   DCHECK(backing_.get());
   DCHECK(commit_batch_.get());
-  DCHECK(!in_flight_commit_batch_.get());
+  DCHECK(!commit_batches_in_flight_);
 
   // This method executes on the primary sequence, we schedule
   // a task for immediate execution on the commit sequence.
   DCHECK(task_runner_->IsRunningOnPrimarySequence());
-  in_flight_commit_batch_ = commit_batch_.Pass();
   bool success = task_runner_->PostShutdownBlockingTask(
       FROM_HERE,
       DomStorageTaskRunner::COMMIT_SEQUENCE,
-      base::Bind(&DomStorageArea::CommitChanges, this));
+      base::Bind(&DomStorageArea::CommitChanges, this,
+                 base::Owned(commit_batch_.release())));
+  ++commit_batches_in_flight_;
   DCHECK(success);
 }
 
-void DomStorageArea::CommitChanges() {
+void DomStorageArea::CommitChanges(const CommitBatch* commit_batch) {
   // This method executes on the commit sequence.
   DCHECK(task_runner_->IsRunningOnCommitSequence());
-  DCHECK(in_flight_commit_batch_.get());
-  bool success = backing_->CommitChanges(
-      in_flight_commit_batch_->clear_all_first,
-      in_flight_commit_batch_->changed_values);
+  bool success = backing_->CommitChanges(commit_batch->clear_all_first,
+                                         commit_batch->changed_values);
   DCHECK(success);  // TODO(michaeln): what if it fails?
   task_runner_->PostTask(
       FROM_HERE,
@@ -284,8 +284,8 @@ void DomStorageArea::OnCommitComplete() {
   DCHECK(task_runner_->IsRunningOnPrimarySequence());
   if (is_shutdown_)
     return;
-  in_flight_commit_batch_.reset();
-  if (commit_batch_.get()) {
+  --commit_batches_in_flight_;
+  if (commit_batch_.get() && !commit_batches_in_flight_) {
     // More changes have accrued, restart the timer.
     task_runner_->PostDelayedTask(
         FROM_HERE,
@@ -306,7 +306,6 @@ void DomStorageArea::ShutdownInCommitSequence() {
     DCHECK(success);
   }
   commit_batch_.reset();
-  in_flight_commit_batch_.reset();
   backing_.reset();
 }
 
diff --git a/webkit/dom_storage/dom_storage_area.h b/webkit/dom_storage/dom_storage_area.h
index b08ff1e..af43c82 100644
--- a/webkit/dom_storage/dom_storage_area.h
+++ b/webkit/dom_storage/dom_storage_area.h
@@ -98,7 +98,7 @@ class DomStorageArea
   // task sequence when complete.
   CommitBatch* CreateCommitBatchIfNeeded();
   void OnCommitTimer();
-  void CommitChanges();
+  void CommitChanges(const CommitBatch* commit_batch);
   void OnCommitComplete();
 
   void ShutdownInCommitSequence();
@@ -112,7 +112,7 @@ class DomStorageArea
   bool is_initial_import_done_;
   bool is_shutdown_;
   scoped_ptr<CommitBatch> commit_batch_;
-  scoped_ptr<CommitBatch> in_flight_commit_batch_;
+  int commit_batches_in_flight_;
 };
 
 }  // namespace dom_storage
diff --git a/webkit/dom_storage/dom_storage_area_unittest.cc b/webkit/dom_storage/dom_storage_area_unittest.cc
index 12ccfd8..858eb23 100644
--- a/webkit/dom_storage/dom_storage_area_unittest.cc
+++ b/webkit/dom_storage/dom_storage_area_unittest.cc
@@ -38,7 +38,7 @@ class DomStorageAreaTest : public testing::Test {
   void InjectedCommitSequencingTask(DomStorageArea* area) {
     // At this point the OnCommitTimer has run.
     // Verify that it put a commit in flight.
-    EXPECT_TRUE(area->in_flight_commit_batch_.get());
+    EXPECT_EQ(1, area->commit_batches_in_flight_);
     EXPECT_FALSE(area->commit_batch_.get());
     EXPECT_TRUE(area->HasUncommittedChanges());
     // Make additional change and verify that a new commit batch
@@ -46,7 +46,7 @@ class DomStorageAreaTest : public testing::Test {
     NullableString16 old_value;
     EXPECT_TRUE(area->SetItem(kKey2, kValue2, &old_value));
     EXPECT_TRUE(area->commit_batch_.get());
-    EXPECT_TRUE(area->in_flight_commit_batch_.get());
+    EXPECT_EQ(1, area->commit_batches_in_flight_);
     EXPECT_TRUE(area->HasUncommittedChanges());
   }
 
@@ -174,12 +174,12 @@ TEST_F(DomStorageAreaTest, BackingDatabaseOpened) {
     ASSERT_TRUE(old_value.is_null());
     EXPECT_TRUE(area->is_initial_import_done_);
     EXPECT_TRUE(area->commit_batch_.get());
-    EXPECT_FALSE(area->in_flight_commit_batch_.get());
+    EXPECT_EQ(0, area->commit_batches_in_flight_);
 
     MessageLoop::current()->RunAllPending();
 
     EXPECT_FALSE(area->commit_batch_.get());
-    EXPECT_FALSE(area->in_flight_commit_batch_.get());
+    EXPECT_EQ(0, area->commit_batches_in_flight_);
     EXPECT_TRUE(area->backing_->IsOpen());
     EXPECT_EQ(1u, area->Length());
     EXPECT_EQ(kValue, area->GetItem(kKey).string());
@@ -226,7 +226,7 @@ TEST_F(DomStorageAreaTest, CommitTasks) {
   MessageLoop::current()->RunAllPending();
   EXPECT_FALSE(area->HasUncommittedChanges());
   EXPECT_FALSE(area->commit_batch_.get());
-  EXPECT_FALSE(area->in_flight_commit_batch_.get());
+  EXPECT_EQ(0, area->commit_batches_in_flight_);
   // Verify the changes made it to the database.
   values.clear();
   area->backing_->ReadAllValues(&values);
@@ -241,7 +241,7 @@ TEST_F(DomStorageAreaTest, CommitTasks) {
   EXPECT_TRUE(area->commit_batch_->changed_values.empty());
   MessageLoop::current()->RunAllPending();
   EXPECT_FALSE(area->commit_batch_.get());
-  EXPECT_FALSE(area->in_flight_commit_batch_.get());
+  EXPECT_EQ(0, area->commit_batches_in_flight_);
   // Verify the changes made it to the database.
   values.clear();
   area->backing_->ReadAllValues(&values);