Replace StreamListenSocket with StreamSocket in HttpServer.

net/server/http_connection.cc

Index: net/server/http_connection.cc
diff --git a/net/server/http_connection.cc b/net/server/http_connection.cc
index d433012cd651c7b7462972c3835cb818a9a78d8d..0807326e2bada919db89e0842edd75ed2e8e143c 100644
--- a/net/server/http_connection.cc
+++ b/net/server/http_connection.cc
@@ -4,44 +4,169 @@
 
 #include "net/server/http_connection.h"
 
-#include "net/server/http_server.h"
-#include "net/server/http_server_response_info.h"
 #include "net/server/web_socket.h"
-#include "net/socket/stream_listen_socket.h"
+#include "net/socket/stream_socket.h"
 
 namespace net {
 
-int HttpConnection::last_id_ = 0;
+HttpConnection::ReadIOBuffer::ReadIOBuffer()
+    : base_(new GrowableIOBuffer()),
+      capacity_limit_(kDefaultCapacityLimit) {
+  base_->SetCapacity(kInitialBufSize);
+  data_ = base_->StartOfBuffer();
+}
+
+HttpConnection::ReadIOBuffer::~ReadIOBuffer() {
+  data_ = NULL;  // base_ owns data_.
+}
+
+IOBuffer* HttpConnection::ReadIOBuffer::GetUnusedIOBuffer() const {

[mmenke, 2014/06/11 16:05:49]

I think this is a very confusing name and concept.  My suggestion:  Make
ReadIOBuffer inherit from GrowableIOBuffer, and get rid of this function.

You'd have to rename the current data_ to something else, but I think the usage
pattern would be clearer.  I'd suggest just calling it unconsumed_data_.  Open
to other ideas.

[mmenke, 2014/06/11 16:09:04]

Ignore this suggestion - I think we should think about how to make this simpler,
but I don't think this is the way to go.

[byungchul, 2014/06/11 17:30:28]

Okay, I am re-thinking about how to manage the read buffer. But, please
understand that this depends on how http server and websocket handle packets.
They assume read data buffer is continuous. Unless we change this behavior, it's
hard to split read buffer in multiple segments to reduce data copy.

[mmenke, 2014/06/11 17:53:27]

I'm not concerned with reducing data copy for the read buffer, actually, since
we're just reading HTTP requests.  I just think the class can be a bit hard to
follow.  Thinking more along the lines of improving naming / readability of
ReadIOBuffer, than changing how it basically works.

My move suggestion below was more aimed at limiting needed buffer size, in the
worst case, rather than improving performance.

[byungchul, 2014/08/05 07:06:18]

Changed read buffer similar to GrowableIOBuffer. Couldn't inherit
GrowableIOBuffer unfortunately because it is not allowed due to private
destructor.
Basically, it moves unconsumed data to the start of buffer on DidConsume() call.

+  return base_.get();
+}
+
+int HttpConnection::ReadIOBuffer::GetUnusedCapacity() const {

[mmenke, 2014/06/11 16:05:49]

Suggest keeping this as GetRemainingCapacity, for everyone's sanity.

[byungchul, 2014/08/05 07:06:18]

Done.

+  return base_->RemainingCapacity();
+}
+
+int HttpConnection::ReadIOBuffer::GetCapacity() const {
+  return base_->capacity();
+}
+
+void HttpConnection::ReadIOBuffer::SetCapacity(int capacity) {
+  DCHECK(data_);
+  int consumed_size = static_cast<int>(data_ - base_->StartOfBuffer());
+  base_->SetCapacity(capacity);
+  data_ = base_->StartOfBuffer() + consumed_size;
+}
+
+bool HttpConnection::ReadIOBuffer::IncreaseCapacity() {
+  if (GetCapacity() >= capacity_limit_) {
+    LOG(ERROR) << "Too large read data is pending: capacity=" << GetCapacity()

[mmenke, 2014/06/11 16:05:49]

DLOG, to avoid bloating up binaries.

[byungchul, 2014/08/05 07:06:18]

Http server closes the connection for this error. Isn't LOG(ERROR) appropriate
to show the reason of close?

+               << ", capacity_limit=" << capacity_limit_
+               << ", consumed=" << base_->offset();
+    return false;
+  }
+
+  int new_capacity = GetCapacity() * kCapacityIncreaseFactor;

[mmenke, 2014/06/11 16:05:49]

Seems a little weird to me that we never make an effort to move unhandled
requests back to the start of the buffer.  I don't see this causing us problems
in actual usage, but seems a somewhat weird behavior that we just resize the
buffer instead of moving all unread data to the start of the buffer, when we
can.

[byungchul, 2014/06/11 17:30:28]

Yes, your suggestion would be helpful though this will be called again soon in
practical.

[byungchul, 2014/08/05 07:06:18]

Changed to move data on DidConsume() call.

+  if (new_capacity > capacity_limit_) {
+    new_capacity = capacity_limit_;
+  }
+  SetCapacity(new_capacity);
+  return true;
+}
+
+int HttpConnection::ReadIOBuffer::GetUnconsumedSize() const {
+  DCHECK_GE(base_->data(), data_);
+  return base_->data() - data_;
+}
+
+void HttpConnection::ReadIOBuffer::DidRead(int bytes) {
+  DCHECK_LE(bytes, GetUnusedCapacity());
+  base_->set_offset(base_->offset() + bytes);
+}
 
-void HttpConnection::Send(const std::string& data) {
-  if (!socket_.get())
+void HttpConnection::ReadIOBuffer::DidConsume(int bytes) {
+  DCHECK_LE(bytes, GetUnconsumedSize());
+  if (bytes < GetUnconsumedSize()) {
+    data_ += bytes;
     return;
-  socket_->Send(data);
+  }
+  // Since bytes == GetUnconsumedSize() here, base_->offset() is the size of
+  // consumed data, and all data has been consumed.  No need to keep consumed
+  // data any more because no data will be moved.
+  // If capacity is too big, reduce it.
+  if (GetCapacity() > kMinimumBufSize
+      && GetCapacity() > base_->offset() * kCapacityIncreaseFactor) {
+    int new_capacity = GetCapacity() / kCapacityIncreaseFactor;
+    if (new_capacity < kMinimumBufSize) {
+      new_capacity = kMinimumBufSize;
+    }
+    // realloc() within GrowableIOBuffer::SetCapacity() could move data even
+    // when size is reduced. Free internal data first to guarantee no data move.
+    base_->SetCapacity(0);
+    base_->SetCapacity(new_capacity);
+  } else {
+    base_->set_offset(0);
+  }
+  data_ = base_->StartOfBuffer();
+}
+
+HttpConnection::PendingWriteIOBuffer::PendingWriteIOBuffer()
+    : total_size_(0),
+      total_size_limit_(kDefaultTotalSizeLimit) {
+}
+
+HttpConnection::PendingWriteIOBuffer::~PendingWriteIOBuffer() {
+  data_ = NULL;  // pending_data_ owns data_.
 }
 
-void HttpConnection::Send(const char* bytes, int len) {
-  if (!socket_.get())
+bool HttpConnection::PendingWriteIOBuffer::IsEmpty() const {
+  return pending_data_.empty();
+}
+
+bool HttpConnection::PendingWriteIOBuffer::Append(const std::string& data) {
+  if (data.empty()) {
+    return true;
+  }
+
+  if (total_size_ + static_cast<int>(data.size()) > total_size_limit_) {
+    LOG(ERROR) << "Too large write data is pending: size="
+               << total_size_ + data.size()
+               << ", size_limit=" << total_size_limit_;
+    return false;
+  }
+
+  pending_data_.push(data);
+  total_size_ += data.size();
+
+  // If new data is the first pending data, updates data_.
+  if (pending_data_.size() == 1) {
+    data_ = const_cast<char*>(pending_data_.front().data());
+  }
+  return true;
+}
+
+void HttpConnection::PendingWriteIOBuffer::DidConsume(int size) {
+  DCHECK_LE(size, total_size_);
+  DCHECK_LE(size, GetSizeToWrite());
+  if (size == 0) {
     return;
-  socket_->Send(bytes, len);
+  }
+
+  if (size < GetSizeToWrite()) {
+    data_ += size;
+  } else {  // size == GetSizeToWrite(). Updates data_ to next pending data.
+    pending_data_.pop();
+    data_ = IsEmpty() ? NULL : const_cast<char*>(pending_data_.front().data());
+  }
+  total_size_ -= size;
 }
 
-void HttpConnection::Send(const HttpServerResponseInfo& response) {
-  Send(response.Serialize());
+int HttpConnection::PendingWriteIOBuffer::GetSizeToWrite() const {
+  if (IsEmpty()) {
+    DCHECK_EQ(total_size_, 0);

[mmenke, 2014/06/11 16:05:49]

nit:  Expected goes before actual.

[byungchul, 2014/08/05 07:06:18]

Done.

+    return 0;
+  }
+  DCHECK_GE(data_, pending_data_.front().data());
+  int consumed = static_cast<int>(data_ - pending_data_.front().data());
+  DCHECK_GT(static_cast<int>(pending_data_.front().size()), consumed);
+  return pending_data_.front().size() - consumed;
 }
 
-HttpConnection::HttpConnection(HttpServer* server,
-                               scoped_ptr<StreamListenSocket> sock)
-    : server_(server),
-      socket_(sock.Pass()) {
-  id_ = last_id_++;
+HttpConnection::HttpConnection(int id,
+                               scoped_ptr<StreamSocket> socket)
+    : id_(id),
+      socket_(socket.Pass()),
+      read_buf_(new ReadIOBuffer()),
+      write_buf_(new PendingWriteIOBuffer()) {
 }
 
 HttpConnection::~HttpConnection() {
-  server_->delegate_->OnClose(id_);
 }
 
-void HttpConnection::Shift(int num_bytes) {
-  recv_data_ = recv_data_.substr(num_bytes);
+void HttpConnection::SetWebSocket(scoped_ptr<WebSocket> web_socket) {
+  DCHECK(!web_socket_);
+  web_socket_ = web_socket.Pass();
 }
 
 }  // namespace net