indexeddb: convert mock idb class factory to IndexedDBControl

Bug: 1015214
Change-Id: I8836e79a52f35e166c5d5ec310ccc82dc52689a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090878
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748824}

components/services/storage/public/mojom/indexed_db_control_test.mojom

@@ -15,9 +15,32 @@ enum V2SchemaCorruptionStatus {
   CORRUPTION_YES,
 };
 
+enum FailClass {
+  NOTHING,
+  LEVELDB_ITERATOR,
+  LEVELDB_DIRECT_TRANSACTION,
+  LEVELDB_TRANSACTION,
+  LEVELDB_DATABASE,
+};
+
+enum FailMethod {
+  NOTHING,
+  COMMIT,
+  COMMIT_DISK_FULL,
+  GET,
+  SEEK,
+  WRITE,
+};
+
+// A test-only interface for creating fake failures in IndexedDB methods.
+interface MockFailureInjector {
+  // Causes indexeddb to fail in a given way on a given call.
+  FailOperation(FailClass failure_class, FailMethod failure_method,
+                int32 instance_num, int32 call_num) => ();
+};
+
 // A test-only interface extending storage.mojom.IndexedDBControl.
 interface IndexedDBControlTest {
-
   // Gets the root data path for storage.
   GetBaseDataPathForTesting() => (mojo_base.mojom.FilePath path);
 
@@ -53,4 +76,18 @@ interface IndexedDBControlTest {
 
   // Compacts the backing store for a given origin.
   CompactBackingStoreForTesting(url.mojom.Origin origin) => ();
+
+  // Overrides IndexedDB with a mock class factory that can inject failures
+  // at specific points.  For now, because this is static, there can only
+  // be one of these bound at a single time.
+  //
+  // Note: in order for this not to be flaky, this should be called before
+  // using the IndexedDB system in any way to ensure that that the default
+  // class factory isn't used before the testing one is set.
+  BindMockFailureSingletonForTesting(
+      pending_receiver<MockFailureInjector> receiver);
+
+  // Return keys used in WriteToIndexedDBForTesting.
+  GetDatabaseKeysForTesting() =>
+      (string schema_version_key, string data_version_key);
 };

content/browser/BUILD.gn

@@ -1059,6 +1059,8 @@ jumbo_source_set("browser") {
     "indexed_db/indexed_db_value.cc",
     "indexed_db/indexed_db_value.h",
     "indexed_db/list_set.h",
+    "indexed_db/mock_browsertest_indexed_db_class_factory.cc",
+    "indexed_db/mock_browsertest_indexed_db_class_factory.h",
     "indexed_db/transaction_impl.cc",
     "indexed_db/transaction_impl.h",
     "initiator_csp_context.cc",

content/browser/indexed_db/indexed_db_browsertest.cc

@@ -30,7 +30,6 @@
 #include "components/services/storage/public/mojom/indexed_db_control.mojom-test-utils.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
 #include "content/browser/browser_main_loop.h"
-#include "content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -60,8 +59,10 @@
 #include "url/origin.h"
 
 using base::ASCIIToUTF16;
-using storage::QuotaManager;
 using storage::DatabaseUtil;
+using storage::QuotaManager;
+using storage::mojom::FailClass;
+using storage::mojom::FailMethod;
 
 namespace content {
 
@@ -72,22 +73,17 @@ class IndexedDBBrowserTest : public ContentBrowserTest,
  public:
   IndexedDBBrowserTest() = default;
 
-  void SetUp() override {
-    GetTestClassFactory()->Reset();
-    IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(GetIDBClassFactory);
-
+  void SetUpOnMainThread() override {
     // Some tests need more space than the default used for browser tests.
     static storage::QuotaSettings quota_settings =
         storage::GetHardCodedSettings(100 * 1024 * 1024);
     StoragePartition::SetDefaultQuotaSettingsForTesting(&quota_settings);
 
-    ContentBrowserTest::SetUp();
+    GetControlTest()->BindMockFailureSingletonForTesting(
+        failure_injector_.BindNewPipeAndPassReceiver());
   }
 
-  void TearDown() override {
-    IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(nullptr);
-    ContentBrowserTest::TearDown();
-  }
+  void TearDownOnMainThread() override { failure_injector_.reset(); }
 
   bool UseProductionQuotaSettings() override {
     // So that the browser test harness doesn't call
@@ -99,8 +95,21 @@ class IndexedDBBrowserTest : public ContentBrowserTest,
                      FailMethod failure_method,
                      int fail_on_instance_num,
                      int fail_on_call_num) {
-    GetTestClassFactory()->FailOperation(
-        failure_class, failure_method, fail_on_instance_num, fail_on_call_num);
+    base::RunLoop loop;
+    FailOperationWithCallback(failure_class, failure_method,
+                              fail_on_instance_num, fail_on_call_num,
+                              loop.QuitClosure());
+    loop.Run();
+  }
+
+  void FailOperationWithCallback(FailClass failure_class,
+                                 FailMethod failure_method,
+                                 int fail_on_instance_num,
+                                 int fail_on_call_num,
+                                 base::OnceClosure callback) {
+    failure_injector_->FailOperation(failure_class, failure_method,
+                                     fail_on_instance_num, fail_on_call_num,
+                                     std::move(callback));
   }
 
   void SimpleTest(const GURL& test_url,
@@ -274,18 +283,9 @@ class IndexedDBBrowserTest : public ContentBrowserTest,
     loop.Run();
   }
 
- protected:
-  static MockBrowserTestIndexedDBClassFactory* GetTestClassFactory() {
-    static ::base::LazyInstance<MockBrowserTestIndexedDBClassFactory>::Leaky
-        s_factory = LAZY_INSTANCE_INITIALIZER;
-    return s_factory.Pointer();
-  }
-
-  static IndexedDBClassFactory* GetIDBClassFactory() {
-    return GetTestClassFactory();
-  }
-
  private:
+  mojo::Remote<storage::mojom::MockFailureInjector> failure_injector_;
+
   DISALLOW_COPY_AND_ASSIGN(IndexedDBBrowserTest);
 };
 
@@ -381,13 +381,26 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, Bug941965Test) {
 }
 
 IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, NegativeDBSchemaVersion) {
+  return;
   const GURL database_open_url = GetTestUrl("indexeddb", "database_test.html");
   const url::Origin origin = url::Origin::Create(database_open_url);
   // Create the database.
   SimpleTest(database_open_url);
   // -10, little endian.
   std::string value = "\xF6\xFF\xFF\xFF\xFF\xFF\xFF\xFF";
-  WriteToIndexedDB(origin, SchemaVersionKey::Encode(), value);
+
+  auto control_test = GetControlTest();
+  base::RunLoop loop;
+  std::string key;
+  control_test->GetDatabaseKeysForTesting(
+      base::BindLambdaForTesting([&](const std::string& schema_version_key,
+                                     const std::string& data_version_key) {
+        key = schema_version_key;
+        loop.Quit();
+      }));
+  loop.Run();
+
+  WriteToIndexedDB(origin, key, value);
   // Crash the tab to ensure no old navigations are picked up.
   CrashTab(shell()->web_contents());
   SimpleTest(GetTestUrl("indexeddb", "open_bad_db.html"));
@@ -400,7 +413,19 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, NegativeDBDataVersion) {
   SimpleTest(database_open_url);
   // -10, little endian.
   std::string value = "\xF6\xFF\xFF\xFF\xFF\xFF\xFF\xFF";
-  WriteToIndexedDB(origin, DataVersionKey::Encode(), value);
+
+  auto control_test = GetControlTest();
+  base::RunLoop loop;
+  std::string key;
+  control_test->GetDatabaseKeysForTesting(
+      base::BindLambdaForTesting([&](const std::string& schema_version_key,
+                                     const std::string& data_version_key) {
+        key = data_version_key;
+        loop.Quit();
+      }));
+  loop.Run();
+
+  WriteToIndexedDB(origin, key, value);
   // Crash the tab to ensure no old navigations are picked up.
   CrashTab(shell()->web_contents());
   SimpleTest(GetTestUrl("indexeddb", "open_bad_db.html"));
@@ -758,7 +783,7 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, DiskFullOnCommit) {
   //   #2: IndexedDBTransaction::Commit - the test's "readwrite" transaction)
   const int instance_num = 2;
   const int call_num = 1;
-  FailOperation(FAIL_CLASS_LEVELDB_TRANSACTION, FAIL_METHOD_COMMIT_DISK_FULL,
+  FailOperation(FailClass::LEVELDB_TRANSACTION, FailMethod::COMMIT_DISK_FULL,
                 instance_num, call_num);
   SimpleTest(GetTestUrl("indexeddb", "disk_full_on_commit.html"));
 }
@@ -870,8 +895,8 @@ std::unique_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler(
     http_response->set_code(net::HTTP_OK);
     return std::move(http_response);
   } else if (request_path == "fail" && !request_query.empty()) {
-    FailClass failure_class = FAIL_CLASS_NOTHING;
-    FailMethod failure_method = FAIL_METHOD_NOTHING;
+    FailClass failure_class = FailClass::NOTHING;
+    FailMethod failure_method = FailMethod::NOTHING;
     int instance_num = 1;
     int call_num = 1;
     std::string fail_class;
@@ -901,29 +926,29 @@ std::unique_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler(
     }
 
     if (fail_class == "LevelDBTransaction") {
-      failure_class = FAIL_CLASS_LEVELDB_TRANSACTION;
+      failure_class = FailClass::LEVELDB_TRANSACTION;
       if (fail_method == "Get")
-        failure_method = FAIL_METHOD_GET;
+        failure_method = FailMethod::GET;
       else if (fail_method == "Commit")
-        failure_method = FAIL_METHOD_COMMIT;
+        failure_method = FailMethod::COMMIT;
       else
         NOTREACHED() << "Unknown method: \"" << fail_method << "\"";
     } else if (fail_class == "LevelDBIterator") {
-      failure_class = FAIL_CLASS_LEVELDB_ITERATOR;
+      failure_class = FailClass::LEVELDB_ITERATOR;
       if (fail_method == "Seek")
-        failure_method = FAIL_METHOD_SEEK;
+        failure_method = FailMethod::SEEK;
       else
         NOTREACHED() << "Unknown method: \"" << fail_method << "\"";
     } else if (fail_class == "LevelDBDatabase") {
-      failure_class = FAIL_CLASS_LEVELDB_DATABASE;
+      failure_class = FailClass::LEVELDB_DATABASE;
       if (fail_method == "Write")
-        failure_method = FAIL_METHOD_WRITE;
+        failure_method = FailMethod::WRITE;
       else
         NOTREACHED() << "Unknown method: \"" << fail_method << "\"";
     } else if (fail_class == "LevelDBDirectTransaction") {
-      failure_class = FAIL_CLASS_LEVELDB_DIRECT_TRANSACTION;
+      failure_class = FailClass::LEVELDB_DIRECT_TRANSACTION;
       if (fail_method == "Get")
-        failure_method = FAIL_METHOD_GET;
+        failure_method = FailMethod::GET;
       else
         NOTREACHED() << "Unknown method: \"" << fail_method << "\"";
     } else {
@@ -933,7 +958,13 @@ std::unique_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler(
     DCHECK_GE(instance_num, 1);
     DCHECK_GE(call_num, 1);
 
-    test->FailOperation(failure_class, failure_method, instance_num, call_num);
+    base::RunLoop loop;
+    task_runner->PostTask(
+        FROM_HERE,
+        base::BindOnce(&IndexedDBBrowserTest::FailOperationWithCallback,
+                       base::Unretained(test), failure_class, failure_method,
+                       instance_num, call_num, loop.QuitClosure()));
+    loop.Run();
 
     std::unique_ptr<net::test_server::BasicHttpResponse> http_response(
         new net::test_server::BasicHttpResponse);
@@ -1109,14 +1140,7 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, ForceCloseEventTest) {
 
 // The V2 schema corruption test runs in a separate class to avoid corrupting
 // an IDB store that other tests use.
-class IndexedDBBrowserTestV2SchemaCorruption : public IndexedDBBrowserTest {
- public:
-  void SetUp() override {
-    GetTestClassFactory()->Reset();
-    IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(GetIDBClassFactory);
-    ContentBrowserTest::SetUp();
-  }
-};
+class IndexedDBBrowserTestV2SchemaCorruption : public IndexedDBBrowserTest {};
 
 // Verify the V2 schema corruption lifecycle:
 // - create a current version backing store (v3 or later)
@@ -1187,12 +1211,6 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTestSingleProcess,
 // This test is for https://crbug.com/1039446.
 class IndexedDBBrowserTestBlobKeyCorruption : public IndexedDBBrowserTest {
  public:
-  void SetUp() override {
-    GetTestClassFactory()->Reset();
-    IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(GetIDBClassFactory);
-    ContentBrowserTest::SetUp();
-  }
-
   int64_t GetNextBlobNumber(const url::Origin& origin, int64_t database_id) {
     int64_t number;
 

content/browser/indexed_db/indexed_db_context_impl.cc

@@ -39,6 +39,7 @@
 #include "content/browser/indexed_db/indexed_db_quota_client.h"
 #include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
+#include "content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h"
 #include "storage/browser/database/database_util.h"
 #include "storage/common/database/database_identifier.h"
 #include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom.h"
@@ -57,6 +58,16 @@ const base::FilePath::CharType IndexedDBContextImpl::kIndexedDBDirectory[] =
 
 namespace {
 
+static MockBrowserTestIndexedDBClassFactory* GetTestClassFactory() {
+  static ::base::LazyInstance<MockBrowserTestIndexedDBClassFactory>::Leaky
+      s_factory = LAZY_INSTANCE_INITIALIZER;
+  return s_factory.Pointer();
+}
+
+static IndexedDBClassFactory* GetTestIDBClassFactory() {
+  return GetTestClassFactory();
+}
+
 bool IsAllowedPath(const std::vector<base::FilePath>& allowed_paths,
                    const base::FilePath& candidate_path) {
   for (const base::FilePath& allowed_path : allowed_paths) {
@@ -579,6 +590,28 @@ void IndexedDBContextImpl::CompactBackingStoreForTesting(
   std::move(callback).Run();
 }
 
+void IndexedDBContextImpl::BindMockFailureSingletonForTesting(
+    mojo::PendingReceiver<storage::mojom::MockFailureInjector> receiver) {
+  // Lazily instantiate the GetTestClassFactory.
+  if (!mock_failure_injector_.has_value())
+    mock_failure_injector_.emplace(GetTestClassFactory());
+
+  // TODO(enne): this should really not be a static setter.
+  CHECK(!mock_failure_injector_->is_bound());
+  GetTestClassFactory()->Reset();
+  IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(GetTestIDBClassFactory);
+
+  mock_failure_injector_->Bind(std::move(receiver));
+  mock_failure_injector_->set_disconnect_handler(base::BindOnce([]() {
+    IndexedDBClassFactory::SetIndexedDBClassFactoryGetter(nullptr);
+  }));
+}
+
+void IndexedDBContextImpl::GetDatabaseKeysForTesting(
+    GetDatabaseKeysForTestingCallback callback) {
+  std::move(callback).Run(SchemaVersionKey::Encode(), DataVersionKey::Encode());
+}
+
 void IndexedDBContextImpl::ForceCloseSync(
     const Origin& origin,
     storage::mojom::ForceCloseReason reason) {

content/browser/indexed_db/indexed_db_context_impl.h

@@ -120,6 +120,11 @@ class CONTENT_EXPORT IndexedDBContextImpl
       GetPathForBlobForTestingCallback callback) override;
   void CompactBackingStoreForTesting(const url::Origin& origin,
                                      base::OnceClosure callback) override;
+  void BindMockFailureSingletonForTesting(
+      mojo::PendingReceiver<storage::mojom::MockFailureInjector> receiver)
+      override;
+  void GetDatabaseKeysForTesting(
+      GetDatabaseKeysForTestingCallback callback) override;
 
   // TODO(enne): fix internal indexeddb callers to use ForceClose async instead.
   void ForceCloseSync(const url::Origin& origin,
@@ -241,6 +246,8 @@ class CONTENT_EXPORT IndexedDBContextImpl
 
   mojo::ReceiverSet<storage::mojom::IndexedDBControl> receivers_;
   mojo::ReceiverSet<storage::mojom::IndexedDBControlTest> test_receivers_;
+  base::Optional<mojo::Receiver<storage::mojom::MockFailureInjector>>
+      mock_failure_injector_;
   mojo::RemoteSet<storage::mojom::IndexedDBObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBContextImpl);

content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h

@@ -13,10 +13,12 @@
 
 #include "components/services/storage/indexed_db/scopes/scopes_lock_manager.h"
 #include "components/services/storage/indexed_db/transactional_leveldb/transactional_leveldb_factory.h"
+#include "components/services/storage/public/mojom/indexed_db_control_test.mojom.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 #include "content/browser/indexed_db/indexed_db_class_factory.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_task_helper.h"
+#include "content/common/content_export.h"
 #include "third_party/blink/public/common/indexeddb/web_idb_types.h"
 
 namespace content {
@@ -30,26 +32,10 @@ class LevelDBSnapshot;
 class TransactionalLevelDBTransaction;
 class TransactionalLevelDBDatabase;
 
-enum FailClass {
-  FAIL_CLASS_NOTHING,
-  FAIL_CLASS_LEVELDB_ITERATOR,
-  FAIL_CLASS_LEVELDB_DIRECT_TRANSACTION,
-  FAIL_CLASS_LEVELDB_TRANSACTION,
-  FAIL_CLASS_LEVELDB_DATABASE,
-};
-
-enum FailMethod {
-  FAIL_METHOD_NOTHING,
-  FAIL_METHOD_COMMIT,
-  FAIL_METHOD_COMMIT_DISK_FULL,
-  FAIL_METHOD_GET,
-  FAIL_METHOD_SEEK,
-  FAIL_METHOD_WRITE,
-};
-
-class MockBrowserTestIndexedDBClassFactory
+class CONTENT_EXPORT MockBrowserTestIndexedDBClassFactory
     : public IndexedDBClassFactory,
-      public DefaultTransactionalLevelDBFactory {
+      public DefaultTransactionalLevelDBFactory,
+      public storage::mojom::MockFailureInjector {
  public:
   MockBrowserTestIndexedDBClassFactory();
   ~MockBrowserTestIndexedDBClassFactory() override;
@@ -90,18 +76,19 @@ class MockBrowserTestIndexedDBClassFactory
       base::WeakPtr<TransactionalLevelDBTransaction> txn,
       std::unique_ptr<LevelDBSnapshot> snapshot) override;
 
-  void FailOperation(FailClass failure_class,
-                     FailMethod failure_method,
+  void FailOperation(storage::mojom::FailClass failure_class,
+                     storage::mojom::FailMethod failure_method,
                      int fail_on_instance_num,
-                     int fail_on_call_num);
+                     int fail_on_call_num,
+                     base::OnceClosure callback) override;
   void Reset();
 
  private:
-  FailClass failure_class_;
-  FailMethod failure_method_;
-  std::map<FailClass, int> instance_count_;
-  std::map<FailClass, int> fail_on_instance_num_;
-  std::map<FailClass, int> fail_on_call_num_;
+  storage::mojom::FailClass failure_class_;
+  storage::mojom::FailMethod failure_method_;
+  std::map<storage::mojom::FailClass, int> instance_count_;
+  std::map<storage::mojom::FailClass, int> fail_on_instance_num_;
+  std::map<storage::mojom::FailClass, int> fail_on_call_num_;
   bool only_trace_calls_;
 };
 

content/test/BUILD.gn

@@ -915,8 +915,6 @@ test("content_browsertests") {
     "../browser/idle/idle_browsertest.cc",
     "../browser/indexed_db/indexed_db_browsertest.cc",
     "../browser/indexed_db/indexed_db_feature_observer_browsertest.cc",
-    "../browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc",
-    "../browser/indexed_db/mock_browsertest_indexed_db_class_factory.h",
     "../browser/isolated_origin_browsertest.cc",
     "../browser/keyboard_lock_browsertest.cc",
     "../browser/keyboard_lock_browsertest.h",