SessionStorageDatabase fix: Delete namespace keys if there are no areas.

webkit/dom_storage/session_storage_database.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/dom_storage/session_storage_database.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
@@ skipping 222 matching lines @@
     // "<namespaceid>" parts from the keys.
     if (current_namespace_start_key.empty() ||
         key.substr(0, current_namespace_start_key.length()) !=
         current_namespace_start_key) {
       // The key is of the form "namespace-<namespaceid>-" for a new
       // <namespaceid>.
       current_namespace_start_key = key;
       current_namespace_id =
           key.substr(namespace_prefix.length(),
                      key.length() - namespace_prefix.length() - 1);
+      // Ensure that we keep track of the namespace even if it doesn't contain
+      // any origins.
+      (*namespaces_and_origins)[current_namespace_id];

[michaeln, 2013/02/12 01:42:46]

maybe map->insert(id, std::vector<GURL>()) would be more clear despite being a
few more chars of code?

[marja, 2013/02/12 09:10:41]

Done.

     } else {
       // The key is of the form "namespace-<namespaceid>-<origin>".
       std::string origin = key.substr(current_namespace_start_key.length());
       (*namespaces_and_origins)[current_namespace_id].push_back(GURL(origin));
     }
   }
   db_->ReleaseSnapshot(options.snapshot);
   return true;
 }
 
@@ skipping 151 matching lines @@
   bool exists;
   if (!GetMapForArea(namespace_id, origin, leveldb::ReadOptions(), &exists,
                      &map_id))
     return false;
   if (!exists)
     return true;  // Nothing to delete.
   if (!DecreaseMapRefCount(map_id, 1, batch))
     return false;
   std::string namespace_key = NamespaceKey(namespace_id, origin);
   batch->Delete(namespace_key);
+
+  // If this was the only area in the namespace, delete the namespace start key,
+  // too.

[michaeln, 2013/02/12 01:42:46]

I doubt this explains the massive size of the file in the bug report. Is there a
compaction step that we have to do on leveldb instances that we aren't doing?

Let's ask the idb folks?

[marja, 2013/02/12 09:10:41]

Yeah, this is not the reason behind the bug report.

+  std::string namespace_start_key = NamespaceStartKey(namespace_id);
+  scoped_ptr<leveldb::Iterator> it(db_->NewIterator(leveldb::ReadOptions()));
+  it->Seek(namespace_start_key);
+  if (!ConsistencyCheck(it->Valid()))
+    return false;
+  // Advance the iterator 2 times (we still haven't really deleted
+  // namespace_key).
+  it->Next();
+  if (!ConsistencyCheck(it->Valid()))
+    return false;
+  it->Next();
+  if (!it->Valid())
+    return true;
+  std::string key = it->key().ToString();
+  if (key.find(namespace_start_key) != 0) {

[michaeln, 2013/02/12 01:42:46]

nit: {} not needed on the one-liner

[marja, 2013/02/12 09:10:41]

Done.

+    batch->Delete(namespace_start_key);
+  }
   return true;
 }
 
 bool SessionStorageDatabase::GetMapForArea(const std::string& namespace_id,
                                            const std::string& origin,
                                            const leveldb::ReadOptions& options,
                                            bool* exists, std::string* map_id) {
   std::string namespace_key = NamespaceKey(namespace_id, origin);
   leveldb::Status s = db_->Get(options, namespace_key, map_id);
   if (s.IsNotFound()) {
@@ skipping 194 matching lines @@
 std::string SessionStorageDatabase::MapKey(const std::string& map_id,
                                            const std::string& key) {
   return base::StringPrintf("map-%s-%s", map_id.c_str(), key.c_str());
 }
 
 const char* SessionStorageDatabase::NextMapIdKey() {
   return "next-map-id";
 }
 
 }  // namespace dom_storage