[Chromoting] Add DataChannelManager to manage optional incoming data channels

remoting/protocol/data_channel_manager.cc

Index: remoting/protocol/data_channel_manager.cc
diff --git a/remoting/protocol/data_channel_manager.cc b/remoting/protocol/data_channel_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9444a39328343e0cd61bb0b7d0e3852c3e4e0e49
--- /dev/null
+++ b/remoting/protocol/data_channel_manager.cc
@@ -0,0 +1,56 @@
+// 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/bind.h"
+#include "base/location.h"
+#include "remoting/protocol/data_channel_handler.h"

[joedow, 2017/06/06 23:33:25]

Remove this header since it isn't used here.

[Hzj_jie, 2017/06/07 17:31:25]

Done.

+
+namespace remoting {
+namespace protocol {
+
+DataChannelManager::DataChannelManager() = default;
+DataChannelManager::~DataChannelManager() = default;
+
+bool DataChannelManager::RegisterCreateHandlerCallback(
+    const std::string& regex,

[joedow, 2017/06/06 23:33:25]

replace 'regex' with 'prefix'

[Hzj_jie, 2017/06/07 17:31:24]

Done.

+    CreateHandlerCallback constructor) {
+  if (regex.empty() || !constructor) {

[joedow, 2017/06/06 23:33:25]

Just curious, shouldn't calling this method with an empty prefix or empty
factory callback be a critical error?

I think the if statement should be replaced with:
DCHECK(!prefix.empty());
DCHECK(constructor);

This method should fail hard if the caller does not call it correctly to make
debugging easier.

[Hzj_jie, 2017/06/07 17:31:24]

Done.

+    return false;
+  }
+
+  constructors_.push_back(std::make_pair(regex, constructor));
+  return true;
+}
+
+bool DataChannelManager::OnIncomingDataChannel(
+    const std::string& name,
+    std::unique_ptr<MessagePipe> pipe) {
+#if DCHECK_IS_ON()

[joedow, 2017/06/06 23:33:25]

I think you can remove this block now that you aren't using regexes anymore.  As
long as we put the channel-prefixes in a common place then any overlap should be
obvious.

[Hzj_jie, 2017/06/07 17:31:25]

Done.

+  // Ensures the name matches only one regex.
+  bool matched = false;
+  for (auto& constructor : constructors_) {
+    if (name.find(constructor.first) == 0) {
+      CHECK(!matched);
+      matched = true;
+    }
+  }
+  if (!matched) {
+    return false;
+  }
+#endif
+  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