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 a5321942a7dd26d504c33ce033970ae621f105c1..ea9eb81167b165368077e9fac9f1b6147a4b009e 100644
--- a/webkit/dom_storage/dom_storage_area.cc
+++ b/webkit/dom_storage/dom_storage_area.cc
@@ -15,6 +15,8 @@
 #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"
 
@@ -60,7 +62,8 @@ DomStorageArea::DomStorageArea(const GURL& origin, const FilePath& directory,
       map_(new DomStorageMap(kPerAreaQuota + kPerAreaOverQuotaAllowance)),
       is_initial_import_done_(true),
       is_shutdown_(false),
-      commit_batches_in_flight_(0) {
+      commit_batches_in_flight_(0),
+      deletion_in_progress_(false) {
   if (!directory.empty()) {
     FilePath path = directory.Append(DatabaseFileNameFromOrigin(origin_));
     backing_.reset(new LocalStorageDatabaseAdapter(path));
@@ -72,16 +75,24 @@ DomStorageArea::DomStorageArea(
     int64 namespace_id,
     const std::string& persistent_namespace_id,
     const GURL& origin,
+    SessionStorageDatabase* session_storage_backing,
     DomStorageTaskRunner* task_runner)
     : namespace_id_(namespace_id),
       persistent_namespace_id_(persistent_namespace_id),
       origin_(origin),
       task_runner_(task_runner),
       map_(new DomStorageMap(kPerAreaQuota + kPerAreaOverQuotaAllowance)),
+      session_storage_backing_(session_storage_backing),
       is_initial_import_done_(true),
       is_shutdown_(false),
-      commit_batches_in_flight_(0) {
+      commit_batches_in_flight_(0),
+      deletion_in_progress_(false) {
   DCHECK(namespace_id != kLocalStorageNamespaceId);
+  if (session_storage_backing) {
+    backing_.reset(new SessionStorageDatabaseAdapter(
+        session_storage_backing, persistent_namespace_id, origin));
+    is_initial_import_done_ = false;
+  }
 }
 
 DomStorageArea::~DomStorageArea() {
@@ -168,13 +179,20 @@ DomStorageArea* DomStorageArea::ShallowCopy(
     const std::string& destination_persistent_namespace_id) {
   DCHECK_NE(kLocalStorageNamespaceId, namespace_id_);
   DCHECK_NE(kLocalStorageNamespaceId, destination_namespace_id);
-  DCHECK(!backing_.get());  // SessionNamespaces aren't stored on disk.
 
   DomStorageArea* copy = new DomStorageArea(
       destination_namespace_id, destination_persistent_namespace_id, origin_,
-      task_runner_);
+      session_storage_backing_, task_runner_);
   copy->map_ = map_;
   copy->is_shutdown_ = is_shutdown_;
+  copy->is_initial_import_done_ = true;
+
+  // All the uncommitted changes to this area need to happen before the actual
+  // shallow copy is made (scheduled by the upper layer). Another OnCommitTimer
+  // call might be in the event queue at this point, but it's handled gracefully
+  // when it fires.
+  if (commit_batch_.get())
+    OnCommitTimer();
   return copy;
 }
 
@@ -195,10 +213,24 @@ void DomStorageArea::DeleteOrigin() {
     return;
   }
   map_ = new DomStorageMap(kPerAreaQuota + kPerAreaOverQuotaAllowance);
-  if (backing_.get()) {
+  if (backing_.get() && !session_storage_backing_.get()) {
+    // This is localStorage.
     is_initial_import_done_ = false;
     backing_->Reset();
     backing_->DeleteFiles();
+  } else if (session_storage_backing_.get()) {
+    // No need to read the data (there will be no data), also, the
+    // PRIMARY_SEQUENCE thread shouldn't try to read the data while
+    // SessionStorageDatabase::DeleteArea is in progress.
+    is_initial_import_done_ = true;
+    {
+      base::AutoLock lock(deletion_in_progress_lock_);

[michaeln, 2012/07/10 01:16:58]

What does the flag+lock accomplish given that is_initial_import_done_ is true?
Looks like it's to avoid importing while deleting, but i think the
is_import_done flag guards against that already.

Also, i think this method may actually be unreachable in the case of session
storage atm? Namespaces get deleted wholesale, but there's no codepath or
usecase for deleting an individual origin out of a session namespace just yet
(see DomStorageNamespace for the DeleteOrigin callsite). If that's right, you
may want to just DCHECK that assumption here instead.

[marja, 2012/07/10 13:40:52]

The lock was guarding the bool (I didn't want to assume that assigning a bool is
atomic).

This is indeed not wired in -> removed this code (and the deletion_in_progress_,
deletion_in_progress_lock_) and added a DCHECK here.

+      deletion_in_progress_ = true;
+    }
+    task_runner_->PostShutdownBlockingTask(
+        FROM_HERE,
+        DomStorageTaskRunner::COMMIT_SEQUENCE,
+        base::Bind(&DomStorageArea::DeleteOriginInCommitSequence, this));
   }
 }
 
@@ -236,13 +268,22 @@ void DomStorageArea::InitialImportIfNeeded() {
   if (is_initial_import_done_)
     return;
 
-  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
+  is_initial_import_done_ = true;
+  {
+    base::AutoLock lock(deletion_in_progress_lock_);
+    if (deletion_in_progress_) {
+      // The data is getting deleted in the commit sequence; we shouldn't read
+      // it just before deletion. The data will be empty, so it's ok to not read
+      // anything.
+      return;
+    }
+  }
+
   DCHECK(backing_.get());
 
   ValuesMap initial_values;
   backing_->ReadAllValues(&initial_values);
   map_->SwapValues(&initial_values);
-  is_initial_import_done_ = true;
 }
 
 DomStorageArea::CommitBatch* DomStorageArea::CreateCommitBatchIfNeeded() {
@@ -264,13 +305,15 @@ DomStorageArea::CommitBatch* DomStorageArea::CreateCommitBatchIfNeeded() {
 }
 
 void DomStorageArea::OnCommitTimer() {
-  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
   if (is_shutdown_)
     return;
 
   DCHECK(backing_.get());
-  DCHECK(commit_batch_.get());
-  DCHECK(!commit_batches_in_flight_);
+
+  // It's possible that there is nothing to commit, since a shallow copy occured
+  // before the timer fired.
+  if (!commit_batch_.get())
+    return;
 
   // This method executes on the primary sequence, we schedule
   // a task for immediate execution on the commit sequence.
@@ -298,9 +341,9 @@ void DomStorageArea::CommitChanges(const CommitBatch* commit_batch) {
 void DomStorageArea::OnCommitComplete() {
   // We're back on the primary sequence in this method.
   DCHECK(task_runner_->IsRunningOnPrimarySequence());
+  --commit_batches_in_flight_;
   if (is_shutdown_)
     return;
-  --commit_batches_in_flight_;
   if (commit_batch_.get() && !commit_batches_in_flight_) {
     // More changes have accrued, restart the timer.
     task_runner_->PostDelayedTask(
@@ -310,6 +353,12 @@ void DomStorageArea::OnCommitComplete() {
   }
 }
 
+void DomStorageArea::DeleteOriginInCommitSequence() {
+  session_storage_backing_->DeleteArea(persistent_namespace_id_, origin_);
+  base::AutoLock lock(deletion_in_progress_lock_);
+  deletion_in_progress_ = false;
+}
+
 void DomStorageArea::ShutdownInCommitSequence() {
   // This method executes on the commit sequence.
   DCHECK(task_runner_->IsRunningOnCommitSequence());
@@ -323,6 +372,7 @@ void DomStorageArea::ShutdownInCommitSequence() {
   }
   commit_batch_.reset();
   backing_.reset();
+  session_storage_backing_ = NULL;
 }
 
 }  // namespace dom_storage