Blimp: define HeliumTransport/HeliumStream interfaces.

blimp/net/helium/helium_stream.h

+// Copyright 2016 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_HELIUM_HELIUM_STREAM_H_
+#define BLIMP_NET_HELIUM_HELIUM_STREAM_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "blimp/net/helium/helium_result.h"
+
+namespace blimp {
+
+class HeliumMessage;
+
+// Pure virtual interface for HeliumMessage-oriented transport streams.
+// Details about how the HeliumStream is bound to the network layer are handled
+// by subclasses of HeliumStream.
+class HeliumStream {
+ public:
+  class Delegate {
+   public:
+    // Invoked when the HeliumStream is disconnected.
+    //
+    // An observer which owns |this| may delete the HeliumStream in response to
+    // receiving an OnStreamDisconnected() notification. The deletion method
+    // must be asynchronously posted to occur *after* the OnStreamDisconnected()
+    // handler, not during.
+    virtual void OnStreamDisconnected(HeliumStream* stream,
+                                      HeliumResult reason) = 0;

[Garrett Casto, 2016/10/11 22:33:22]

We shouldn't need this and ReceiveMessage(). If we are disconnected,
ReceiveMessage() should just invoke the callback with
HeliumResult::DISCONNECTED.

[Kevin M, 2016/10/11 23:25:16]

Done.

+  };
+
+  virtual ~HeliumStream() = default;
+
+  virtual void SetDelegate(std::unique_ptr<Delegate> delegate) = 0;
+
+  // Sends |helium_message| over the Stream. |callback| is invoked when the
+  // message is sent (or otherwise moved to the low-level write buffers),
+  // which signals the caller that it is clear to send another message.
+  //
+  // The caller is responsible for ensuring that only one outstanding
+  // SendMessage() call is made at a time.
+  virtual void SendMessage(
+      std::unique_ptr<HeliumMessage> helium_message,
+      const base::Callback<void(HeliumResult)>& callback) = 0;

[perumaal, 2016/10/11 23:09:39]

Ah I now see this problem: The caller of SendMessage would need to bookkeep the
list of messages...
(As they cannot send a message until the callback is invoked). This seems
unnecessary complexity to me and would result in some sort of locks *above* the
stream layer (specifically the sync layer).

Can we just remove the 'The caller is responsible...'? Based on my current
implementation, this detail (of buffering during sending) lives in the stream
implementation not in the caller.

Note that for *receiving* this is a completely different story: you can
basically buffer as you want (or *not*, which is also fine) and this imposes no
locks or book-keeping in any layer including the stream (as the completion queue
would take care of it for us).

[Kevin M, 2016/10/11 23:25:16]

That's by design, actually. The construction of HeliumMessages is done lazily,
at the time of delivery to the transport layer. This means that when we are
clear to send, the data that we transmit is as fresh as we can make it, w/o
buffered clutter from bursts of modifications.

+
+  // Asynchronously reads a HeliumMessage from the stream.
+  // The caller is responsible for ensuring that only one outstanding
+  // ReceiveMessage() call is made at a time.
+  // The implementation classes should refrain from preemptively buffering
+  // incoming messages to ensure proper pushback w/transport-level flow control.

[Garrett Casto, 2016/10/11 22:33:22]

Nit: Sounds like we explicitly are okay with pre-emptively buffering at this
point, so we can probably remove this sentence.

[Kevin M, 2016/10/11 23:25:16]

Done.

+  virtual void ReceiveMessage(
+      base::Callback<void(std::unique_ptr<HeliumMessage>, HeliumResult)>

[perumaal, 2016/10/11 22:36:36]

Could we alias this as HeliumMsgReceivedCallback?

[Kevin M, 2016/10/11 23:25:16]

Done.

+          on_receive_cb) = 0;
+};
+
+// Abstract base class, to be used by non-test implementers of HeliumStream.
+class HeliumStreamBase : public HeliumStream {
+ public:
+  HeliumStreamBase();
+  ~HeliumStreamBase() override;
+
+  // HeliumStream implementation.
+  void SetDelegate(std::unique_ptr<Delegate> delegate) override;
+
+ protected:
+  Delegate* delegate() { return delegate_.get(); }
+
+ private:
+  std::unique_ptr<Delegate> delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(HeliumStreamBase);
+};
+
+}  // namespace blimp
+
+#endif  // BLIMP_NET_HELIUM_HELIUM_STREAM_H_