Migrate Bind to BindOnce or BindRepeating in //base/message_loop

base/win/message_window.cc

Index: base/win/message_window.cc
diff --git a/base/win/message_window.cc b/base/win/message_window.cc
index 155047e4aeeeecb66ad5b4d046e728cd162cfe6f..97dd78128285678955c4355a2d371aff869e5b2a 100644
--- a/base/win/message_window.cc
+++ b/base/win/message_window.cc
@@ -81,13 +81,13 @@ MessageWindow::~MessageWindow() {
   }
 }
 
-bool MessageWindow::Create(const MessageCallback& message_callback) {
-  return DoCreate(message_callback, NULL);
+bool MessageWindow::Create(MessageCallback message_callback) {
+  return DoCreate(std::move(message_callback), NULL);

[dcheng, 2017/04/17 17:59:30]

Actually, I do have one question here:

this avoids refcount churn, but I think it also makes these functions slightly
more expensive to call since the caller needs to emit instructions for invoking
the move ctor now, rather than simply passing a reference.

Do we have any measurements on how/if this will affect things?

(This question shouldn't block: we can resolve this independent of this CL, as I
think it's not too hard to fix these globally if we decide that we need to
mandate one style or another)

[tzik, 2017/04/18 05:20:29]

I believe the pass-by-value of Callback is lightweight enough comparing to the
pass-by-ref, and in this case, the generated code for each of them are likely
the same after the inlining and the compiler optimization.

On a simple case below on Linux release build, pass-by-value and pass-by-ref
generated the same binaries for |baz| and |qux|:
  void foo(const base::Closure& cb);
  void bar(base::Closure cb);

  void baz() { foo(base::Bind([]{})); }
  void qux() { bar(base::Bind([]{})); }

Stepping into it for more details with a pass-by-value function call:
  void foo(Closure);

  void bar() {
    Closure cb = base::Bind([]{});
    foo(std::move(cb));
  }

The caller of |foo| makes a temporary object, and pass it by ref.
  void bar() {
    Closure cb = base::Bind([]{});

    // The caller creates a temporary object, and pass it by ref.
    Closure temporary_object = std::move(cb);
    foo(&temporary_object);
  }

Then, everything is inlined as:
  void bar() {
    BindStateBase* cb_bind_state = base::Bind([]{}).bind_state_;

    // inlined from move-construction of Callback.
    BindStateBase* bind_state = cb_bind_state;
    cb_bind_state = nullptr;

    foo(&bind_state);

    // inlined from scoped_refptr dtor.
    if (bind_state)
      bind_state->Release();

    // inlined from scoped_refptr dtor.
    if (cb_bind_state)
      cb_bind_state->Release();
  }

Finally, compiler optimization should remove intermediate variables and
always-false condition:
  void bar() {
    BindStateBase* bind_state = base::Bind([]{});
    foo(&bind_state);
    if (bind_state)
      bind_state->Release();
  }
which is equivalent to pass-by-ref code.

 }
 
-bool MessageWindow::CreateNamed(const MessageCallback& message_callback,
+bool MessageWindow::CreateNamed(MessageCallback message_callback,
                                 const string16& window_name) {
-  return DoCreate(message_callback, window_name.c_str());
+  return DoCreate(std::move(message_callback), window_name.c_str());
 }
 
 // static
@@ -96,13 +96,13 @@ HWND MessageWindow::FindWindow(const string16& window_name) {
                       window_name.c_str());
 }
 
-bool MessageWindow::DoCreate(const MessageCallback& message_callback,
+bool MessageWindow::DoCreate(MessageCallback message_callback,
                              const wchar_t* window_name) {
   DCHECK(CalledOnValidThread());
   DCHECK(message_callback_.is_null());
   DCHECK(!window_);
 
-  message_callback_ = message_callback;
+  message_callback_ = std::move(message_callback);
 
   WindowClass& window_class = g_window_class.Get();
   window_ = CreateWindow(MAKEINTATOM(window_class.atom()), window_name, 0, 0, 0,