[Chromoting] Remove DataChannelManagerCallbacks constructor parameter of ClientSession

remoting/protocol/data_channel_manager.cc

 // Copyright 2017 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 "remoting/protocol/data_channel_manager.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "remoting/protocol/message_pipe.h"
 
 namespace remoting {
 namespace protocol {
 
 DataChannelManager::DataChannelManager() = default;
 DataChannelManager::~DataChannelManager() = default;
 
 void DataChannelManager::RegisterCreateHandlerCallback(
     const std::string& prefix,
     CreateHandlerCallback constructor) {
   DCHECK(!prefix.empty());
   DCHECK(constructor);
-  constructors_.push_back(std::make_pair(prefix, constructor));
+  constructors_.push_back(std::make_pair(prefix, std::move(constructor)));

[jarhar, 2017/07/05 17:51:42]

just curious, what effect does using std::move have on the callback? Why was it
not needed before?

[Hzj_jie, 2017/07/05 18:34:31]

It does not impact the logic, but only the performance: Callback implements a
move constructor, so using it here can move its variables to avoid copying them
(bound objects).

So in general we should use std::move() to move Callback instances when passing
them into containers / functions. But we definitely missed most of them in the
entire Chromium.

I do not think in a short time, C++ will have an "automatic move" rule. So we
may need to take care of this issue for a long time.

 }
 
 bool DataChannelManager::OnIncomingDataChannel(
     const std::string& name,
     std::unique_ptr<MessagePipe> pipe) {
   for (auto& constructor : constructors_) {
     if (name.find(constructor.first) == 0) {
       constructor.second.Run(name, std::move(pipe));
       return true;
     }
   }
   return false;
 }
 
 }  // namespace protocol
 }  // namespace remoting