Hook up DomStorageArea with a DomStorageDatabase. Uses DomStorageTaskRunner to asynchronously write to the database.

BUG=106763


Review URL: http://codereview.chromium.org/9389009

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

8 files changed, 290 insertions, 35 deletions

diff --git a/webkit/dom_storage/dom_storage_area.cc b/webkit/dom_storage/dom_storage_area.cc
index e394f64..3eb195c 100644
--- a/webkit/dom_storage/dom_storage_area.cc
+++ b/webkit/dom_storage/dom_storage_area.cc
@@ -3,57 +3,113 @@
 // found in the LICENSE file.
 
 #include "webkit/dom_storage/dom_storage_area.h"
+
+#include "base/bind.h"
+#include "base/time.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/fileapi/file_system_util.h"
 
 namespace dom_storage {
 
 DomStorageArea::DomStorageArea(
     int64 namespace_id, const GURL& origin,
     const FilePath& directory, DomStorageTaskRunner* task_runner)
-    : namespace_id_(namespace_id),
-      origin_(origin),
+    : namespace_id_(namespace_id), origin_(origin),
       directory_(directory),
       task_runner_(task_runner),
-      map_(new DomStorageMap(kPerAreaQuota)) {
+      map_(new DomStorageMap(kPerAreaQuota)),
+      backing_(NULL),
+      initial_import_done_(false),
+      clear_all_next_commit_(false),
+      commit_in_flight_(false) {
+
+  if (namespace_id == kLocalStorageNamespaceId && !directory.empty()) {
+    FilePath path = directory.Append(DatabaseFileNameFromOrigin(origin_));
+    backing_.reset(new DomStorageDatabase(path));
+  } else {
+    // Not a local storage area or no directory specified for backing
+    // database, (i.e. it's an incognito profile).
+    initial_import_done_ = true;
+  }
 }
 
 DomStorageArea::~DomStorageArea() {
+  if (clear_all_next_commit_ || !changed_values_.empty()) {
+    // Still some data left that was not committed to disk, try now.
+    // We do this regardless of whether we think a commit is in flight
+    // as there is no guarantee that that commit will actually get
+    // processed. For example the task_runner_'s message loop could
+    // unexpectedly quit before the delayed task is fired and leave the
+    // commit_in_flight_ flag set. But there's no way for us to determine
+    // that has happened so force a commit now.
+
+    CommitChanges();
+
+    // TODO(benm): It's possible that the commit failed, and in
+    // that case we're going to lose data. Integrate with UMA
+    // to gather stats about how often this actually happens,
+    // so that we can figure out a contingency plan.
+  }
 }
 
 unsigned DomStorageArea::Length() {
+  InitialImportIfNeeded();
   return map_->Length();
 }
 
 NullableString16 DomStorageArea::Key(unsigned index) {
+  InitialImportIfNeeded();
   return map_->Key(index);
 }
 
 NullableString16 DomStorageArea::GetItem(const string16& key) {
+  InitialImportIfNeeded();
   return map_->GetItem(key);
 }
 
-bool DomStorageArea::SetItem(
-    const string16& key, const string16& value,
-    NullableString16* old_value) {
+bool DomStorageArea::SetItem(const string16& key,
+                             const string16& value,
+                             NullableString16* old_value) {
+  InitialImportIfNeeded();
+
   if (!map_->HasOneRef())
     map_ = map_->DeepCopy();
-  return map_->SetItem(key, value, old_value);
+  bool success = map_->SetItem(key, value, old_value);
+  if (success && backing_.get()) {
+    changed_values_[key] = NullableString16(value, false);
+    ScheduleCommitChanges();
+  }
+  return success;
 }
 
-bool DomStorageArea::RemoveItem(
-    const string16& key,
-    string16* old_value) {
+bool DomStorageArea::RemoveItem(const string16& key, string16* old_value) {
+  InitialImportIfNeeded();
   if (!map_->HasOneRef())
     map_ = map_->DeepCopy();
-  return map_->RemoveItem(key, old_value);
+  bool success = map_->RemoveItem(key, old_value);
+  if (success && backing_.get()) {
+    changed_values_[key] = NullableString16(true);
+    ScheduleCommitChanges();
+  }
+  return success;
 }
 
 bool DomStorageArea::Clear() {
+  InitialImportIfNeeded();
   if (map_->Length() == 0)
     return false;
+
   map_ = new DomStorageMap(kPerAreaQuota);
+
+  if (backing_.get()) {
+    changed_values_.clear();
+    clear_all_next_commit_ = true;
+    ScheduleCommitChanges();
+  }
+
   return true;
 }
 
@@ -61,10 +117,56 @@ DomStorageArea* DomStorageArea::ShallowCopy(int64 destination_namespace_id) {
   DCHECK_NE(kLocalStorageNamespaceId, namespace_id_);
   DCHECK_NE(kLocalStorageNamespaceId, destination_namespace_id);
   // SessionNamespaces aren't backed by files on disk.
+  DCHECK(!backing_.get());
+
   DomStorageArea* copy = new DomStorageArea(destination_namespace_id, origin_,
                                             FilePath(), task_runner_);
   copy->map_ = map_;
   return copy;
 }
 
+void DomStorageArea::InitialImportIfNeeded() {
+  if (initial_import_done_)
+    return;
+
+  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
+  DCHECK(backing_.get());
+
+  ValuesMap initial_values;
+  backing_->ReadAllValues(&initial_values);
+  map_->SwapValues(&initial_values);
+  initial_import_done_ = true;
+}
+
+void DomStorageArea::ScheduleCommitChanges() {
+  DCHECK_EQ(kLocalStorageNamespaceId, namespace_id_);
+  DCHECK(backing_.get());
+  DCHECK(clear_all_next_commit_ || !changed_values_.empty());
+  DCHECK(task_runner_.get());
+
+  if (commit_in_flight_)
+    return;
+
+  commit_in_flight_ = task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&DomStorageArea::CommitChanges, this),
+      base::TimeDelta::FromSeconds(1));
+  DCHECK(commit_in_flight_);
+}
+
+void DomStorageArea::CommitChanges() {
+  DCHECK(backing_.get());
+  if (backing_->CommitChanges(clear_all_next_commit_, changed_values_)) {
+    clear_all_next_commit_ = false;
+    changed_values_.clear();
+  }
+  commit_in_flight_ = false;
+}
+
+// static
+FilePath DomStorageArea::DatabaseFileNameFromOrigin(const GURL& origin) {
+  std::string filename = fileapi::GetOriginIdentifierFromURL(origin)
+      + ".localstorage";
+  return FilePath().AppendASCII(filename);
+}
+
 }  // namespace dom_storage
diff --git a/webkit/dom_storage/dom_storage_area.h b/webkit/dom_storage/dom_storage_area.h
index 0c4a0d5..ba26d60 100644
--- a/webkit/dom_storage/dom_storage_area.h
+++ b/webkit/dom_storage/dom_storage_area.h
@@ -7,18 +7,18 @@
 #pragma once
 
 #include "base/file_path.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/nullable_string16.h"
 #include "base/string16.h"
 #include "googleurl/src/gurl.h"
-#include "webkit/dom_storage/dom_storage_task_runner.h"
-
-class FilePath;
-class GURL;
+#include "webkit/dom_storage/dom_storage_database.h"
+#include "webkit/dom_storage/dom_storage_types.h"
 
 namespace dom_storage {
 
 class DomStorageMap;
+class DomStorageTaskRunner;
 
 // Container for a per-origin Map of key/value pairs potentially
 // backed by storage on disk and lazily commits changes to disk.
@@ -47,8 +47,21 @@ class DomStorageArea
 
  private:
   FRIEND_TEST_ALL_PREFIXES(DomStorageAreaTest, DomStorageAreaBasics);
+  FRIEND_TEST_ALL_PREFIXES(DomStorageAreaTest, BackingDatabaseOpened);
+  FRIEND_TEST_ALL_PREFIXES(DomStorageAreaTest, TestDatabaseFilePath);
   friend class base::RefCountedThreadSafe<DomStorageArea>;
 
+  // If we haven't done so already and this is a local storage area,
+  // will attempt to read any values for this origin currently
+  // stored on disk.
+  void InitialImportIfNeeded();
+
+  // Posts a task to write the set of changed values to disk.
+  void ScheduleCommitChanges();
+  void CommitChanges();
+
+  static FilePath DatabaseFileNameFromOrigin(const GURL& origin);
+
   ~DomStorageArea();
 
   int64 namespace_id_;
@@ -56,8 +69,11 @@ class DomStorageArea
   FilePath directory_;
   scoped_refptr<DomStorageTaskRunner> task_runner_;
   scoped_refptr<DomStorageMap> map_;
-  // TODO(benm): integrate with DomStorageDatabase to read from
-  // and lazily write to disk.
+  scoped_ptr<DomStorageDatabase> backing_;
+  bool initial_import_done_;
+  ValuesMap changed_values_;
+  bool clear_all_next_commit_;
+  bool commit_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 1d51fe3..3b716f6 100644
--- a/webkit/dom_storage/dom_storage_area_unittest.cc
+++ b/webkit/dom_storage/dom_storage_area_unittest.cc
@@ -2,9 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
+#include "base/file_util.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
+#include "base/scoped_temp_dir.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/dom_storage/dom_storage_area.h"
+#include "webkit/dom_storage/dom_storage_task_runner.h"
+#include "webkit/dom_storage/dom_storage_types.h"
 
 namespace dom_storage {
 
@@ -53,4 +62,100 @@ TEST(DomStorageAreaTest, DomStorageAreaBasics) {
   EXPECT_NE(copy->map_.get(), area->map_.get());
 }
 
+TEST(DomStorageAreaTest, BackingDatabaseOpened) {
+  const int64 kSessionStorageNamespaceId = kLocalStorageNamespaceId + 1;
+  const GURL kOrigin("http://www.google.com");
+
+  const string16 kKey = ASCIIToUTF16("test");
+  const string16 kKey2 = ASCIIToUTF16("test2");
+  const string16 kValue = ASCIIToUTF16("value");
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  const FilePath kExpectedOriginFilePath = temp_dir.path().Append(
+      DomStorageArea::DatabaseFileNameFromOrigin(kOrigin));
+
+  // No directory, backing should be null.
+  {
+    scoped_refptr<DomStorageArea> area(
+        new DomStorageArea(kLocalStorageNamespaceId, kOrigin, FilePath(),
+                           NULL));
+    EXPECT_EQ(NULL, area->backing_.get());
+    EXPECT_TRUE(area->initial_import_done_);
+    EXPECT_FALSE(file_util::PathExists(kExpectedOriginFilePath));
+  }
+
+  // Valid directory and origin but non-local namespace id. Backing should
+  // be null.
+  {
+    scoped_refptr<DomStorageArea> area(
+        new DomStorageArea(kSessionStorageNamespaceId, kOrigin,
+                           temp_dir.path(), NULL));
+    EXPECT_EQ(NULL, area->backing_.get());
+    EXPECT_TRUE(area->initial_import_done_);
+
+    NullableString16 old_value;
+    EXPECT_TRUE(area->SetItem(kKey, kValue, &old_value));
+    ASSERT_TRUE(old_value.is_null());
+
+    // Check that saving a value has still left us without a backing database.
+    EXPECT_EQ(NULL, area->backing_.get());
+    EXPECT_FALSE(file_util::PathExists(kExpectedOriginFilePath));
+  }
+
+  // This should set up a DomStorageArea that is correctly backed to disk.
+  {
+    scoped_refptr<DomStorageArea> area(
+        new DomStorageArea(kLocalStorageNamespaceId, kOrigin,
+            temp_dir.path(),
+            new MockDomStorageTaskRunner(base::MessageLoopProxy::current())));
+
+    EXPECT_TRUE(area->backing_.get());
+    EXPECT_FALSE(area->backing_->IsOpen());
+    EXPECT_FALSE(area->initial_import_done_);
+
+    // Switch out the file-backed db with an in-memory db to speed up the test.
+    // We will verify that something is written into the database but not
+    // that a file is written to disk - DOMStorageDatabase unit tests cover
+    // that.
+    area->backing_.reset(new DomStorageDatabase());
+
+    // Need to write something to ensure that the database is created.
+    NullableString16 old_value;
+    EXPECT_TRUE(area->SetItem(kKey, kValue, &old_value));
+    ASSERT_TRUE(old_value.is_null());
+    EXPECT_TRUE(area->SetItem(kKey2, kValue, &old_value));
+    ASSERT_TRUE(old_value.is_null());
+    EXPECT_TRUE(area->initial_import_done_);
+    EXPECT_TRUE(area->commit_in_flight_);
+
+    MessageLoop::current()->RunAllPending();
+
+    EXPECT_FALSE(area->commit_in_flight_);
+    EXPECT_TRUE(area->backing_->IsOpen());
+    EXPECT_EQ(2u, area->Length());
+    EXPECT_EQ(kValue, area->GetItem(kKey).string());
+
+    // Verify the content made it to the in memory database.
+    ValuesMap values;
+    area->backing_->ReadAllValues(&values);
+    EXPECT_EQ(2u, values.size());
+    EXPECT_EQ(kValue, values[kKey].string());
+  }
+}
+
+TEST(DomStorageAreaTest, TestDatabaseFilePath) {
+  EXPECT_EQ(FilePath().AppendASCII("file_path_to_0.localstorage"),
+      DomStorageArea::DatabaseFileNameFromOrigin(
+          GURL("file://path_to/index.html")));
+
+  EXPECT_EQ(FilePath().AppendASCII("https_www.google.com_0.localstorage"),
+      DomStorageArea::DatabaseFileNameFromOrigin(
+          GURL("https://www.google.com/")));
+
+  EXPECT_EQ(FilePath().AppendASCII("https_www.google.com_8080.localstorage"),
+      DomStorageArea::DatabaseFileNameFromOrigin(
+          GURL("https://www.google.com:8080")));
+}
+
 }  // namespace dom_storage
diff --git a/webkit/dom_storage/dom_storage_database.h b/webkit/dom_storage/dom_storage_database.h
index e651a55..2a636c2 100644
--- a/webkit/dom_storage/dom_storage_database.h
+++ b/webkit/dom_storage/dom_storage_database.h
@@ -53,6 +53,7 @@ class DomStorageDatabase {
                            TestCanOpenAndReadWebCoreDatabase);
   FRIEND_TEST_ALL_PREFIXES(DomStorageDatabaseTest,
                            TestCanOpenFileThatIsNotADatabase);
+  FRIEND_TEST_ALL_PREFIXES(DomStorageAreaTest, BackingDatabaseOpened);
 
   enum SchemaVersion {
     INVALID,
diff --git a/webkit/dom_storage/dom_storage_namespace.cc b/webkit/dom_storage/dom_storage_namespace.cc
index e711673..343da59 100644
--- a/webkit/dom_storage/dom_storage_namespace.cc
+++ b/webkit/dom_storage/dom_storage_namespace.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "webkit/dom_storage/dom_storage_area.h"
+#include "webkit/dom_storage/dom_storage_task_runner.h"
 #include "webkit/dom_storage/dom_storage_types.h"
 
 namespace dom_storage {
diff --git a/webkit/dom_storage/dom_storage_namespace.h b/webkit/dom_storage/dom_storage_namespace.h
index 7184cac..93a3884 100644
--- a/webkit/dom_storage/dom_storage_namespace.h
+++ b/webkit/dom_storage/dom_storage_namespace.h
@@ -11,7 +11,6 @@
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
-#include "webkit/dom_storage/dom_storage_area.h"
 
 class GURL;
 
diff --git a/webkit/dom_storage/dom_storage_task_runner.cc b/webkit/dom_storage/dom_storage_task_runner.cc
index 6a1afb5..84e5c305 100644
--- a/webkit/dom_storage/dom_storage_task_runner.cc
+++ b/webkit/dom_storage/dom_storage_task_runner.cc
@@ -20,17 +20,18 @@ DomStorageTaskRunner::DomStorageTaskRunner(
 DomStorageTaskRunner::~DomStorageTaskRunner() {
 }
 
-void DomStorageTaskRunner::PostTask(
+bool DomStorageTaskRunner::PostTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task) {
-  message_loop_->PostTask(from_here, task);
+  return message_loop_->PostTask(from_here, task);
 }
 
-void DomStorageTaskRunner::PostDelayedTask(
+bool DomStorageTaskRunner::PostDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
-  message_loop_->PostDelayedTask(from_here, task, delay.InMilliseconds());
+  return message_loop_->PostDelayedTask(from_here, task,
+                                        delay.InMilliseconds());
 }
 
 // DomStorageWorkerPoolTaskRunner
@@ -47,25 +48,41 @@ DomStorageWorkerPoolTaskRunner::DomStorageWorkerPoolTaskRunner(
 DomStorageWorkerPoolTaskRunner::~DomStorageWorkerPoolTaskRunner() {
 }
 
-void DomStorageWorkerPoolTaskRunner::PostTask(
+bool DomStorageWorkerPoolTaskRunner::PostTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task) {
-  // TODO(michaeln): Do all tasks need to be run prior to shutdown?
-  // Maybe make better use of the SHUTDOWN_BEHAVIOR.
-  sequenced_worker_pool_->PostSequencedWorkerTask(
-      sequence_token_, from_here, task);
+  // We can skip on shutdown as the destructor of DomStorageArea will ensure
+  // that any remaining data is committed to disk.
+  return sequenced_worker_pool_->PostSequencedWorkerTaskWithShutdownBehavior(
+      sequence_token_, from_here, task,
+      base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
 }
 
-void DomStorageWorkerPoolTaskRunner::PostDelayedTask(
+bool DomStorageWorkerPoolTaskRunner::PostDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
   // Post a task to call this->PostTask() after the delay.
-  message_loop_->PostDelayedTask(
+  return message_loop_->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&DomStorageWorkerPoolTaskRunner::PostTask, this,
-                 from_here, task),
+      base::Bind(base::IgnoreResult(&DomStorageWorkerPoolTaskRunner::PostTask),
+                 this, from_here, task),
       delay.InMilliseconds());
 }
 
+// MockDomStorageTaskRunner
+
+MockDomStorageTaskRunner::MockDomStorageTaskRunner(
+    base::MessageLoopProxy* message_loop)
+    : DomStorageTaskRunner(message_loop) {
+}
+
+bool MockDomStorageTaskRunner::PostDelayedTask(
+    const tracked_objects::Location& from_here,
+    const base::Closure& task,
+    base::TimeDelta delay) {
+  // Don't wait in unit tests.
+  return PostTask(from_here, task);
+}
+
 }  // namespace dom_storage
diff --git a/webkit/dom_storage/dom_storage_task_runner.h b/webkit/dom_storage/dom_storage_task_runner.h
index a5c09b1..21e4ab7 100644
--- a/webkit/dom_storage/dom_storage_task_runner.h
+++ b/webkit/dom_storage/dom_storage_task_runner.h
@@ -25,12 +25,12 @@ class DomStorageTaskRunner
   virtual ~DomStorageTaskRunner();
 
   // Schedules a task to be run immediately.
-  virtual void PostTask(
+  virtual bool PostTask(
       const tracked_objects::Location& from_here,
       const base::Closure& task);
 
   // Schedules a task to be run after a delay.
-  virtual void PostDelayedTask(
+  virtual bool PostDelayedTask(
       const tracked_objects::Location& from_here,
       const base::Closure& task,
       base::TimeDelta delay);
@@ -50,12 +50,12 @@ class DomStorageWorkerPoolTaskRunner : public DomStorageTaskRunner {
   virtual ~DomStorageWorkerPoolTaskRunner();
 
   // Schedules a sequenced worker task to be run immediately.
-  virtual void PostTask(
+  virtual bool PostTask(
       const tracked_objects::Location& from_here,
       const base::Closure& task) OVERRIDE;
 
   // Schedules a sequenced worker task to be run after a delay.
-  virtual void PostDelayedTask(
+  virtual bool PostDelayedTask(
       const tracked_objects::Location& from_here,
       const base::Closure& task,
       base::TimeDelta delay) OVERRIDE;
@@ -65,6 +65,20 @@ class DomStorageWorkerPoolTaskRunner : public DomStorageTaskRunner {
   base::SequencedWorkerPool::SequenceToken sequence_token_;
 };
 
+// A derived class used in unit tests that causes us to ignore the
+// delay in PostDelayedTask so we don't need to block in unit tests
+// for the timeout to expire.
+class MockDomStorageTaskRunner : public DomStorageTaskRunner {
+ public:
+  explicit MockDomStorageTaskRunner(base::MessageLoopProxy* message_loop);
+  virtual ~MockDomStorageTaskRunner() { }
+
+  virtual bool PostDelayedTask(
+      const tracked_objects::Location& from_here,
+      const base::Closure& task,
+      base::TimeDelta delay) OVERRIDE;
+};
+
 }  // namespace dom_storage
 
 #endif  // WEBKIT_DOM_STORAGE_DOM_STORAGE_TASK_RUNNER_