Persist sessionStorage on disk.

webkit/dom_storage/dom_storage_area.cc

Index: webkit/dom_storage/dom_storage_area.cc
diff --git a/webkit/dom_storage/dom_storage_area.cc b/webkit/dom_storage/dom_storage_area.cc
index ed4885b7b078f8e01f368fb944db7f066ff6a9df..8f87e726ff0a9c900b0e64893c75dd298ac426a0 100644
--- a/webkit/dom_storage/dom_storage_area.cc
+++ b/webkit/dom_storage/dom_storage_area.cc
@@ -5,16 +5,19 @@
 #include "webkit/dom_storage/dom_storage_area.h"
 
 #include "base/bind.h"
-#include "base/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "webkit/database/database_util.h"
+#include "webkit/dom_storage/dom_storage_database.h"
 #include "webkit/dom_storage/dom_storage_map.h"
 #include "webkit/dom_storage/dom_storage_namespace.h"
 #include "webkit/dom_storage/dom_storage_task_runner.h"
 #include "webkit/dom_storage/dom_storage_types.h"
+#include "webkit/dom_storage/local_storage_database_adapter.h"
+#include "webkit/dom_storage/session_storage_database.h"
+#include "webkit/dom_storage/session_storage_database_adapter.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/glue/webkit_glue.h"
 
