Let IndexedDBContextImpl create its own task runner

content/browser/indexed_db/indexed_db_unittest.cc

Index: content/browser/indexed_db/indexed_db_unittest.cc
diff --git a/content/browser/indexed_db/indexed_db_unittest.cc b/content/browser/indexed_db/indexed_db_unittest.cc
index 25258618a33c3c0c54486d8713f6f703d003d713..6183c71d2c7f2383f0a0fbb52d041d5988b06c68 100644
--- a/content/browser/indexed_db/indexed_db_unittest.cc
+++ b/content/browser/indexed_db/indexed_db_unittest.cc
@@ -10,9 +10,8 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
-#include "base/test/test_simple_task_runner.h"
-#include "base/threading/thread.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/test/scoped_task_environment.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"
 #include "content/browser/indexed_db/indexed_db_factory_impl.h"
@@ -22,6 +21,7 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
 #include "storage/browser/quota/quota_manager.h"
 #include "storage/browser/quota/special_storage_policy.h"
 #include "storage/browser/test/mock_quota_manager_proxy.h"
@@ -41,7 +41,8 @@ class IndexedDBTest : public testing::Test {
   IndexedDBTest()
       : kNormalOrigin(GURL("http://normal/")),
         kSessionOnlyOrigin(GURL("http://session-only/")),
-        task_runner_(new base::TestSimpleTaskRunner),
+        scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::UI),
         special_storage_policy_(new MockSpecialStoragePolicy),
         quota_manager_proxy_(new MockQuotaManagerProxy(nullptr, nullptr)) {
     special_storage_policy_->AddSessionOnly(kSessionOnlyOrigin.GetURL());
@@ -51,9 +52,7 @@ class IndexedDBTest : public testing::Test {
   }
 
  protected:
-  void FlushIndexedDBTaskRunner() { task_runner_->RunUntilIdle(); }
-
-  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
   scoped_refptr<MockSpecialStoragePolicy> special_storage_policy_;
   scoped_refptr<MockQuotaManagerProxy> quota_manager_proxy_;
 
@@ -75,19 +74,17 @@ TEST_F(IndexedDBTest, ClearSessionOnlyDatabases) {
   {
     scoped_refptr<IndexedDBContextImpl> idb_context = new IndexedDBContextImpl(
         temp_dir.GetPath(), special_storage_policy_.get(),
-        quota_manager_proxy_.get(), task_runner_.get());
+        quota_manager_proxy_.get());
 
     normal_path = idb_context->GetFilePathForTesting(kNormalOrigin);
     session_only_path = idb_context->GetFilePathForTesting(kSessionOnlyOrigin);
     ASSERT_TRUE(base::CreateDirectory(normal_path));
     ASSERT_TRUE(base::CreateDirectory(session_only_path));
-    FlushIndexedDBTaskRunner();
-    base::RunLoop().RunUntilIdle();
+    RunAllBlockingPoolTasksUntilIdle();
     quota_manager_proxy_->SimulateQuotaManagerDestroyed();
   }
 
-  FlushIndexedDBTaskRunner();
-  base::RunLoop().RunUntilIdle();
+  RunAllBlockingPoolTasksUntilIdle();
 
   EXPECT_TRUE(base::DirectoryExists(normal_path));
   EXPECT_FALSE(base::DirectoryExists(session_only_path));
@@ -106,7 +103,7 @@ TEST_F(IndexedDBTest, SetForceKeepSessionState) {
     // With the levelDB backend, these are directories.
     scoped_refptr<IndexedDBContextImpl> idb_context = new IndexedDBContextImpl(
         temp_dir.GetPath(), special_storage_policy_.get(),
-        quota_manager_proxy_.get(), task_runner_.get());
+        quota_manager_proxy_.get());
 
     // Save session state. This should bypass the destruction-time deletion.
     idb_context->SetForceKeepSessionState();
@@ -130,10 +127,7 @@ class ForceCloseDBCallbacks : public IndexedDBCallbacks {
  public:
   ForceCloseDBCallbacks(scoped_refptr<IndexedDBContextImpl> idb_context,
                         const Origin& origin)
-      : IndexedDBCallbacks(nullptr,
-                           origin,
-                           nullptr,
-                           base::ThreadTaskRunnerHandle::Get()),
+      : IndexedDBCallbacks(nullptr, origin, nullptr, idb_context->TaskRunner()),
         idb_context_(idb_context),
         origin_(origin) {}
 
@@ -176,7 +170,12 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnDelete) {
 
     scoped_refptr<IndexedDBContextImpl> idb_context = new IndexedDBContextImpl(
         temp_dir.GetPath(), special_storage_policy_.get(),
-        quota_manager_proxy_.get(), task_runner_.get());
+        quota_manager_proxy_.get());
+
+    // Assign current task runner so that methods on context can be invoked
+    // directly from test body.
+    idb_context->SetTaskRunnerForTesting(
+        base::SequencedTaskRunnerHandle::Get());

[gab, 2017/06/13 15:31:23]

A cleaner alternative to doing this would be to post your entire test body which
needs to run on |task_runner| as a lambda and then run until idle.

That way you properly create a multi-threaded test where things run as expected
(and highlight races for TSAN if there are any in the product).

i.e.:

instead of 

TEST(...) {
  idb_context->Foo();
  idb_context->Bar();
}

you could:

TEST(...) {
  idb_context->TaskRunner()->PostTask(
      base::Bind([](IndexedDBContextImpl* idb_context) {
          idb_context->Foo();
          idb_context->Bar();
      }, base::Unretained(idb_context)));

  run until idle...
}

 
     scoped_refptr<ForceCloseDBCallbacks> open_callbacks =
         new ForceCloseDBCallbacks(idb_context, kTestOrigin);
@@ -211,12 +210,12 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnDelete) {
         base::Bind(static_cast<void (IndexedDBContextImpl::*)(const Origin&)>(
                        &IndexedDBContextImpl::DeleteForOrigin),
                    idb_context, kTestOrigin));
