Indexed DB: Simplify cross-sequence test code

Now that base::BindLambdaForTesting() can be used, tasks posted to the
Indexed DB task runner in tests don't need to explicitly bring along
ambient state. Also, use an explicit RunLoop's Run()/Quit() instead of
RunAllTasksUntilIdle() where possible, and 'auto' for readability.

Change-Id: Iede419cda01c166e8fe2047c901f528f687f940d
Reviewed-on: https://chromium-review.googlesource.com/c/1298416
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603052}

content/browser/indexed_db/indexed_db_quota_client_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -79,11 +80,13 @@ class IndexedDBQuotaClientTest : public testing::Test {
                          const url::Origin& origin,
                          StorageType type) {
     usage_ = -1;
-    client->GetOriginUsage(
-        origin, type,
-        base::BindOnce(&IndexedDBQuotaClientTest::OnGetOriginUsageComplete,
-                       weak_factory_.GetWeakPtr()));
-    RunAllTasksUntilIdle();
+    base::RunLoop loop;
+    client->GetOriginUsage(origin, type,
+                           base::BindLambdaForTesting([&](int64_t usage) {
+                             usage_ = usage;
+                             loop.Quit();
+                           }));
+    loop.Run();
     EXPECT_GT(usage_, -1);
     return usage_;
   }
@@ -91,10 +94,14 @@ class IndexedDBQuotaClientTest : public testing::Test {
   const std::set<url::Origin>& GetOriginsForType(storage::QuotaClient* client,
                                                  StorageType type) {
     origins_.clear();
+    base::RunLoop loop;
     client->GetOriginsForType(
-        type, base::BindOnce(&IndexedDBQuotaClientTest::OnGetOriginsComplete,
-                             weak_factory_.GetWeakPtr()));
-    RunAllTasksUntilIdle();
+        type,
+        base::BindLambdaForTesting([&](const std::set<url::Origin>& origins) {
+          origins_ = origins;
+          loop.Quit();
+        }));
+    loop.Run();
     return origins_;
   }
 
@@ -102,11 +109,14 @@ class IndexedDBQuotaClientTest : public testing::Test {
                                                  StorageType type,
                                                  const std::string& host) {
     origins_.clear();
+    base::RunLoop loop;
     client->GetOriginsForHost(
         type, host,
-        base::BindOnce(&IndexedDBQuotaClientTest::OnGetOriginsComplete,
-                       weak_factory_.GetWeakPtr()));
-    RunAllTasksUntilIdle();
+        base::BindLambdaForTesting([&](const std::set<url::Origin>& origins) {
+          origins_ = origins;
+          loop.Quit();
+        }));
+    loop.Run();
     return origins_;
   }
 
@@ -114,11 +124,14 @@ class IndexedDBQuotaClientTest : public testing::Test {
                                              const url::Origin& origin,
                                              StorageType type) {
     delete_status_ = blink::mojom::QuotaStatusCode::kUnknown;
+    base::RunLoop loop;
     client->DeleteOriginData(
         origin, type,
-        base::BindOnce(&IndexedDBQuotaClientTest::OnDeleteOriginComplete,
-                       weak_factory_.GetWeakPtr()));
-    RunAllTasksUntilIdle();
+        base::BindLambdaForTesting([&](blink::mojom::QuotaStatusCode code) {
+          delete_status_ = code;
+          loop.Quit();
+        }));
+    loop.Run();
     return delete_status_;
   }
 
@@ -142,16 +155,6 @@ class IndexedDBQuotaClientTest : public testing::Test {
   }
 
  private:
-  void OnGetOriginUsageComplete(int64_t usage) { usage_ = usage; }
-
-  void OnGetOriginsComplete(const std::set<url::Origin>& origins) {
-    origins_ = origins;
-  }
-
-  void OnDeleteOriginComplete(blink::mojom::QuotaStatusCode code) {
-    delete_status_ = code;
-  }
-
   content::TestBrowserThreadBundle thread_bundle_;
   base::ScopedTempDir temp_dir_;
   int64_t usage_;

