[ON HOLD] Implement pull-based design for content decoding

net/filter/block_buffer.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 NET_FILTER_BLOCK_BUFFER_H
+#define NET_FILTER_BLOCK_BUFFER_H
+
+#include "base/memory/ref_counted.h"
+#include "net/base/io_buffer.h"
+
+namespace net {
+
+// A refillable input buffer, wrapping an IOBuffer.
+// The WasRefilled() and WasDrained() methods are responsible for maintaining
+// this class's state machine. A client class should call WasDrained() when
+// bytes are drained from the buffer, and WasRefilled() when new bytes are
+// placed in the underlying IOBuffer.
+class BlockBuffer {

[mmenke, 2016/04/29 19:10:54]

Can we just use DrainableIOBuffer instead?

[xunjieli, 2016/07/20 21:00:47]

Done.

+ public:
+  BlockBuffer();
+  ~BlockBuffer();
+
+  bool HasMoreBytes() const;

[Randy Smith (Not in Mondays), 2016/04/26 21:54:01]

Personal preference, therefore suggestion: I find .h files easier to read when,
if there are comments describing methods, there's a single blank line between
the methods, so it's easy to visually associate the comments with the method.

[xunjieli, 2016/07/20 21:00:47]

Acknowledged. I got rid of this class per Matt's suggestion on using
DrainableIOBuffer directly.

+  // Marks that the buffer has been refilled |filled_bytes| bytes. It is illegal

[Randy Smith (Not in Mondays), 2016/04/26 21:54:01]

nit, suggestion: I think this would be clearer with something like "Marks that
|filled_bytes| bytes has been written into the buffer starting at location
buffer().  This resets the bytes() pointer to buffer().  It is illegal to refill
the buffer when |bytes_left() != 0|.

[xunjieli, 2016/07/20 21:00:47]

Acknowledged.

+  // to refill the buffer when there are undrained bytes.
+  void WasRefilled(size_t filled_bytes);
+  // Marks that the buffer has been drained |drained_bytes| bytes. It is illegal
+  // to drain more bytes than that are availbale.
+  void WasDrained(size_t drained_bytes);
+
+  // Returns the size of the IOBuffer.
+  size_t size() const;
+  // Returns the underlying IOBuffer.
+  IOBuffer* buffer() { return buffer_.get(); }
+  // Returns the pointer to the next byte that may be read or drained.

[Randy Smith (Not in Mondays), 2016/04/26 21:54:01]

nit, suggestion: I'd leave out the "or drained"; in fact, I'd suggest  rewriting
these descriptions to not refer to draining bytes, but instead refer to having
read them and thus updating the bytes() pointer.

[xunjieli, 2016/07/20 21:00:47]

Acknowledged.

+  char* bytes() const { return bytes_; }
+  // Returns the number of bytes that are remaining.

[Randy Smith (Not in Mondays), 2016/04/26 21:54:01]

nit: I'd use the same language as above in the comment to WasDrained; i.e. use
"remaining" there or "available" here.  (I think the words mean the same thing
in this context.)

[xunjieli, 2016/07/20 21:00:47]

Acknowledged.

+  size_t bytes_left() const { return bytes_left_; }
+
+ private:
+  scoped_refptr<IOBuffer> buffer_;
+  // Points to the next byte that may be read or drained.
+  char* bytes_;
+  size_t bytes_left_;
+};

[Randy Smith (Not in Mondays), 2016/04/26 21:54:01]

Should there be a DISALLOW_COPY_AND_ASSIGN?

[xunjieli, 2016/07/20 21:00:47]

Acknowledged.

+
+}  // namespace net
+
+#endif  // NET_FILTER_BLOCK_BUFFER_H