-    FlushIndexedDBTaskRunner();
-    base::RunLoop().RunUntilIdle();
+
+    scoped_task_environment_.RunUntilIdle();
   }
 
   // Make sure we wait until the destructor has run.
-  base::RunLoop().RunUntilIdle();
+  scoped_task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(open_db_callbacks->forced_close_called());
   EXPECT_FALSE(closed_db_callbacks->forced_close_called());
@@ -230,7 +229,7 @@ TEST_F(IndexedDBTest, DeleteFailsIfDirectoryLocked) {
 
   scoped_refptr<IndexedDBContextImpl> idb_context = new IndexedDBContextImpl(
       temp_dir.GetPath(), special_storage_policy_.get(),
-      quota_manager_proxy_.get(), task_runner_.get());
+      quota_manager_proxy_.get());
 
   base::FilePath test_path = idb_context->GetFilePathForTesting(kTestOrigin);
   ASSERT_TRUE(base::CreateDirectory(test_path));
@@ -245,7 +244,7 @@ TEST_F(IndexedDBTest, DeleteFailsIfDirectoryLocked) {
   idb_context->TaskRunner()->PostTask(
       FROM_HERE,
       base::Bind(delete_for_origin, idb_context, kTestOrigin));
-  FlushIndexedDBTaskRunner();
+  RunAllBlockingPoolTasksUntilIdle();
 
   EXPECT_TRUE(base::DirectoryExists(test_path));
 }
@@ -258,7 +257,11 @@ TEST_F(IndexedDBTest, ForceCloseOpenDatabasesOnCommitFailure) {
 
   scoped_refptr<IndexedDBContextImpl> context = new IndexedDBContextImpl(
       temp_dir.GetPath(), special_storage_policy_.get(),
-      quota_manager_proxy_.get(), task_runner_.get());
+      quota_manager_proxy_.get());
+
+  // Assign current task runner so that methods on context can be invoked
+  // directly from test body.
+  context->SetTaskRunnerForTesting(base::SequencedTaskRunnerHandle::Get());
 
   scoped_refptr<IndexedDBFactoryImpl> factory =
       static_cast<IndexedDBFactoryImpl*>(context->GetIDBFactory());