Blimp: create MessageDispatcher class.

blimp/net/blimp_message_dispatcher.h

+// 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.
+
+#ifndef BLIMP_NET_BLIMP_MESSAGE_DISPATCHER_H_
+#define BLIMP_NET_BLIMP_MESSAGE_DISPATCHER_H_
+
+#include <map>
+
+#include "base/containers/small_map.h"
+#include "base/threading/thread_checker.h"
+#include "blimp/common/proto/blimp_message.pb.h"
+#include "blimp/net/blimp_message_receiver.h"
+#include "blimp/net/blimp_net_export.h"
+
+namespace blimp {
+
+// Multiplexing BlimpMessageReceiver which routes BlimpMessages to other
+// receivers based on the messages' feature type.
+class BLIMP_NET_EXPORT BlimpMessageDispatcher : public BlimpMessageReceiver {
+ public:
+  BlimpMessageDispatcher();
+  ~BlimpMessageDispatcher() override;
+
+  // Registers a message receiver which will process all messages
+  // with the type |type|.

[Wez, 2015/10/09 23:15:34]

nit: Suggest "... of the specified |type|."

[Kevin M, 2015/10/10 00:42:34]

Done.

+  // Only one handler may be added per type.
+  //
+  // The caller must ensure that the lifetime of |handler| exceeds the lifetime
+  // of |this|.

[Wez, 2015/10/09 23:15:34]

nit: Strictly |handler| only need be valid when OnMessage() is called.

[Kevin M, 2015/10/10 00:42:34]

Done.

+  void AddReceiver(BlimpMessage::Type type, BlimpMessageReceiver* handler);
+
+  // Takes a BlimpMessage and dispatches it to the appropriate tab and
+  // feature-specific handler, based on the value of |message.type|.
+  //
+  // Logs an error if there is no BlimpMessageReceiver registered for
+  // |message.type|, or if the BlimpMessageReceiver failed to validate the
+  // message.

[Wez, 2015/10/09 23:15:34]

This API is part of the BlimpMessageReceiver interface - no need to re-document
it here. Suggest instead just a one line comment "// BlimpMessageReceiver
implementation.".

As regards behaviour in case of no handler registered, we should be returning an
error code, not logging an error. You can probably document the no-receiver
behaviour on the AddReceiver API.

[Kevin M, 2015/10/10 00:42:34]

Done.

+  bool OnMessage(const BlimpMessage& message) override;
+
+ private:
+  base::SmallMap<std::map<BlimpMessage::Type, BlimpMessageReceiver*>>
+      feature_receiver_map_;
+  base::ThreadChecker thread_checker_;

[Wez, 2015/10/09 23:15:34]

nit: This seems overkill - I don't think there's any reason we'd expect the
caller to get confused about which thread this class is being called on?

[Kevin M, 2015/10/10 00:42:34]

OK, I'm still figuring out the right degree of anal-retentiveness to employ for
these sorts of things.

[Wez, 2015/10/12 23:47:21]

Yeah, it's a fine line!

+
+  DISALLOW_COPY_AND_ASSIGN(BlimpMessageDispatcher);
+};
+
+}  // namespace blimp
+
+#endif  // BLIMP_NET_BLIMP_MESSAGE_DISPATCHER_H_