Blimp: define HeliumTransport/HeliumStream interfaces.

blimp/net/helium/helium_stream.h

Index: blimp/net/helium/helium_stream.h
diff --git a/blimp/net/helium/helium_stream.h b/blimp/net/helium/helium_stream.h
new file mode 100644
index 0000000000000000000000000000000000000000..3e73a5bdf32aaefec1a4fdad1188916a70b4a7f6
--- /dev/null
+++ b/blimp/net/helium/helium_stream.h
@@ -0,0 +1,71 @@
+// 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/observer_list.h"
+#include "net/base/completion_callback.h"
+
+namespace blimp {
+
+class HeliumMessage;
+
+// Abstract base class 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 Observer {
+   public:
+    // Informs observers that the HeliumStream is disconnected.
+    // Any outstanding read or write callbacks will be dropped and never
+    // invoked.
+    //
+    // 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) = 0;
+  };
+
+  HeliumStream();
+  virtual ~HeliumStream();
+
+  // 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 net::CompletionCallback& callback) = 0;
+
+  // Asynchronously reads a HeliumMessage from the stream.
+  // The caller is responsible for ensuring that only one outstanding
+  // ReceiveMessage() call is made at a time.

[Brian Goldman, 2016/09/29 18:20:21]

Maybe I'm just not familiar with the pattern, but it took me a solid one minute
of thinking to understand what's going on here.

I think this documentation comment could be clearer by saying what the caller is
doing/expecting when this method is called. My best shot at it: "Invokes a
callback when a HeliumMessage is available to be read from the stream."

Also, what's the reason for using this kind of interface, where the caller
having to make sure there's only one pending request? I'm a bit worried about
"undefined" behavior with that kind of thing. Could you instead have some way to
just register a callback that gets invoked repeatedly? I haven't thought a lot
about the exact use cases, but if there's a 1:1 relationship between caller
object and HeliumStream instance, that might be simpler.

[scf, 2016/09/29 19:12:22]

I agree. Or what we could do is make the impl of HeliumStream "bark" if its
called two times. 

+  virtual void ReceiveMessage(
+      base::Callback<void(std::unique_ptr<HeliumMessage>)> on_receive_cb) = 0;
+
+  // Lets other objects observe the Stream for major state changes (e.g.
+  // connectivity).
+  void AddObserver(Observer* observer);

[scf, 2016/09/29 19:12:22]

dumb q: What is the rationale for allowing multiple observers but just one
caller to ReceiveMessage()? 

+
+ protected:
+  // Notifies observers that the underlying transport for this Stream is no
+  // longer connected. Must be called by a subclass before the HeliumStream
+  // can be torn down.
+  void NotifyStreamDisconnected();
+
+ private:
+  base::ObserverList<HeliumStream::Observer> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(HeliumStream);
+};
+
+}  // namespace blimp
+
+#endif  // BLIMP_NET_HELIUM_HELIUM_STREAM_H_