Adding task runner/ message loop to tests that use IsolateHolder.

content/test/test_blink_web_unit_test_support.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/test/test_blink_web_unit_test_support.h"
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/message_loop/message_loop.h"

[Sami, 2015/05/05 13:55:54]

Do you need this for anything?

[ssid, 2015/05/05 16:00:06]

Done.

 #include "base/path_service.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/thread_task_runner_handle.h"
 #include "components/scheduler/renderer/renderer_scheduler.h"
 #include "components/scheduler/renderer/webthread_impl_for_renderer_scheduler.h"
 #include "content/test/mock_webclipboard_impl.h"
 #include "content/test/web_gesture_curve_mock.h"
 #include "content/test/web_layer_tree_view_impl_for_testing.h"
 #include "content/test/weburl_loader_mock_factory.h"
 #include "media/base/media.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "storage/browser/database/vfs_backend.h"
@@ skipping 12 matching lines @@
 
 #if defined(OS_MACOSX)
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #endif
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
 #include "gin/v8_initializer.h"
 #endif
 
+namespace {
+
+class DummyTaskRunner : public base::SingleThreadTaskRunner {
+ public:
+  DummyTaskRunner() {}
+
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       const base::Closure& task,
+                       base::TimeDelta delay) override {
+    NOTREACHED();
+    return false;
+  }
+
+  bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
+                                  const base::Closure& task,
+                                  base::TimeDelta delay) override {
+    NOTREACHED();
+    return false;
+  }
+
+  // Always returns true to avoid triggering DCHECKs.

[Sami, 2015/05/05 13:55:54]

Could you make this do the right thing instead? Some call sites change their
behavior based on what this method returns.

[ssid, 2015/05/05 16:00:06]

Done.

+  bool RunsTasksOnCurrentThread() const override { return true; }
+
+ protected:
+  ~DummyTaskRunner() override {}
+
+  DISALLOW_COPY_AND_ASSIGN(DummyTaskRunner);
+};
+
+}  // namespace
+
 namespace content {
 
 TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport() {
 #if defined(OS_MACOSX)
   base::mac::ScopedNSAutoreleasePool autorelease_pool;
 #endif
 
   url_loader_factory_.reset(new WebURLLoaderMockFactory());
   mock_clipboard_.reset(new MockWebClipboardImpl());
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
   gin::V8Initializer::LoadV8Snapshot();
 #endif
 
+  scoped_refptr<base::SingleThreadTaskRunner> dummy_task_runner;
+  scoped_ptr<base::ThreadTaskRunnerHandle> handle;

[Sami, 2015/05/05 13:55:54]

nit: |handle| is a little too nonspecific for a variable name I think.

[ssid, 2015/05/05 16:00:06]

Done.

   if (base::MessageLoopProxy::current()) {
     renderer_scheduler_ = scheduler::RendererScheduler::Create();
     web_thread_.reset(new scheduler::WebThreadImplForRendererScheduler(
         renderer_scheduler_.get()));
+  } else {
+    // Dummy task runner is initialized here because the blink::initialize
+    // creates IsolateHolder which needs the current task runner handle. There
+    // should be no task posted to this handle. The message loop is not created
+    // before this initialization because tests need different kinds of message
+    // loop which is not known upfront and each test suite that uses thread

[Sami, 2015/05/05 13:55:54]

nit: "posted to this handle" => "posted to this task runner"

The final sentence is a little hard to read. How about:

The message loop is not created before this initialization because some tests
need specific kinds of message loops, and their types are not known upfront.
Some tests also create their own thread bundles and message loops, and doing the
same in TestBlinkWebUnitTestSupport would introduce a conflict.

[ssid, 2015/05/05 16:00:06]

Done.

+    // bundle or message loop initializes its own.
+    dummy_task_runner = make_scoped_refptr(new DummyTaskRunner());
+    handle.reset(new base::ThreadTaskRunnerHandle(dummy_task_runner));
   }
 
   blink::initialize(this);
+
+  if (dummy_task_runner.get()) {

[Sami, 2015/05/05 13:55:54]

There's no need to reset the task explicitly is there? Please mention in the
comment though that this task runner will only exist within the scope of this
function.

[Primiano Tucci (use gerrit), 2015/05/05 14:26:51]

Nit: just if (dummy_task_runner), scoped_refptr is already boolean-testable.

On 2015/05/05 13:55:54, Sami wrote:
UH? I think he actually needs to reset the TLS slot (by resetting
dummy_task_runner_handle) and to destroy the dummy_task_runner itself, right?

[Sami, 2015/05/05 14:35:46]

I meant the destructors of both variables would automatically do that when they
go out of scope of this function. I don't see any harm in leaving the task
runner around for a few more lines of initialization.

[ssid, 2015/05/05 16:00:06]

Done.

+    // The dummy task runner is destroyed after the initialization is done.
+    handle.reset();
+    dummy_task_runner = nullptr;
+  }
+
   blink::setLayoutTestMode(true);
   blink::WebSecurityPolicy::registerURLSchemeAsLocal(
       blink::WebString::fromUTF8("test-shell-resource"));
   blink::WebSecurityPolicy::registerURLSchemeAsNoAccess(
       blink::WebString::fromUTF8("test-shell-resource"));
   blink::WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(
       blink::WebString::fromUTF8("test-shell-resource"));
   blink::WebSecurityPolicy::registerURLSchemeAsEmptyDocument(
       blink::WebString::fromUTF8("test-shell-resource"));
   blink::WebRuntimeFeatures::enableApplicationCache(true);
@@ skipping 248 matching lines @@
   DCHECK(base::MessageLoop::current());
   DCHECK(!base::MessageLoop::current()->is_running());
   base::MessageLoop::current()->Run();
 }
 
 void TestBlinkWebUnitTestSupport::exitRunLoop() {
   base::MessageLoop::current()->Quit();
 }
 
 }  // namespace content