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

base/win/message_window.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/win/message_window.h"
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/win/current_module.h"
@@ skipping 63 matching lines @@
 
 MessageWindow::~MessageWindow() {
   DCHECK(CalledOnValidThread());
 
   if (window_ != NULL) {
     BOOL result = DestroyWindow(window_);
     DCHECK(result);
   }
 }
 
-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
 HWND MessageWindow::FindWindow(const string16& window_name) {
   return FindWindowEx(HWND_MESSAGE, NULL, kMessageWindowClassName,
                       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,
                          0, 0, HWND_MESSAGE, 0, window_class.instance(), this);
   if (!window_) {
     PLOG(ERROR) << "Failed to create a message-only window";
     return false;
   }
 
   return true;
@@ skipping 40 matching lines @@
     LRESULT message_result;
     if (self->message_callback_.Run(message, wparam, lparam, &message_result))
       return message_result;
   }
 
   return DefWindowProc(hwnd, message, wparam, lparam);
 }
 
 }  // namespace win
 }  // namespace base