content/browser/indexed_db/indexed_db_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_context_impl.h"
@@ -69,8 +70,7 @@ TEST_F(IndexedDBTest, ClearSessionOnlyDatabases) {
   // Create the scope which will ensure we run the destructor of the context
   // which should trigger the clean up.
   {
-    scoped_refptr<IndexedDBContextImpl> idb_context =
-        base::MakeRefCounted<IndexedDBContextImpl>(
+    auto idb_context = base::MakeRefCounted<IndexedDBContextImpl>(
         temp_dir.GetPath(), special_storage_policy_.get(),
         quota_manager_proxy_.get());
 
@@ -99,8 +99,7 @@ TEST_F(IndexedDBTest, SetForceKeepSessionState) {
   {
     // Create some indexedDB paths.
     // With the levelDB backend, these are directories.
-    scoped_refptr<IndexedDBContextImpl> idb_context =
-        base::MakeRefCounted<IndexedDBContextImpl>(
+    auto idb_context = base::MakeRefCounted<IndexedDBContextImpl>(
         temp_dir.GetPath(), special_storage_policy_.get(),
         quota_manager_proxy_.get());
 
@@ -154,22 +153,24 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnDelete) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  scoped_refptr<IndexedDBContextImpl> idb_context =
-      base::MakeRefCounted<IndexedDBContextImpl>(temp_dir.GetPath(),
-                                                 special_storage_policy_.get(),
+  auto idb_context = base::MakeRefCounted<IndexedDBContextImpl>(
+      temp_dir.GetPath(), special_storage_policy_.get(),
       quota_manager_proxy_.get());
 
   const Origin kTestOrigin = Origin::Create(GURL("http://test/"));
-  idb_context->TaskRunner()->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          [](IndexedDBContextImpl* idb_context,
-             scoped_refptr<MockIndexedDBDatabaseCallbacks> open_db_callbacks,
-             scoped_refptr<MockIndexedDBDatabaseCallbacks> closed_db_callbacks,
-             scoped_refptr<ForceCloseDBCallbacks> open_callbacks,
-             scoped_refptr<ForceCloseDBCallbacks> closed_callbacks,
-             const Origin& origin) {
 
+  auto open_db_callbacks =
+      base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>();
+  auto closed_db_callbacks =
+      base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>();
+  auto open_callbacks =
+      base::MakeRefCounted<ForceCloseDBCallbacks>(idb_context, kTestOrigin);
+  auto closed_callbacks =
+      base::MakeRefCounted<ForceCloseDBCallbacks>(idb_context, kTestOrigin);
+
+  base::RunLoop loop;
+  idb_context->TaskRunner()->PostTask(
+      FROM_HERE, base::BindLambdaForTesting([&]() {
         const int child_process_id = 0;
         const int64_t host_transaction_id = 0;
         const int64_t version = 0;
@@ -177,36 +178,31 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnDelete) {
         IndexedDBFactory* factory = idb_context->GetIDBFactory();
 
         base::FilePath test_path =
-                idb_context->GetFilePathForTesting(origin);
+            idb_context->GetFilePathForTesting(kTestOrigin);
 
         factory->Open(base::ASCIIToUTF16("opendb"),
                       std::make_unique<IndexedDBPendingConnection>(
-                              open_callbacks, open_db_callbacks,
-                              child_process_id, host_transaction_id, version),
-                          origin, idb_context->data_path());
+                          open_callbacks, open_db_callbacks, child_process_id,
+                          host_transaction_id, version),
+                      kTestOrigin, idb_context->data_path());
         EXPECT_TRUE(base::DirectoryExists(test_path));
 
         factory->Open(base::ASCIIToUTF16("closeddb"),
                       std::make_unique<IndexedDBPendingConnection>(
                           closed_callbacks, closed_db_callbacks,
                           child_process_id, host_transaction_id, version),
-                          origin, idb_context->data_path());
+                      kTestOrigin, idb_context->data_path());
 
         closed_callbacks->connection()->Close();
 
-            idb_context->DeleteForOrigin(origin);
+        idb_context->DeleteForOrigin(kTestOrigin);
 
         EXPECT_TRUE(open_db_callbacks->forced_close_called());
         EXPECT_FALSE(closed_db_callbacks->forced_close_called());
         EXPECT_FALSE(base::DirectoryExists(test_path));
-          },
-          base::Unretained(idb_context.get()),
-          base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>(),
-          base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>(),
-          base::MakeRefCounted<ForceCloseDBCallbacks>(idb_context, kTestOrigin),
-          base::MakeRefCounted<ForceCloseDBCallbacks>(idb_context, kTestOrigin),
-          kTestOrigin));
-  RunAllTasksUntilIdle();
+        loop.Quit();
+      }));
+  loop.Run();
 }
 
 TEST_F(IndexedDBTest, DeleteFailsIfDirectoryLocked) {
@@ -214,24 +210,23 @@ TEST_F(IndexedDBTest, DeleteFailsIfDirectoryLocked) {
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   const Origin kTestOrigin = Origin::Create(GURL("http://test/"));
 
-  scoped_refptr<IndexedDBContextImpl> idb_context =
-      base::MakeRefCounted<IndexedDBContextImpl>(temp_dir.GetPath(),
-                                                 special_storage_policy_.get(),
+  auto idb_context = base::MakeRefCounted<IndexedDBContextImpl>(
+      temp_dir.GetPath(), special_storage_policy_.get(),
       quota_manager_proxy_.get());
 
   base::FilePath test_path = idb_context->GetFilePathForTesting(kTestOrigin);
   ASSERT_TRUE(base::CreateDirectory(test_path));
 
-  std::unique_ptr<LevelDBLock> lock =
-      LevelDBDatabase::LockForTesting(test_path);
+  auto lock = LevelDBDatabase::LockForTesting(test_path);
   ASSERT_TRUE(lock);
 
-  // TODO(jsbell): Remove static_cast<> when overloads are eliminated.
-  void (IndexedDBContextImpl::* delete_for_origin)(const Origin&) =
-      &IndexedDBContextImpl::DeleteForOrigin;
+  base::RunLoop loop;
   idb_context->TaskRunner()->PostTask(
-      FROM_HERE, base::BindOnce(delete_for_origin, idb_context, kTestOrigin));
-  RunAllTasksUntilIdle();
+      FROM_HERE, base::BindLambdaForTesting([&]() {
+        idb_context->DeleteForOrigin(kTestOrigin);
+        loop.Quit();
+      }));
+  loop.Run();
 
   EXPECT_TRUE(base::DirectoryExists(test_path));
 }
@@ -240,30 +235,28 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnCommitFailure) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  scoped_refptr<IndexedDBContextImpl> idb_context =
-      base::MakeRefCounted<IndexedDBContextImpl>(temp_dir.GetPath(),
-                                                 special_storage_policy_.get(),
+  auto idb_context = base::MakeRefCounted<IndexedDBContextImpl>(
+      temp_dir.GetPath(), special_storage_policy_.get(),
       quota_manager_proxy_.get());
 
+  auto temp_path = temp_dir.GetPath();
+  auto callbacks = base::MakeRefCounted<MockIndexedDBCallbacks>();
+  auto db_callbacks = base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>();
+
+  base::RunLoop loop;
   idb_context->TaskRunner()->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          [](IndexedDBContextImpl* idb_context, const base::FilePath temp_path,
-             scoped_refptr<MockIndexedDBCallbacks> callbacks,
-             scoped_refptr<MockIndexedDBDatabaseCallbacks> db_callbacks) {
+      FROM_HERE, base::BindLambdaForTesting([&]() {
         const Origin kTestOrigin = Origin::Create(GURL("http://test/"));
 
-            scoped_refptr<IndexedDBFactoryImpl> factory =
-                static_cast<IndexedDBFactoryImpl*>(
-                    idb_context->GetIDBFactory());
+        auto* factory =
+            static_cast<IndexedDBFactoryImpl*>(idb_context->GetIDBFactory());
 
         const int child_process_id = 0;
         const int64_t transaction_id = 1;
 
-            std::unique_ptr<IndexedDBPendingConnection> connection(
-                std::make_unique<IndexedDBPendingConnection>(
+        auto connection = std::make_unique<IndexedDBPendingConnection>(
             callbacks, db_callbacks, child_process_id, transaction_id,
-                    IndexedDBDatabaseMetadata::DEFAULT_VERSION));
+            IndexedDBDatabaseMetadata::DEFAULT_VERSION);
         factory->Open(base::ASCIIToUTF16("db"), std::move(connection),
                       Origin(kTestOrigin), temp_path);
 
@@ -275,19 +268,15 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnCommitFailure) {
         EXPECT_TRUE(factory->IsBackingStoreOpen(kTestOrigin));
 
         // Simulate the write failure.
-            leveldb::Status status =
-                leveldb::Status::IOError("Simulated failure");
-            idb_context->GetIDBFactory()->HandleBackingStoreFailure(
-                kTestOrigin);
+        leveldb::Status status = leveldb::Status::IOError("Simulated failure");
+        factory->HandleBackingStoreFailure(kTestOrigin);
 
         EXPECT_TRUE(db_callbacks->forced_close_called());
         EXPECT_FALSE(factory->IsBackingStoreOpen(kTestOrigin));
-          },
-          base::Unretained(idb_context.get()), temp_dir.GetPath(),
 
-          base::MakeRefCounted<MockIndexedDBCallbacks>(),
-          base::MakeRefCounted<MockIndexedDBDatabaseCallbacks>()));
-  RunAllTasksUntilIdle();
+        loop.Quit();
+      }));
+  loop.Run();
 }
 
 }  // namespace content