Blimp: create MessageDispatcher class.

blimp/common/net/message_dispatcher.cc

Index: blimp/common/net/message_dispatcher.cc
diff --git a/blimp/common/net/message_dispatcher.cc b/blimp/common/net/message_dispatcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..14115736a2214c326f3b0f232dfab51d736ebd76
--- /dev/null
+++ b/blimp/common/net/message_dispatcher.cc
@@ -0,0 +1,55 @@
+// Copyright 2015 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 "blimp/common/net/message_dispatcher.h"
+
+#include <string>
+
+#include "base/strings/stringprintf.h"
+
+namespace blimp {
+namespace {
+
+std::string BlimpMessageToDebugString(const BlimpMessage& message) {
+  return base::StringPrintf("<message type=%d>", message.type());
+}
+
+}  // namespace
+
+MessageDispatcher::MessageDispatcher() {}
+
+MessageDispatcher::~MessageDispatcher() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[Wez, 2015/10/02 01:10:29]

nit: Doesn't thread-checker do this for you, and before the std::map dtor, since
it's the final member of the class?

[Kevin M, 2015/10/02 22:02:00]

I guess it would, neato!

+}
+
+void MessageDispatcher::AddHandler(BlimpMessage::Type type, Handler* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (feature_handler_map_.find(type) == feature_handler_map_.end()) {
+    feature_handler_map_.insert(std::make_pair(type, handler));
+  } else {
+    LOG(ERROR) << "Handler already registered for type=" << type << ".";

[Wez, 2015/10/02 01:10:29]

Replace this with a DCHECK; it's a coding-time error to add >1

[Kevin M, 2015/10/02 22:02:00]

Done.

+  }
+}
+
+void MessageDispatcher::Dispatch(const BlimpMessage& message) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  auto handler_iter = feature_handler_map_.find(message.type());

[Wez, 2015/10/02 01:12:09]

Note that for the small numbers of message-types we'll be dealing with, a map
lookup may actually involve sufficient work (e.g. in terms of memory accesses)
to be slower than a simple linear if..elseif... sequence of tests.

[Kevin M, 2015/10/02 22:02:00]

How about this, SmallMap? Linear comparisons across a contiguous array; should
be fast for small numbers of things. It cuts down on code duplication vs. adding
conditionals and fields for every feature type.

+  if (handler_iter == feature_handler_map_.end()) {
+    LOG(ERROR) << "No registered handler for "

[Wez, 2015/10/02 01:10:29]

There's no real point logging this - who will see it? :P

We should have a UMA stat for this case and DCHECK on it; in Release builds we
should probably disconnect the client.

[Kevin M, 2015/10/02 22:02:00]

OK. Added a bool return value so the session/channel can take the appropriate
action.

+               << BlimpMessageToDebugString(message) << ".";
+    return;
+  }
+
+  DCHECK(handler_iter->second);

[Wez, 2015/10/02 01:10:29]

This DCHECK should be on |handler| when it's passed into AddHandler, since
that's the path through which a nullptr handler would have reached here.

[Kevin M, 2015/10/02 22:02:00]

Done

+  if (!handler_iter->second->Validate(message)) {
+    LOG(WARNING) << BlimpMessageToDebugString(message)

[Wez, 2015/10/02 01:10:29]

See above re logging vs DCHECK & disconnect.

[Kevin M, 2015/10/02 22:02:00]

Done.

+                 << " rejected by handler.";
+    return;
+  }
+
+  handler_iter->second->OnMessage(message);
+}
+
+}  // namespace blimp