Blimp: create MessageDispatcher class.

blimp/common/net/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_COMMON_NET_MESSAGE_DISPATCHER_H_
+#define BLIMP_COMMON_NET_MESSAGE_DISPATCHER_H_
+
+#include <map>
+
+#include "base/containers/small_map.h"
+#include "base/threading/thread_checker.h"
+#include "blimp/common/blimp_common_export.h"
+#include "blimp/common/proto/blimp_message.pb.h"
+
+namespace blimp {
+
+// Routes BlimpMessages to feature-specific handlers according to the messages'
+// types.
+class BLIMP_COMMON_EXPORT MessageDispatcher {

[Wez, 2015/10/07 00:20:15]

Since this dispatches BlimpMessages, let's call it BlimpMessageDispatcher?

[Kevin M, 2015/10/07 00:58:51]

Done.

+ public:
+  class Handler {
+   public:
+    // Returns true if |this| is capable of handling |message|.
+    virtual bool Validate(const BlimpMessage& message) const = 0;

[Wez, 2015/10/07 00:20:15]

As previously noted, what's the value of the Validate()/OnMessage() split?

[Kevin M, 2015/10/07 00:58:51]

I guess it doesn't make a lot of sense to validate it at this layer - the
delegates can report errors in whichever way suits them.

I'll have OnMessage return a bool.

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

bool or net::Error? net::Error means you have to pick an error-code but it also
means that we can have the message pump log something vaguely helpful if a
handler balks at a message.

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

Done.

+
+    // Receives |message| for processing.
+    // Validate() is guaranteed to be called prior to this call to OnMessage().
+    // The handler is responsible for routing the call through the appropriate
+    // thread.
+    virtual void OnMessage(const BlimpMessage& message) = 0;
+  };
+
+  MessageDispatcher();
+  ~MessageDispatcher();
+
+  // Registers a message dispatcher handler which will process all messages
+  // with the type |type|.
+  // Only one handler may be added per type.
+  //
+  // The caller must ensure that the lifetime of |handler| exceeds the lifetime
+  // of |this|.
+  void AddHandler(BlimpMessage::Type type, Handler* 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 Handler registered for
+  // |message.type|, or if the Handler failed to validate the message.
+  //
+  // Returns true if |message| was dispatched successfully.
+  // Returns false if no handler exists for |message|, or if the handler
+  // rejected |message|.
+  bool Dispatch(const BlimpMessage& message) const;

[Wez, 2015/10/07 00:20:15]

Why do we want this to have a separate message-handling API to the handlers
themselves?

If we have a BlimpMessageHandler interface then MessageDispatcher just becomes a
de-multiplexing handler, and that in turn means that the BlimpMessagePump can
take any arbitrary BlimpMessageHandler and pump BlimpMessages to it.

Suggest making an explicit comment that calling code should typically treat
Dispatch errors as fatal, and e.g. disconnect the peer.

[Kevin M, 2015/10/07 00:58:52]

Good idea, done.

See comments on BlimpMessageReceiver regarding disconnection policy.

+
+ private:
+  base::SmallMap<std::map<BlimpMessage::Type, Handler*>> feature_handler_map_;
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageDispatcher);
+};
+
+}  // namespace blimp
+
+#endif  // BLIMP_COMMON_NET_MESSAGE_DISPATCHER_H_