Implement the Stream registry in content

content/browser/streams/stream.h

Index: content/browser/streams/stream.h
diff --git a/content/browser/streams/stream.h b/content/browser/streams/stream.h
new file mode 100644
index 0000000000000000000000000000000000000000..031dc4ffd673101b53d066ffec621c2b07777df2
--- /dev/null
+++ b/content/browser/streams/stream.h
@@ -0,0 +1,94 @@
+// Copyright (c) 2013 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 CONTENT_BROWSER_STREAMS_STREAM_H_
+#define CONTENT_BROWSER_STREAMS_STREAM_H_
+
+#include <deque>

[kinuko, 2013/03/07 08:35:40]

not used?

[Zachary Kuznia, 2013/03/07 09:19:00]

Done.

+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/observer_list.h"
+#include "content/browser/download/byte_stream.h"

[darin (slow to review), 2013/03/07 07:54:03]

nit: There should probably be a TODO to factor byte_stream.h out of the download
system.  (It doesn't make sense for streams to depend on the download system.) 
I recommend sync'ing up with rdsmith@ on this since he authored the class.

+#include "googleurl/src/gurl.h"
+
+namespace net {
+class IOBuffer;
+}
+
+namespace content {
+
+class StreamReadObserver;
+class StreamRegistry;
+class StreamWriteObserver;
+
+// A stream that sends data from an arbitrary source to an internal URL
+// that can be read by an internal consumer.  It will continue to pull from the
+// original URL as long as there is data available.  It can be read from
+// multiple clients, but results are undefined if more than one client reads at
+// the same time.  This allows a reader to consume part of the stream, then pass
+// it along to another client to continue processing the stream.
+class Stream : public base::RefCountedThreadSafe<Stream> {
+ public:
+  // Creates a stream useable from the |security_origin|.
+  Stream(StreamRegistry* registry,
+         const GURL& security_origin,
+         const GURL& url);
+
+  void AddReadObserver(StreamReadObserver* observer);
+  void RemoveReadObserver(StreamReadObserver* observer);
+
+  void AddWriteObserver(StreamWriteObserver* observer);
+  void RemoveWriteObserver(StreamWriteObserver* observer);
+
+  // Adds the data in |buffer| to the stream.  Takes ownership of |buffer|.
+  void AddData(scoped_refptr<net::IOBuffer> buffer, size_t size);
+
+  // Notifies this stream that it will not be receiving any more data.
+  void Finalize();
+
+  // Reads a maximum of |buf_size| from the stream into |buf|.  Sets
+  // |*bytes_read| to the number of bytes actually read.
+  // returns false if there is no data available, but the stream is not

[kinuko, 2013/03/07 08:35:40]

nit: returns -> Returns

[Zachary Kuznia, 2013/03/07 09:19:00]

Done.

+  // finalized, and true otherwise.

[kinuko, 2013/03/07 08:35:40]

This behavior still confuses me a bit.  So we return true 1. when we have data
and also 2. when the stream is closed and has no data?  Can we make the return
value have a single meaning, say by adding another out parameter etc?

[Zachary Kuznia, 2013/03/07 09:19:00]

Changed to return an enum that matches ByteStream.

+  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read);
+
+  // Indicates whether there is space in the buffer to add more data.
+  bool can_add_data() const { return can_add_data_; }
+
+  void SetUrl(const GURL& url) { url_ = url; }

[kinuko, 2013/03/07 08:35:40]

nit: set_url() should be fine for simple inline setter

Do we need this setter btw?  It looks the StreamRegistry could be in a troubled
state if the value's changed later.

[Zachary Kuznia, 2013/03/07 09:19:00]

Removed.

+  const GURL& url() const { return url_; }
+
+  const GURL& security_origin() const { return security_origin_; }
+
+ private:
+  friend class base::RefCountedThreadSafe<Stream>;
+
+  ~Stream();
+
+  void OnSpaceAvailable();
+  void OnDataAvailable();
+
+  size_t bytes_read_;
+  bool can_add_data_;
+
+  GURL security_origin_;
+  GURL url_;
+
+  scoped_refptr<net::IOBuffer> data_;
+  size_t data_length_;
+
+  scoped_ptr<ByteStreamWriter> writer_;
+  scoped_ptr<ByteStreamReader> reader_;
+
+  StreamRegistry* registry_;

[kinuko, 2013/03/07 08:35:40]

This one's ref-counted while StreamRegistry is not.  Isn't there a case where a
Stream instance could outlive registry?

[Zachary Kuznia, 2013/03/07 09:19:00]

There should not be such a case.  Other Stream references should be cleaned up
before the registry at shutdown.

+  ObserverList<StreamReadObserver> read_observers_;
+  ObserverList<StreamWriteObserver> write_observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(Stream);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_STREAMS_STREAM_H_