base: Make it possible to replace the MessageLoop's task runner

base/message_loop/message_loop.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 "base/message_loop/message_loop.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 349 matching lines @@
 }
 
 bool MessageLoop::IsIdleForTesting() {
   // We only check the imcoming queue|, since we don't want to lock the work
   // queue.
   return incoming_task_queue_->IsIdleForTesting();
 }
 
 //------------------------------------------------------------------------------
 
+// static
 scoped_ptr<MessageLoop> MessageLoop::CreateUnbound(
     Type type, MessagePumpFactoryCallback pump_factory) {
   return make_scoped_ptr(new MessageLoop(type, pump_factory));
 }
 
 MessageLoop::MessageLoop(Type type, MessagePumpFactoryCallback pump_factory)
     : type_(type),
 #if defined(OS_WIN)
       pending_high_res_tasks_(0),
       in_high_res_mode_(false),
@@ skipping 16 matching lines @@
   if (!pump_factory_.is_null())
     pump_ = pump_factory_.Run();
   else
     pump_ = CreateMessagePumpForType(type_);
 
   DCHECK(!current()) << "should only have one message loop per thread";
   lazy_tls_ptr.Pointer()->Set(this);
 
   incoming_task_queue_->StartScheduling();
   task_runner_->BindToCurrentThread();
-  thread_task_runner_handle_.reset(new ThreadTaskRunnerHandle(task_runner_));
+  SwapTaskRunner(task_runner_);

[danakj, 2015/06/29 19:07:23]

Why do we want to support both:

1. SwapTaskRunner
2. BindToCurrentThread

and

1. BindToCurrentThread
2. SwapTaskRunner

?

[danakj, 2015/06/29 19:07:23]

can we DCHECK that the return from this is null?

can you .Pass() the task_runner_?

[alex clarke (OOO till 29th), 2015/06/30 10:51:43]

I don't know, and Sami is currently OOO so I can't ask him.

[alex clarke (OOO till 29th), 2015/06/30 10:51:43]

Done.

+}
+
+scoped_refptr<BindableSingleThreadTaskRunner> MessageLoop::SwapTaskRunner(
+    scoped_refptr<BindableSingleThreadTaskRunner> task_runner) {
+  // If this message loop was already bound to a thread, ensure the task runner
+  // belongs to the same thread and update the thread task runner handle.
+  // Otherwise the task runner will be bound later in BindToCurrentThread().
+  if (pump_) {
+    DCHECK_EQ(this, current());
+    DCHECK(task_runner->BelongsToCurrentThread());
+    // This reset() is needed or ThreadTaskRunnerHandle::Get() will crash.

[danakj, 2015/06/29 19:07:23]

We're not calling Get() here.. can you expand the explanation in this comment?

[alex clarke (OOO till 29th), 2015/06/30 10:51:43]

It's there because in the original review kinuko asked why there are two resets.
 This does look weird, and it's because reset() doesn't behave the way we would
expect.

+    thread_task_runner_handle_.reset();
+    thread_task_runner_handle_.reset(new ThreadTaskRunnerHandle(task_runner));
+  }
+  task_runner_.swap(task_runner);
+  return task_runner;
 }
 
 void MessageLoop::RunHandler() {
   DCHECK_EQ(this, current());
 
   StartHistogrammer();
 
 #if defined(OS_WIN)
   if (run_loop_->dispatcher_ && type() == TYPE_UI) {
     static_cast<MessagePumpForUI*>(pump_.get())->
@@ skipping 310 matching lines @@
       persistent,
       mode,
       controller,
       delegate);
 }
 #endif
 
 #endif  // !defined(OS_NACL_SFI)
 
 }  // namespace base