Revert of Use TaskScheduler instead of WorkerPool in v8_platform.cc. …

…(patchset chromium#18 id:340001 of https://codereview.chromium.org/2610473002/ )

Reason for revert:
Causing extensions_unittests to hang and take 15 minutes.

BUG=694828

Original issue's description:
> Use TaskScheduler instead of WorkerPool in v8_platform.cc.
>
> The following traits are used:
>
> Priority: Inherited (default)
>   The priority is inherited from the calling context (i.e. TaskTraits
>   are initialized with the priority of the current task).
>
> Shutdown behavior: CONTINUE_ON_SHUTDOWN
>   Tasks posted with this mode which have not started executing before
>   shutdown is initiated will never run. Tasks with this mode running at
>   shutdown will be ignored (the worker will not be joined).
>
>   Note: Tasks that were previously posted to base::WorkerPool should
>   use this shutdown behavior because this is how base::WorkerPool
>   handles all its tasks.
>
> MayBlock():
>   The task may block.
>
> BUG=659191
>
> Review-Url: https://codereview.chromium.org/2610473002
> Cr-Commit-Position: refs/heads/master@{#449976}
> Committed: https://chromium.googlesource.com/chromium/src/+/b83be4ca0e00b50b17619adf7f7de275455e9852

TBR=jochen@chromium.org,fdoray@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=659191

Review-Url: https://codereview.chromium.org/2711703002
Cr-Commit-Position: refs/heads/master@{#452058}

chrome/test/base/v8_unit_test.h

@@ -10,7 +10,6 @@
 
 #include "base/files/file_path.h"
 #include "base/strings/string_piece.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "v8/include/v8.h"
 
@@ -75,10 +74,6 @@ class V8UnitTest : public testing::Test {
   // Initializes paths and libraries.
   void InitPathsAndLibraries();
 
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
   // Handle scope that is used throughout the life of this class.
   v8::HandleScope handle_scope_;
 

content/public/test/render_view_test.h

@@ -13,7 +13,6 @@
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string16.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "base/test/test_io_thread.h"
 #include "build/build_config.h"
 #include "content/public/browser/native_web_keyboard_event.h"
@@ -191,14 +190,7 @@ class RenderViewTest : public testing::Test, blink::WebLeakDetectorClient {
   // blink::WebLeakDetectorClient implementation.
   void onLeakDetectionComplete(const Result& result) override;
 
- private:
   base::MessageLoop msg_loop_;
-
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
- protected:
   std::unique_ptr<FakeCompositorDependencies> compositor_deps_;
   std::unique_ptr<MockRenderProcess> mock_process_;
   // We use a naked pointer because we don't want to expose RenderViewImpl in

content/renderer/pepper/host_var_tracker_unittest.cc

@@ -7,7 +7,6 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "content/renderer/pepper/host_globals.h"
 #include "content/renderer/pepper/mock_resource.h"
 #include "content/renderer/pepper/pepper_plugin_instance_impl.h"
@@ -80,11 +79,6 @@ class HostVarTrackerTest : public PpapiUnittest {
   }
 
   HostVarTracker& tracker() { return *HostGlobals::Get()->host_var_tracker(); }
-
- private:
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
 };
 
 TEST_F(HostVarTrackerTest, DeleteObjectVarWithInstance) {

content/test/blink_test_environment.cc

@@ -10,7 +10,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_tokenizer.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "base/test/test_discardable_memory_allocator.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "build/build_config.h"
@@ -37,9 +36,17 @@ namespace {
 
 class TestEnvironment {
  public:
+#if defined(OS_ANDROID)
+  // Android UI message loop goes through Java, so don't use it in tests.
+  typedef base::MessageLoop MessageLoopType;
+#else
+  typedef base::MessageLoopForUI MessageLoopType;
+#endif
+
   TestEnvironment() {
-    // TestBlinkWebUnitTestSupport must be instantiated after the main
-    // MessageLoop.
+    main_message_loop_.reset(new MessageLoopType);
+
+    // TestBlinkWebUnitTestSupport must be instantiated after MessageLoopType.
     blink_test_support_.reset(new TestBlinkWebUnitTestSupport);
     content_initializer_.reset(new content::TestContentClientInitializer());
 
@@ -55,17 +62,7 @@ class TestEnvironment {
   }
 
  private:
-#if defined(OS_ANDROID)
-  // Android UI message loop goes through Java, so don't use it in tests.
-  base::MessageLoop main_message_loop_;
-#else
-  base::MessageLoopForUI main_message_loop_;
-#endif
-
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler;
-
+  std::unique_ptr<MessageLoopType> main_message_loop_;
   std::unique_ptr<TestBlinkWebUnitTestSupport> blink_test_support_;
   std::unique_ptr<TestContentClientInitializer> content_initializer_;
   base::TestDiscardableMemoryAllocator discardable_memory_allocator_;

extensions/renderer/api_binding_test.h

@@ -9,7 +9,6 @@
 
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "v8/include/v8.h"
 
@@ -44,11 +43,6 @@ class APIBindingTest : public testing::Test {
 
  private:
   base::MessageLoop message_loop_;
-
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
   std::unique_ptr<gin::IsolateHolder> isolate_holder_;
   std::unique_ptr<gin::ContextHolder> context_holder_;
 

extensions/renderer/gc_callback_unittest.cc

@@ -2,16 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "extensions/renderer/gc_callback.h"
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/features/feature.h"
+#include "extensions/renderer/gc_callback.h"
 #include "extensions/renderer/scoped_web_frame.h"
 #include "extensions/renderer/script_context.h"
 #include "extensions/renderer/script_context_set.h"
@@ -35,6 +34,8 @@ class GCCallbackTest : public testing::Test {
   GCCallbackTest() : script_context_set_(&active_extensions_) {}
 
  protected:
+  base::MessageLoop& message_loop() { return message_loop_; }
+
   ScriptContextSet& script_context_set() { return script_context_set_; }
 
   v8::Local<v8::Context> v8_context() {
@@ -71,11 +72,6 @@ class GCCallbackTest : public testing::Test {
   }
 
   base::MessageLoop message_loop_;
-
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
   ScopedWebFrame web_frame_;  // (this will construct the v8::Isolate)
   // ExtensionsRendererClient is a dependency of ScriptContextSet.
   TestExtensionsRendererClient extensions_renderer_client_;

extensions/renderer/module_system_test.h

@@ -6,7 +6,6 @@
 #define EXTENSIONS_RENDERER_MODULE_SYSTEM_TEST_H_
 
 #include "base/macros.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "extensions/renderer/module_system.h"
 #include "extensions/renderer/script_context.h"
 #include "gin/public/context_holder.h"
@@ -99,10 +98,6 @@ class ModuleSystemTest : public testing::Test {
   void RunResolvedPromises();
 
  private:
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
   v8::Isolate* isolate_;
   std::unique_ptr<ModuleSystemTestEnvironment> env_;
   bool should_assertions_be_made_;

gin/BUILD.gn

@@ -151,7 +151,6 @@ source_set("gin_test") {
     "//testing/gtest",
   ]
   deps = [
-    "//base/test:test_support",
     "//v8",
   ]
 

gin/shell/gin_main.cc

@@ -14,8 +14,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
-#include "base/sys_info.h"
-#include "base/task_scheduler/task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "gin/array_buffer.h"
 #include "gin/modules/console.h"
@@ -75,44 +73,34 @@ int main(int argc, char** argv) {
 #endif
 
   base::MessageLoop message_loop;
-  base::TaskScheduler::CreateAndSetSimpleTaskScheduler(
-      base::SysInfo::NumberOfProcessors());
 
   // Initialize the base::FeatureList since IsolateHolder can depend on it.
   base::FeatureList::SetInstance(base::WrapUnique(new base::FeatureList));
 
+  gin::IsolateHolder::Initialize(gin::IsolateHolder::kStrictMode,
+                                 gin::IsolateHolder::kStableV8Extras,
+                                 gin::ArrayBufferAllocator::SharedInstance());
+  gin::IsolateHolder instance(base::ThreadTaskRunnerHandle::Get());
+
+  gin::GinShellRunnerDelegate delegate;
+  gin::ShellRunner runner(&delegate, instance.isolate());
+
   {
-    gin::IsolateHolder::Initialize(gin::IsolateHolder::kStrictMode,
-                                   gin::IsolateHolder::kStableV8Extras,
-                                   gin::ArrayBufferAllocator::SharedInstance());
-    gin::IsolateHolder instance(base::ThreadTaskRunnerHandle::Get());
-
-    gin::GinShellRunnerDelegate delegate;
-    gin::ShellRunner runner(&delegate, instance.isolate());
-
-    {
-      gin::Runner::Scope scope(&runner);
-      runner.GetContextHolder()
-          ->isolate()
-          ->SetCaptureStackTraceForUncaughtExceptions(true);
-    }
-
-    base::CommandLine::StringVector args =
-        base::CommandLine::ForCurrentProcess()->GetArgs();
-    for (base::CommandLine::StringVector::const_iterator it = args.begin();
-         it != args.end(); ++it) {
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE,
-          base::Bind(gin::Run, runner.GetWeakPtr(), base::FilePath(*it)));
-    }
-
-    base::RunLoop().RunUntilIdle();
+    gin::Runner::Scope scope(&runner);
+    runner.GetContextHolder()
+        ->isolate()
+        ->SetCaptureStackTraceForUncaughtExceptions(true);
   }
 
-  // gin::IsolateHolder waits for tasks running in TaskScheduler in its
-  // destructor and thus must be destroyed before TaskScheduler starts skipping
-  // CONTINUE_ON_SHUTDOWN tasks.
-  base::TaskScheduler::GetInstance()->Shutdown();
+  base::CommandLine::StringVector args =
+      base::CommandLine::ForCurrentProcess()->GetArgs();
+  for (base::CommandLine::StringVector::const_iterator it = args.begin();
+       it != args.end(); ++it) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(gin::Run, runner.GetWeakPtr(), base::FilePath(*it)));
+  }
 
+  base::RunLoop().RunUntilIdle();
   return 0;
 }

gin/shell_runner_unittest.cc

@@ -6,7 +6,6 @@
 
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "gin/array_buffer.h"
 #include "gin/converter.h"
@@ -27,7 +26,6 @@ namespace gin {
 
 TEST(RunnerTest, Run) {
   base::MessageLoop message_loop;
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler;
   std::string source = "this.result = 'PASS';\n";
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA

gin/test/file_runner.cc

@@ -8,7 +8,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "gin/array_buffer.h"
 #include "gin/converter.h"
@@ -61,7 +60,6 @@ void RunTestFromFile(const base::FilePath& path, FileRunnerDelegate* delegate,
   ASSERT_TRUE(ReadFileToString(path, &source));
 
   base::MessageLoop message_loop;
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler;
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
   gin::V8Initializer::LoadV8Snapshot();

gin/test/v8_test.h

@@ -10,7 +10,6 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
-#include "base/test/scoped_async_task_scheduler.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "v8/include/v8.h"
 
@@ -30,11 +29,6 @@ class V8Test : public testing::Test {
 
  protected:
   base::MessageLoop message_loop_;
-
-  // Required by gin::V8Platform::CallOnBackgroundThread(). Can't be a
-  // ScopedTaskScheduler because v8 synchronously waits for tasks to run.
-  base::test::ScopedAsyncTaskScheduler scoped_async_task_scheduler_;
-
   std::unique_ptr<IsolateHolder> instance_;
   v8::Persistent<v8::Context> context_;
 

gin/v8_platform.cc

@@ -7,8 +7,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/sys_info.h"
-#include "base/task_scheduler/post_task.h"
-#include "base/task_scheduler/task_scheduler.h"
+#include "base/threading/worker_pool.h"
 #include "base/trace_event/trace_event.h"
 #include "gin/per_isolate_data.h"
 
@@ -43,11 +42,6 @@ class IdleTaskWithLocker : public v8::IdleTask {
   DISALLOW_COPY_AND_ASSIGN(IdleTaskWithLocker);
 };
 
-base::TaskTraits GetBackgroundThreadTaskTraits() {
-  return base::TaskTraits().WithShutdownBehavior(
-      base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN);
-}
-
 }  // namespace
 
 // static
@@ -58,16 +52,25 @@ V8Platform::V8Platform() {}
 V8Platform::~V8Platform() {}
 
 size_t V8Platform::NumberOfAvailableBackgroundThreads() {
-  return base::TaskScheduler::GetInstance()
-      ->GetMaxConcurrentTasksWithTraitsDeprecated(
-          GetBackgroundThreadTaskTraits());
+  // WorkerPool will currently always create additional threads for posted
+  // background tasks, unless there are threads sitting idle (on posix).
+  // Indicate that V8 should create no more than the number of cores available,
+  // reserving one core for the main thread.
+  const size_t available_cores =
+    static_cast<size_t>(base::SysInfo::NumberOfProcessors());
+  if (available_cores > 1) {
+    return available_cores - 1;
+  }
+  return 1;
 }
 
 void V8Platform::CallOnBackgroundThread(
     v8::Task* task,
     v8::Platform::ExpectedRuntime expected_runtime) {
-  base::PostTaskWithTraits(FROM_HERE, GetBackgroundThreadTaskTraits(),
-                           base::Bind(&v8::Task::Run, base::Owned(task)));
+  base::WorkerPool::PostTask(
+      FROM_HERE,
+      base::Bind(&v8::Task::Run, base::Owned(task)),
+      expected_runtime == v8::Platform::kLongRunningTask);
 }
 
 void V8Platform::CallOnForegroundThread(v8::Isolate* isolate, v8::Task* task) {

mojo/edk/js/tests/BUILD.gn

@@ -23,7 +23,6 @@ group("tests") {
 test("mojo_js_integration_tests") {
   deps = [
     ":js_to_cpp_bindings",
-    "//base/test:test_support",
     "//gin:gin_test",
     "//mojo/common",
     "//mojo/edk/js",