From fb7282a3c0353cb2bb880e8f48c1655f129cdb97 Mon Sep 17 00:00:00 2001
From: "jsbell@chromium.org"
 <jsbell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 11 Aug 2014 23:30:55 +0000
Subject: [PATCH] IndexedDB: Database enumeration should not include failed
 opens

When a database is first opened, metadata is written to the backing
store. This metadata is retained even if the open is eventually
aborted. This would show up when the (nonstandard) getDatabaseNames
method was used... such as by the inspector. The inspector would
then attempt to open the database and implicitly create it!

As a quick fix, ignore databases with the default version (this case)
when enumerating. A longer term fix would be to not write the metadata
separately from the upgrade transaction, but we must still cope with
stale metadata in existing backing stores.

BUG=395472
R=cmumford@chromium.org,dgrogan@chromium.org

Review URL: https://codereview.chromium.org/429713002

Cr-Commit-Position: refs/heads/master@{#288826}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288826 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../browser/indexed_db/indexed_db_backing_store.cc | 28 ++++++++++++++++++++-
 .../indexed_db_backing_store_unittest.cc           | 29 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc
index 95639f5af7d0..93db0512ed2e 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -1167,6 +1167,7 @@ std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames(
   for (*s = it->Seek(start_key);
        s->ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0;
        *s = it->Next()) {
+    // Decode database name (in iterator key).
     StringPiece slice(it->Key());
     DatabaseNameKey database_name_key;
     if (!DatabaseNameKey::Decode(&slice, &database_name_key) ||
@@ -1174,7 +1175,31 @@ std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames(
       INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_DATABASE_NAMES);
       continue;
     }
-    found_names.push_back(database_name_key.database_name());
+
+    // Decode database id (in iterator value).
+    int64 database_id = 0;
+    StringPiece valueSlice(it->Value());
+    if (!DecodeVarInt(&valueSlice, &database_id) || !valueSlice.empty()) {
+      INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_DATABASE_NAMES);
+      continue;
+    }
+
+    // Look up version by id.
+    bool found = false;
+    int64 database_version = IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION;
+    *s = GetVarInt(db_.get(),
+                   DatabaseMetaDataKey::Encode(
+                       database_id, DatabaseMetaDataKey::USER_INT_VERSION),
+                   &database_version,
+                   &found);
+    if (!s->ok() || !found) {
+      INTERNAL_READ_ERROR_UNTESTED(GET_DATABASE_NAMES);
+      continue;
+    }
+
+    // Ignore stale metadata from failed initial opens.
+    if (database_version != IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION)
+      found_names.push_back(database_name_key.database_name());
   }
 
   if (!s->ok())
@@ -1289,6 +1314,7 @@ leveldb::Status IndexedDBBackingStore::CreateIDBDatabaseMetaData(
     const base::string16& version,
     int64 int_version,
     int64* row_id) {
+  // TODO(jsbell): Don't persist metadata if open fails. http://crbug.com/395472
   scoped_refptr<LevelDBTransaction> transaction =
       IndexedDBClassFactory::Get()->CreateLevelDBTransaction(db_.get());
 
diff --git a/content/browser/indexed_db/indexed_db_backing_store_unittest.cc b/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
index 1fea813e6591..c6628ff6f1f3 100644
--- a/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
@@ -970,6 +970,35 @@ TEST_F(IndexedDBBackingStoreTest, CreateDatabase) {
   }
 }
 
+TEST_F(IndexedDBBackingStoreTest, GetDatabaseNames) {
+  const base::string16 string_version(ASCIIToUTF16("string_version"));
+
+  const base::string16 db1_name(ASCIIToUTF16("db1"));
+  const int64 db1_version = 1LL;
+  int64 db1_id;
+
+  // Database records with DEFAULT_INT_VERSION represent stale data,
+  // and should not be enumerated.
+  const base::string16 db2_name(ASCIIToUTF16("db2"));
+  const int64 db2_version = IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION;
+  int64 db2_id;
+
+  leveldb::Status s = backing_store_->CreateIDBDatabaseMetaData(
+      db1_name, string_version, db1_version, &db1_id);
+  EXPECT_TRUE(s.ok());
+  EXPECT_GT(db1_id, 0LL);
+
+  s = backing_store_->CreateIDBDatabaseMetaData(
+      db2_name, string_version, db2_version, &db2_id);
+  EXPECT_TRUE(s.ok());
+  EXPECT_GT(db2_id, db1_id);
+
+  std::vector<base::string16> names = backing_store_->GetDatabaseNames(&s);
+  EXPECT_TRUE(s.ok());
+  EXPECT_EQ(names.size(), 1ULL);
+  EXPECT_EQ(names[0], db1_name);
+}
+
 }  // namespace
 
 }  // namespace content
-- 
2.11.4.GIT