SessionStorageDatabase: Allow writing data into a shallow copy.

BUG=104292
TEST=SessionStorageDatabaseTest.{WriteIntoShallowCopy,NamespaceOffsetWriteIntoShallowCopy}

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

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

3 files changed, 60 insertions, 76 deletions

diff --git a/webkit/dom_storage/session_storage_database.cc b/webkit/dom_storage/session_storage_database.cc
index e66d477..11db6e5 100644
--- a/webkit/dom_storage/session_storage_database.cc
+++ b/webkit/dom_storage/session_storage_database.cc
@@ -82,15 +82,15 @@ bool SessionStorageDatabase::CommitAreaChanges(int64 namespace_id,
   if (!GetMapForArea(namespace_id, origin, &exists, &map_id))
     return false;
   if (exists) {
-    // We shouldn't write data into a shallow copy. If this is a shallow copy,
-    // it's a caller error (not an inconsistency in the database).
     int64 ref_count;
     if (!GetMapRefCount(map_id, &ref_count))
       return false;
-    if (!CallerErrorCheck(ref_count == 1))
-      return false;
-
-    if (clear_all_first) {
+    if (ref_count > 1) {
+      if (!DeepCopyArea(namespace_id, origin, !clear_all_first,
+                        &map_id, &batch))
+        return false;
+    }
+    else if (clear_all_first) {
       if (!ClearMap(map_id, &batch))
         return false;
     }
@@ -151,62 +151,6 @@ bool SessionStorageDatabase::CloneNamespace(int64 namespace_id,
   return DatabaseErrorCheck(s.ok());
 }
 
-bool SessionStorageDatabase::DeepCopyArea(int64 namespace_id,
-                                          const GURL& origin) {
-  // Example, data before deep copy:
-  // | namespace-1 (1 = namespace id) | dummy               |
-  // | namespace-1-origin1            | 1 (mapid)           |
-  // | namespace-2                    | dummy               |
-  // | namespace-2-origin1            | 1 (mapid) << references the same map
-  // | map-1                          | 2 (refcount)        |
-  // | map-1-a                        | b                   |
-
-  // Example, data after deep copy copy:
-  // | namespace-1 (1 = namespace id) | dummy               |
-  // | namespace-1-origin1            | 1 (mapid)           |
-  // | namespace-2                    | dummy               |
-  // | namespace-2-origin1            | 2 (mapid) << references the new map
-  // | map-1                          | 1 (dec. refcount)   |
-  // | map-1-a                        | b                   |
-  // | map-2                          | 1 (refcount)        |
-  // | map-2-a                        | b                   |
-
-  if (!LazyOpen(true))
-    return false;
-
-  std::string old_map_id;
-  bool exists;
-  if (!GetMapForArea(namespace_id, origin, &exists, &old_map_id))
-    return false;
-
-  // If the area doesn't exist, or if it's not a shallow copy, it's a caller
-  // error.
-  if (!CallerErrorCheck(exists))
-    return false;
-  int64 ref_count;
-  if (!GetMapRefCount(old_map_id, &ref_count))
-    return false;
-  if (!CallerErrorCheck(ref_count > 1))
-    return false;
-
-  leveldb::WriteBatch batch;
-  std::string new_map_id;
-  if (!CreateMapForArea(namespace_id, origin, &new_map_id, &batch))
-    return false;
-
-  // Copy the values in the map.
-  ValuesMap values;
-  if (!ReadMap(old_map_id, &values, false))
-    return false;
-  WriteValuesToMap(new_map_id, values, &batch);
-
-  if (!DecreaseMapRefCount(old_map_id, 1, &batch))
-    return false;
-
-  leveldb::Status s = db_->Write(leveldb::WriteOptions(), &batch);
-  return DatabaseErrorCheck(s.ok());
-}
-
 bool SessionStorageDatabase::DeleteArea(int64 namespace_id,
                                         const GURL& origin) {
   if (!LazyOpen(false)) {
@@ -603,6 +547,43 @@ bool SessionStorageDatabase::ClearMap(const std::string& map_id,
   return true;
 }
 
+bool SessionStorageDatabase::DeepCopyArea(
+    int64 namespace_id, const GURL& origin, bool copy_data,
+    std::string* map_id, leveldb::WriteBatch* batch) {
+  // Example, data before deep copy:
+  // | namespace-1 (1 = namespace id) | dummy               |
+  // | namespace-1-origin1            | 1 (mapid)           |
+  // | namespace-2                    | dummy               |
+  // | namespace-2-origin1            | 1 (mapid) << references the same map
+  // | map-1                          | 2 (refcount)        |
+  // | map-1-a                        | b                   |
+
+  // Example, data after deep copy copy:
+  // | namespace-1 (1 = namespace id) | dummy               |
+  // | namespace-1-origin1            | 1 (mapid)           |
+  // | namespace-2                    | dummy               |
+  // | namespace-2-origin1            | 2 (mapid) << references the new map
+  // | map-1                          | 1 (dec. refcount)   |
+  // | map-1-a                        | b                   |
+  // | map-2                          | 1 (refcount)        |
+  // | map-2-a                        | b                   |
+
+  // Read the values from the old map here. If we don't need to copy the data,
+  // this can stay empty.
+  ValuesMap values;
+  if (copy_data && !ReadMap(*map_id, &values, false))
+    return false;
+  if (!DecreaseMapRefCount(*map_id, 1, batch))
+    return false;
+  // Create a new map (this will also break the association to the old map) and
+  // write the old data into it. This will write the id of the created map into
+  // |map_id|.
+  if (!CreateMapForArea(namespace_id, origin, map_id, batch))
+    return false;
+  WriteValuesToMap(*map_id, values, batch);
+  return true;
+}
+
 std::string SessionStorageDatabase::NamespaceStartKey(
     const std::string& namespace_id_str) {
   return base::StringPrintf("namespace-%s", namespace_id_str.c_str());
diff --git a/webkit/dom_storage/session_storage_database.h b/webkit/dom_storage/session_storage_database.h
index 5230f37..5b427fd 100644
--- a/webkit/dom_storage/session_storage_database.h
+++ b/webkit/dom_storage/session_storage_database.h
@@ -44,7 +44,9 @@ class SessionStorageDatabase :
   // Updates the data for |namespace_id| and |origin|. Will remove all keys
   // before updating the database if |clear_all_first| is set. Then all entries
   // in |changes| will be examined - keys mapped to a null NullableString16 will
-  // be removed and all others will be inserted/updated as appropriate.
+  // be removed and all others will be inserted/updated as appropriate. It is
+  // allowed to write data into a shallow copy created by CloneNamespace, and in
+  // that case the copy will be made deep before writing the values.
   bool CommitAreaChanges(int64 namespace_id,
                          const GURL& origin,
                          bool clear_all_first,
@@ -54,9 +56,6 @@ class SessionStorageDatabase :
   // with |new_namespace_id|.
   bool CloneNamespace(int64 namespace_id, int64 new_namespace_id);
 
-  // Creates a deep copy of the area for |namespace_id| and |origin|.
-  bool DeepCopyArea(int64 namespace_id, const GURL& origin);
-
   // Deletes the data for |namespace_id| and |origin|.
   bool DeleteArea(int64 namespace_id, const GURL& origin);
 
@@ -158,6 +157,15 @@ class SessionStorageDatabase :
   // Deletes all values in |map_id|.
   bool ClearMap(const std::string& map_id, leveldb::WriteBatch* batch);
 
+  // Breaks the association between (|namespace_id|, |origin|) and |map_id| and
+  // creates a new map for (|namespace_id|, |origin|). Copies the data from the
+  // old map if |copy_data| is true.
+  bool DeepCopyArea(int64 namespace_id,
+                    const GURL& origin,
+                    bool copy_data,
+                    std::string* map_id,
+                    leveldb::WriteBatch* batch);
+
   // Helper functions for creating the keys needed for the schema.
   static std::string NamespaceStartKey(const std::string& namespace_id_str);
   static std::string NamespaceStartKey(int64 namespace_id,
diff --git a/webkit/dom_storage/session_storage_database_unittest.cc b/webkit/dom_storage/session_storage_database_unittest.cc
index c047009..150b9a4 100644
--- a/webkit/dom_storage/session_storage_database_unittest.cc
+++ b/webkit/dom_storage/session_storage_database_unittest.cc
@@ -479,17 +479,14 @@ TEST_F(SessionStorageDatabaseTest, ShallowCopy) {
   EXPECT_EQ(2, GetMapRefCount(GetMapForArea(1, kOrigin2)));
 }
 
-TEST_F(SessionStorageDatabaseTest, DeepCopy) {
+TEST_F(SessionStorageDatabaseTest, WriteIntoShallowCopy) {
   ValuesMap data1;
   data1[kKey1] = kValue1;
   data1[kKey2] = kValue2;
   ASSERT_TRUE(db_->CommitAreaChanges(1, kOrigin1, false, data1));
   EXPECT_TRUE(db_->CloneNamespace(1, 5));
 
-  // Make the shallow copy deep.
-  EXPECT_TRUE(db_->DeepCopyArea(5, kOrigin1));
-
-  // Write data into the deep copy.
+  // Write data into a shallow copy.
   ValuesMap changes;
   ValuesMap reference;
   changes[kKey1] = kValueNull;
@@ -501,7 +498,7 @@ TEST_F(SessionStorageDatabaseTest, DeepCopy) {
 
   // Values in the original namespace were not changed.
   CheckAreaData(1, kOrigin1, data1);
-  // But values in the deep copy were.
+  // But values in the copy were.
   CheckAreaData(5, kOrigin1, reference);
 
   // The namespaces no longer refer to the same map.
@@ -765,7 +762,7 @@ TEST_F(SessionStorageDatabaseTest, NamespaceOffsetCloneNamespace) {
   CheckAreaData(-22, kOrigin1, data1);
 }
 
-TEST_F(SessionStorageDatabaseTest, NamespaceOffsetDeepCopyArea) {
+TEST_F(SessionStorageDatabaseTest, NamespaceOffsetWriteIntoShallowCopy) {
   // Create a namespace with id 1.
   ValuesMap data1;
   data1[kKey1] = kValue1;
@@ -782,10 +779,8 @@ TEST_F(SessionStorageDatabaseTest, NamespaceOffsetDeepCopyArea) {
 
   // Make a shallow copy of the newly created namespace.
   EXPECT_TRUE(db_->CloneNamespace(1, 20));
-  // And make it deep.
-  EXPECT_TRUE(db_->DeepCopyArea(20, kOrigin1));
 
-  // Now the values can be altered in the deep copy.
+  // Now the values can be altered and a deep copy will be made.
   ValuesMap data3;
   data3[kKey1] = kValue2;
   EXPECT_TRUE(db_->CommitAreaChanges(20, kOrigin1, false, data3));