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

base/message_loop/message_loop.h

 // 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.
 
 #ifndef BASE_MESSAGE_LOOP_MESSAGE_LOOP_H_
 #define BASE_MESSAGE_LOOP_MESSAGE_LOOP_H_
 
 #include <queue>
 #include <string>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/debug/task_annotator.h"
+#include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/incoming_task_queue.h"
 #include "base/message_loop/message_loop_task_runner.h"
 #include "base/message_loop/message_pump.h"
 #include "base/message_loop/timer_slack.h"
 #include "base/observer_list.h"
 #include "base/pending_task.h"
 #include "base/sequenced_task_runner_helpers.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
 #include "base/tracking_info.h"
 
 // TODO(sky): these includes should not be necessary. Nuke them.
 #if defined(OS_WIN)
 #include "base/message_loop/message_pump_win.h"
 #elif defined(OS_IOS)
 #include "base/message_loop/message_pump_io_ios.h"
 #elif defined(OS_POSIX)
 #include "base/message_loop/message_pump_libevent.h"
 #endif
 
 namespace base {
 
+class BindableSingleThreadTaskRunner;
 class HistogramBase;
 class RunLoop;
 class ThreadTaskRunnerHandle;
 class WaitableEvent;
 
 // A MessageLoop is used to process events for a particular thread.  There is
 // at most one MessageLoop instance per thread.
 //
 // Events include at a minimum Task instances submitted to PostTask and its
 // variants.  Depending on the type of message pump used by the MessageLoop
@@ skipping 239 matching lines @@
   Type type() const { return type_; }
 
   // Optional call to connect the thread name with this loop.
   void set_thread_name(const std::string& thread_name) {
     DCHECK(thread_name_.empty()) << "Should not rename this thread!";
     thread_name_ = thread_name;
   }
   const std::string& thread_name() const { return thread_name_; }
 
   // Gets the TaskRunner associated with this message loop.
-  // TODO(skyostil): Change this to return a const reference to a refptr
-  // once the internal type matches what is being returned (crbug.com/465354).
-  scoped_refptr<SingleThreadTaskRunner> task_runner() { return task_runner_; }
+  scoped_refptr<BindableSingleThreadTaskRunner> task_runner() {

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

What happened to the TODO?

[danakj, 2015/06/29 19:09:05]

Should the fact that this is Bindable really be public? Should this instead
return a STTR?

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

I agree the fact it's Bindable feels like an implementation detail that ideally
shouldn't be public.

I think we have the following options:

1. Return const scoped_refptr<BindableSingleThreadTaskRunner>&
2. Add a base_task_runner_ as well and return const
scoped_refptr<SingleThreadTaskRunner>& here
3. Regard returning a const reference as micro-optimization and leave it as is,
and remove the todo.

I think I prefer #1 or #3, WDYT?

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

That was a mistake.  I note it compiles if we change it to:

const scoped_refptr<BindableSingleThreadTaskRunner>& task_runner() {
  return task_runner_;
}

+    return task_runner_;
+  }
+
+  // Sets a new TaskRunner for this message loop and returns the previous one.
+  // If the message loop was already bound to a thread, this function must be
+  // called on that thread and the task runner must also belong to that thread.
+  // Note that changing the task runner will also affect the
+  // ThreadTaskRunnerHandle for the target thread.
+  scoped_refptr<BindableSingleThreadTaskRunner> SwapTaskRunner(
+      scoped_refptr<BindableSingleThreadTaskRunner> task_runner);

[kinuko, 2015/06/30 02:26:55]

Would we want to call this before the message loop's bound to a thread (other
than in test)?  Creating an unbound message loop is a private construction
method, if there're not such cases just asserting that the loop's already bound
might be simpler/clearer?

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

Apparently yes, see SchedulerMessageLoopDelegate::SetDefaultTaskRunner in
https://codereview.chromium.org/987193002/
I think it would have been cleaner, but it seems the method is needed. 

 
   // Enables or disables the recursive task processing. This happens in the case
   // of recursive message loops. Some unwanted message loop may occurs when
   // using common controls or printer functions. By default, recursive task
   // processing is disabled.
   //
   // Please utilize |ScopedNestableTaskAllower| instead of calling these methods
   // directly.  In general nestable message loops are to be avoided.  They are
   // dangerous and difficult to get right, so please use with extreme caution.
   //
@@ skipping 82 matching lines @@
 
   //----------------------------------------------------------------------------
  protected:
   scoped_ptr<MessagePump> pump_;
 
  private:
   friend class RunLoop;
   friend class internal::IncomingTaskQueue;
   friend class ScheduleWorkTest;
   friend class Thread;
+  FRIEND_TEST_ALL_PREFIXES(MessageLoopTest, SwapTaskRunnerBeforeBinding);
 
   using MessagePumpFactoryCallback = Callback<scoped_ptr<MessagePump>()>;
 
   // Creates a MessageLoop without binding to a thread.
   // If |type| is TYPE_CUSTOM non-null |pump_factory| must be also given
   // to create a message pump for this message loop.  Otherwise a default
   // message pump for the |type| is created.
   //
   // It is valid to call this to create a new message loop on one thread,
   // and then pass it to the thread where the message loop actually runs.
   // The message loop's BindToCurrentThread() method must be called on the
   // thread the message loop runs on, before calling Run().
   // Before BindToCurrentThread() is called only Post*Task() functions can
   // be called on the message loop.
-  scoped_ptr<MessageLoop> CreateUnbound(
+  static scoped_ptr<MessageLoop> CreateUnbound(

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

Can you make this static in a separate patch?

[kinuko, 2015/06/30 02:26:55]

This's already in ToT, you can drop it from this patch (sorry for the
confusion).

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

Acknowledged.

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

Acknowledged.

       Type type,
       MessagePumpFactoryCallback pump_factory);
 
   // Common private constructor. Other constructors delegate the initialization
   // to this constructor.
   MessageLoop(Type type, MessagePumpFactoryCallback pump_factory);
 
   // Configure various members and bind this message loop to the current thread.
   void BindToCurrentThread();
 
@@ skipping 87 matching lines @@
 
   RunLoop* run_loop_;
 
   ObserverList<TaskObserver> task_observers_;
 
   debug::TaskAnnotator task_annotator_;
 
   scoped_refptr<internal::IncomingTaskQueue> incoming_task_queue_;
 
   // The task runner associated with this message loop.
-  scoped_refptr<internal::MessageLoopTaskRunner> task_runner_;
+  scoped_refptr<BindableSingleThreadTaskRunner> task_runner_;
   scoped_ptr<ThreadTaskRunnerHandle> thread_task_runner_handle_;
 
   template <class T, class R> friend class base::subtle::DeleteHelperInternal;
   template <class T, class R> friend class base::subtle::ReleaseHelperInternal;
 
   void DeleteSoonInternal(const tracked_objects::Location& from_here,
                           void(*deleter)(const void*),
                           const void* object);
   void ReleaseSoonInternal(const tracked_objects::Location& from_here,
                            void(*releaser)(const void*),
@@ skipping 137 matching lines @@
 
 // Do not add any member variables to MessageLoopForIO!  This is important b/c
 // MessageLoopForIO is often allocated via MessageLoop(TYPE_IO).  Any extra
 // data that you need should be stored on the MessageLoop's pump_ instance.
 COMPILE_ASSERT(sizeof(MessageLoop) == sizeof(MessageLoopForIO),
                MessageLoopForIO_should_not_have_extra_member_variables);
 
 }  // namespace base
 
 #endif  // BASE_MESSAGE_LOOP_MESSAGE_LOOP_H_