[IndexedDB] Treat v2 schemas w/ blobs as corrupt, behind flag.

The flag is present so we can finch this feature on stable to mitigate
risk for first-party customers like Docs Offline.

Bug: 829141
Change-Id: Ibfed20dfe45c4a96445001f721a499e08e2bcb7e
Reviewed-on: https://chromium-review.googlesource.com/1069601Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560846}

content/browser/indexed_db/indexed_db_backing_store.cc

@@ -42,6 +42,7 @@
 #include "content/common/indexed_db/indexed_db_key_path.h"
 #include "content/common/indexed_db/indexed_db_key_range.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/content_features.h"
 #include "net/base/load_flags.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_request_context.h"
@@ -773,9 +774,6 @@ WARN_UNUSED_RESULT Status IndexedDBBackingStore::SetUpMetadata() {
       //
       // In order to prevent corrupt databases, when upgrading from 2 to 3 this
       // will consider any v2 databases with BlobEntryKey entries as corrupt.
-      // Unfortunately this will blow away a lot of data for third party
-      // customers, so first we will only upgrade the non-corrupt v2 databases
-      // (and leave the corrupt v2 ones for one release).
       // https://crbug.com/756447, https://crbug.com/829125,
       // https://crbug.com/829141
       db_schema_version = 3;
@@ -787,10 +785,13 @@ WARN_UNUSED_RESULT Status IndexedDBBackingStore::SetUpMetadata() {
       }
       indexed_db::ReportV2Schema(has_blobs, origin_);
       if (has_blobs) {
-        // TODO(dmurph): Treat this as corruption.
-        // https://crbug.com/829141
         INTERNAL_CONSISTENCY_ERROR(UPGRADING_SCHEMA_CORRUPTED_BLOBS);
-        // return InternalInconsistencyStatus();
+        // Put database wiping behind a flag so we can use finch to stop this
+        // behavior if first-party customers have problems.
+        if (base::FeatureList::IsEnabled(
+                features::kWipeCorruptV2IDBDatabases)) {
+          return InternalInconsistencyStatus();
+        }
       } else {
         PutInt(transaction.get(), schema_version_key, db_schema_version);
       }

content/browser/indexed_db/indexed_db_backing_store_unittest.cc

@@ -1574,5 +1574,120 @@ TEST_F(IndexedDBBackingStoreTest, SchemaUpgradeWithoutBlobsSurvives) {
   RunAllTasksUntilIdle();
 }
 
+// Our v2->v3 schema migration code forgot to bump the on-disk version number.
+// This test covers migrating a v3 database mislabeled as v2 to a properly
+// labeled v3 database. When the mislabeled database has blob entries, we must
+// treat it as corrupt and delete it.
+// https://crbug.com/756447, https://crbug.com/829125, https://crbug.com/829141
+TEST_F(IndexedDBBackingStoreTestWithBlobs, SchemaUpgradeWithBlobsCorrupt) {
+  struct TestState {
+    int64_t database_id;
+    const int64_t object_store_id = 99;
+    std::unique_ptr<IndexedDBBackingStore::Transaction> transaction1;
+    scoped_refptr<TestCallback> callback1;
+    std::unique_ptr<IndexedDBBackingStore::Transaction> transaction3;
+    scoped_refptr<TestCallback> callback3;
+  } state;
+
+  // The database metadata needs to be written so the blob entry keys can
+  // be detected.
+  idb_context_->TaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](IndexedDBBackingStore* backing_store, TestState* state) {
+            const base::string16 database_name(ASCIIToUTF16("db1"));
+            const int64_t version = 9;
+
+            const base::string16 object_store_name(
+                ASCIIToUTF16("object_store1"));
+            const bool auto_increment = true;
+            const IndexedDBKeyPath object_store_key_path(
+                ASCIIToUTF16("object_store_key"));
+
+            IndexedDBMetadataCoding metadata_coding;
+
+            {
+              IndexedDBDatabaseMetadata database;
+              leveldb::Status s = metadata_coding.CreateDatabase(
+                  backing_store->db(), backing_store->origin_identifier(),
+                  database_name, version, &database);
+              EXPECT_TRUE(s.ok());
+              EXPECT_GT(database.id, 0);
+              state->database_id = database.id;
+
+              IndexedDBBackingStore::Transaction transaction(backing_store);
+              transaction.Begin();
+
+              IndexedDBObjectStoreMetadata object_store;
+              s = metadata_coding.CreateObjectStore(
+                  transaction.transaction(), database.id,
+                  state->object_store_id, object_store_name,
+                  object_store_key_path, auto_increment, &object_store);
+              EXPECT_TRUE(s.ok());
+
+              scoped_refptr<TestCallback> callback(
+                  base::MakeRefCounted<TestCallback>());
+              EXPECT_TRUE(transaction.CommitPhaseOne(callback).ok());
+              EXPECT_TRUE(callback->called);
+              EXPECT_TRUE(callback->succeeded);
+              EXPECT_TRUE(transaction.CommitPhaseTwo().ok());
+            }
+          },
+          base::Unretained(backing_store()), base::Unretained(&state)));
+  RunAllTasksUntilIdle();
+
+  idb_context_->TaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](IndexedDBBackingStoreTestWithBlobs* test, TestState* state) {
+            // Initiate transaction1 - writing blobs.
+            state->transaction1 =
+                std::make_unique<IndexedDBBackingStore::Transaction>(
+                    test->backing_store());
+            state->transaction1->Begin();
+            std::vector<std::unique_ptr<storage::BlobDataHandle>> handles;
+            IndexedDBBackingStore::RecordIdentifier record;
+            EXPECT_TRUE(test->backing_store()
+                            ->PutRecord(state->transaction1.get(),
+                                        state->database_id,
+                                        state->object_store_id, test->key3_,
+                                        &test->value3_, &handles, &record)
+                            .ok());
+            state->callback1 = base::MakeRefCounted<TestCallback>();
+            EXPECT_TRUE(
+                state->transaction1->CommitPhaseOne(state->callback1).ok());
+          },
+          base::Unretained(this), base::Unretained(&state)));
+  RunAllTasksUntilIdle();
+
+  idb_context_->TaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](IndexedDBBackingStoreTestWithBlobs* test, TestState* state) {
+            // Finish up transaction1, verifying blob writes.
+            EXPECT_TRUE(state->callback1->called);
+            EXPECT_TRUE(state->callback1->succeeded);
+            EXPECT_TRUE(test->CheckBlobWrites());
+            EXPECT_TRUE(state->transaction1->CommitPhaseTwo().ok());
+
+            // Set the schema to 2, which was before blob support.
+            scoped_refptr<LevelDBTransaction> transaction =
+                IndexedDBClassFactory::Get()->CreateLevelDBTransaction(
+                    test->backing_store()->db());
+            const std::string schema_version_key = SchemaVersionKey::Encode();
+            indexed_db::PutInt(transaction.get(), schema_version_key, 2);
+            ASSERT_TRUE(transaction->Commit().ok());
+          },
+          base::Unretained(this), base::Unretained(&state)));
+  RunAllTasksUntilIdle();
+
+  DestroyFactoryAndBackingStore();
+  CreateFactoryAndBackingStore();
+
+  // The factory returns a null backing store pointer when there is a corrupt
+  // database.
+  EXPECT_EQ(nullptr, backing_store());
+}
+
 }  // namespace indexed_db_backing_store_unittest
 }  // namespace content

content/public/common/content_features.cc

@@ -574,6 +574,10 @@ const base::Feature kWebXrHitTest{"WebXRHitTest",
 const base::Feature kWebXrOrientationSensorDevice{
     "WebXROrientationSensorDevice", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Wipe corrupt v2 IndexedDB databases.
+const base::Feature kWipeCorruptV2IDBDatabases{
+    "WipeCorruptV2IDBDatabases", base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Enabled "work stealing" in the script runner.
 const base::Feature kWorkStealingInScriptRunner{
     "WorkStealingInScriptRunner", base::FEATURE_DISABLED_BY_DEFAULT};

content/public/common/content_features.h

@@ -131,6 +131,7 @@ CONTENT_EXPORT extern const base::Feature kWebXr;
 CONTENT_EXPORT extern const base::Feature kWebXrGamepadSupport;
 CONTENT_EXPORT extern const base::Feature kWebXrHitTest;
 CONTENT_EXPORT extern const base::Feature kWebXrOrientationSensorDevice;
+CONTENT_EXPORT extern const base::Feature kWipeCorruptV2IDBDatabases;
 CONTENT_EXPORT extern const base::Feature kWorkStealingInScriptRunner;
 
 #if defined(OS_ANDROID)