@@ -25,7 +28,8 @@ namespace dom_storage {
 static const int kCommitTimerSeconds = 1;
 
 DomStorageArea::CommitBatch::CommitBatch()
-  : clear_all_first(false) {
+    : clear_all_first(false),
+      clear_all_after_deep_copy(false) {
 }
 DomStorageArea::CommitBatch::~CommitBatch() {}
 
@@ -59,13 +63,40 @@ DomStorageArea::DomStorageArea(
       directory_(directory),
       task_runner_(task_runner),
       map_(new DomStorageMap(kPerAreaQuota)),
+      is_initial_import_done_(false),
+      is_shutdown_(false),
+      is_shallow_copy_(false) {
+  DCHECK(!directory.empty());
+  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id);
+  FilePath path = directory.Append(DatabaseFileNameFromOrigin(origin_));
+  backing_.reset(new LocalStorageDatabaseAdapter(new DomStorageDatabase(path)));
+}
+
+DomStorageArea::DomStorageArea(
+    int64 namespace_id, const GURL& origin,
+    SessionStorageDatabase* session_storage_backing,
+    DomStorageTaskRunner* task_runner)
+    : namespace_id_(namespace_id), origin_(origin),
+      task_runner_(task_runner),
+      map_(new DomStorageMap(kPerAreaQuota)),
+      session_storage_backing_(session_storage_backing),
+      is_initial_import_done_(false),
+      is_shutdown_(false),
+      is_shallow_copy_(false) {
+  DCHECK(namespace_id != kLocalStorageNamespaceId);
+  backing_.reset(new SessionStorageDatabaseAdapter(
+      session_storage_backing, namespace_id, origin));
+}
+
+DomStorageArea::DomStorageArea(int64 namespace_id,
+                               const GURL& origin,
+                               DomStorageTaskRunner* task_runner)
+    : namespace_id_(namespace_id), origin_(origin),
+      task_runner_(task_runner),
+      map_(new DomStorageMap(kPerAreaQuota)),
       is_initial_import_done_(true),
-      is_shutdown_(false) {
-  if (namespace_id == kLocalStorageNamespaceId && !directory.empty()) {
-    FilePath path = directory.Append(DatabaseFileNameFromOrigin(origin_));
-    backing_.reset(new DomStorageDatabase(path));
-    is_initial_import_done_ = false;
-  }
+      is_shutdown_(false),
+      is_shallow_copy_(false) {
 }
 
 DomStorageArea::~DomStorageArea() {
@@ -98,12 +129,32 @@ bool DomStorageArea::SetItem(const string16& key,
   if (is_shutdown_)
     return false;
   InitialImportIfNeeded();
-  if (!map_->HasOneRef())
-    map_ = map_->DeepCopy();
+  bool was_shallow_copy = is_shallow_copy_;
+  if (is_shallow_copy_) {
+    // It's possible that the other DomStorageAreas referring to the same map
+    // already made a deep copy. If that's the case, we don't need to do
+    // anything.

[michaeln, 2012/04/22 21:43:35]

i'd rather depend on HasOneRef() for this and nix the new data member

[marja, 2012/05/11 12:18:32]

Done.

+    if (!map_->HasOneRef())
+      map_ = map_->DeepCopy();
+    is_shallow_copy_ = false;
+  }
   bool success = map_->SetItem(key, value, old_value);
   if (success && backing_.get()) {
     CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
-    commit_batch->changed_values[key] = NullableString16(value, false);
+    // If this area was a shallow copy, the existing changes in the commit batch
+    // should be committed before doing a deep copy. This change, and all the
+    // changes after this, should be committed after the deep copy. The database
+    // takes care of not doing a deep copy of a map which is only referred to
+    // once.

[michaeln, 2012/04/22 21:43:35]

It might be a simplification to flush pending changes prior to deep copying?
Could keep the commit batch structure simpler and the logic to set them up
simpler. I think we'd need a queue of in_flight_commit_batches_ instead of just
one.

bool DomStorageArea::MutatorOfSomeKind() {
  if (is_shutdown_)
    return false;
  InitialImportIfNeeded();

  bool needs_deep_copy = !map_->HasOneRef();
  if (needs_deep_copy ) {
    map_ = map_->DeepCopy();
    if (HasUncommittedChanges())
      OnCommitTimer();  // flushes pending changes
    DCHECK(!commit_batch_);
  }

  // mutate map_ as needed

  if ((success || needs_deep_copy) && backing_get()) {
    CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
    commit_batch->needs_deep_copy = needs_deep_copy;
    if (success) {
      // mutate batch->changed_values as needed
    }
  }
}

wdyt?

[michaeln, 2012/04/22 23:48:02]

or maybe a pattern like this that factors out more of the boiler plate

bool DomStorageArea::MutatorOfSomeKind() {
  if (is_shutdown_)
    return false;
  InitialImportIfNeeded();
  DeepCopyIfNeeded();

  // mutate map_ as needed, code stays the same as before

  if (success && backing_get()) {
    CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
    // code stays the same as before
  }
}

void DomStorageArea::DeepCopyIfNeeded() {
  if (!map_->HasOneRef()) {
    map_ = map_->DeepCopy();
    if (HasUncommittedChanges())
      OnCommitTimer();  // flushes pending changes
    DCHECK(!commit_batch_);
    CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
    commit_batch->needs_deep_copy = true;
  }
}

[marja, 2012/05/11 12:18:32]

- The deep copy problem was solved by http://codereview.chromium.org/10378066/
- The "multiple commits in flight at the same time" possibility was added by
http://codereview.chromium.org/10389061/
- And I modified this CL to queue the things in the right order: When
ShallowCopy() is called, we queue committing whatever is in commit_batch_, then
the upper layer (Namespace) schedules making the clone, and if changes happen
after these, they'll be committed after the clone.

[michaeln, 2012/05/16 08:10:55]

sgtm!

+    if (was_shallow_copy ||
+        !commit_batch->changed_values_after_deep_copy.empty() ||
+        commit_batch->clear_all_after_deep_copy) {
+      commit_batch->changed_values_after_deep_copy[key] =
+          NullableString16(value, false);
+    }
+    else {
+      commit_batch->changed_values[key] = NullableString16(value, false);
+    }
   }
   return success;
 }
@@ -112,12 +163,24 @@ bool DomStorageArea::RemoveItem(const string16& key, string16* old_value) {
   if (is_shutdown_)
     return false;
   InitialImportIfNeeded();
-  if (!map_->HasOneRef())
-    map_ = map_->DeepCopy();
+  // See comments in SetItem.
+  bool was_shallow_copy = is_shallow_copy_;
+  if (is_shallow_copy_) {
+    if (!map_->HasOneRef())
+      map_ = map_->DeepCopy();
+    is_shallow_copy_ = false;
+  }
   bool success = map_->RemoveItem(key, old_value);
   if (success && backing_.get()) {
     CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
-    commit_batch->changed_values[key] = NullableString16(true);
+    if (was_shallow_copy ||
+        !commit_batch->changed_values_after_deep_copy.empty() ||
+        commit_batch->clear_all_after_deep_copy) {
+      commit_batch->changed_values_after_deep_copy[key] =
+          NullableString16(true);
+    } else {
+      commit_batch->changed_values[key] = NullableString16(true);
+    }
   }
   return success;
 }
@@ -129,12 +192,22 @@ bool DomStorageArea::Clear() {
   if (map_->Length() == 0)
     return false;
 
+  // See comments in SetItem.
+  bool was_shallow_copy = is_shallow_copy_;
+  is_shallow_copy_ = false;
   map_ = new DomStorageMap(kPerAreaQuota);
 
   if (backing_.get()) {
     CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
-    commit_batch->clear_all_first = true;
-    commit_batch->changed_values.clear();
+    if (was_shallow_copy ||
+        !commit_batch->changed_values_after_deep_copy.empty() ||
+        commit_batch->clear_all_after_deep_copy) {
+      commit_batch->clear_all_after_deep_copy = true;
+      commit_batch->changed_values_after_deep_copy.clear();
+    } else {
+      commit_batch->clear_all_first = true;
+      commit_batch->changed_values.clear();
+    }
   }
 
   return true;
@@ -143,12 +216,27 @@ bool DomStorageArea::Clear() {
 DomStorageArea* DomStorageArea::ShallowCopy(int64 destination_namespace_id) {
   DCHECK_NE(kLocalStorageNamespaceId, namespace_id_);
   DCHECK_NE(kLocalStorageNamespaceId, destination_namespace_id);
-  DCHECK(!backing_.get());  // SessionNamespaces aren't stored on disk.
+  DCHECK(session_storage_backing_.get());
 
-  DomStorageArea* copy = new DomStorageArea(destination_namespace_id, origin_,
-                                            FilePath(), task_runner_);
+  DomStorageArea* copy = new DomStorageArea(
+      destination_namespace_id, origin_, session_storage_backing_,
+      task_runner_);
+  copy->is_shallow_copy_ = true;
   copy->map_ = map_;
   copy->is_shutdown_ = is_shutdown_;
+
+  // Also this area is now a shallow copy; changes to either of these 2 areas
+  // will initiate deep copying.
+  is_shallow_copy_ = true;
+
+  if (session_storage_backing_.get()) {
+    // If this area has uncommitted changes, they need to happen before
+    // DomStorageNamespace does the shallow-copying of the database. Also, if
+    // the area is empty, we need to ensure that a placeholder for it exists in
+    // the database. An empty commit batch achieves this.

[michaeln, 2012/04/22 21:43:35]

Does the empty placeholder really need to be there? Could this be conditional...

    if (HasUncommittedChanges())
      OnCommitTimer();  // flushes pending changes

[marja, 2012/05/11 12:18:32]

Done. Empty placeholder is not needed any more.

+    CreateCommitBatchIfNeeded();
+    OnCommitTimer();
+  }
   return copy;
 }
 
@@ -169,12 +257,19 @@ void DomStorageArea::DeleteOrigin() {
     return;
   }
   map_ = new DomStorageMap(kPerAreaQuota);
-  if (backing_.get()) {
+  if (backing_.get() && !session_storage_backing_.get()) {
+    // This is localStorage.
     is_initial_import_done_ = false;
-    backing_.reset(new DomStorageDatabase(backing_->file_path()));
-    file_util::Delete(backing_->file_path(), false);
-    file_util::Delete(
-        DomStorageDatabase::GetJournalFilePath(backing_->file_path()), false);
+    backing_->Reset();
+    backing_->DeleteFiles();
+  } else if (session_storage_backing_.get()) {
+    // Delete the origin from the session storage by creating an empty commit
+    // batch.
+    CommitBatch* commit_batch = CreateCommitBatchIfNeeded();
+    if (is_shallow_copy_)
+      commit_batch->clear_all_after_deep_copy = true;
+    else
+      commit_batch->clear_all_first = true;
   }
 }
 
@@ -191,7 +286,7 @@ void DomStorageArea::PurgeMemory() {
 
   // Recreate the database object, this frees up the open sqlite connection
   // and its page cache.
-  backing_.reset(new DomStorageDatabase(backing_->file_path()));
+  backing_->Reset();
 }
 
 void DomStorageArea::Shutdown() {
@@ -212,7 +307,6 @@ void DomStorageArea::InitialImportIfNeeded() {
   if (is_initial_import_done_)
     return;
 
-  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
   DCHECK(backing_.get());
 
   ValuesMap initial_values;
@@ -240,12 +334,15 @@ DomStorageArea::CommitBatch* DomStorageArea::CreateCommitBatchIfNeeded() {
 }
 
 void DomStorageArea::OnCommitTimer() {
-  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
   if (is_shutdown_)
     return;
 
   DCHECK(backing_.get());
-  DCHECK(commit_batch_.get());
+  // It's possible that there is nothing to commit, since a shallow copy occured
+  // before the timer fired.
+  if (!commit_batch_.get())
+    return;
+
   DCHECK(!in_flight_commit_batch_.get());
 
   // This method executes on the primary sequence, we schedule
@@ -267,6 +364,23 @@ void DomStorageArea::CommitChanges() {
       in_flight_commit_batch_->clear_all_first,
       in_flight_commit_batch_->changed_values);
   DCHECK(success);  // TODO(michaeln): what if it fails?
+  if (session_storage_backing_.get() &&
+      (in_flight_commit_batch_->clear_all_after_deep_copy ||
+       !in_flight_commit_batch_->changed_values_after_deep_copy.empty())) {
+    if (in_flight_commit_batch_->clear_all_after_deep_copy) {
+      // Shortcut: If we're going to make a deep copy and clear the area, no
+      // need to create the deep copy first. We will just break the association
+      // to the existing map. A new map will be created when data is inserted
+      // into it.
+      session_storage_backing_->DisassociateMap(namespace_id_, origin_);
+    } else {
+      session_storage_backing_->DeepCopy(namespace_id_, origin_);
+    }
+    bool success = backing_->CommitChanges(
+        false, in_flight_commit_batch_->changed_values_after_deep_copy);
+    DCHECK(success);  // TODO(michaeln): what if it fails?
+  }
+
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DomStorageArea::OnCommitComplete, this));
@@ -301,6 +415,7 @@ void DomStorageArea::ShutdownInCommitSequence() {
   commit_batch_.reset();
   in_flight_commit_batch_.reset();
   backing_.reset();
+  session_storage_backing_ = NULL;
 }
 
 }  // namespace dom_storage