Update the FLIP session to use the FlipIOBuffer.

Also removed some of the testing hacks for URL rewriting
that are no longer needed.

This change removes the 'batching' of frames written
to the socket.  The reason for doing this is because
I'm going to need to start notifying to the upper layer
as progress is made on the writes (e.g. upload notifications).
When the frames are batched (potentially from different 
transactions), it becomes very difficult to know when each
write completes.  I don't think that batching is necessary, 
as writes will accummulate in the socket buffer, so this
should be a better approach.

BUG=none
TEST=flip_session_unittest.cc

Review URL: http://codereview.chromium.org/342088

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30898 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 49 insertions, 121 deletions

diff --git a/net/flip/flip_network_transaction.cc b/net/flip/flip_network_transaction.cc
index 77059fa..f0433ab 100644
--- a/net/flip/flip_network_transaction.cc
+++ b/net/flip/flip_network_transaction.cc
@@ -302,26 +302,8 @@ int FlipNetworkTransaction::DoInitConnection() {
 
   HostResolver::RequestInfo resolve_info(host, port);
 
-// TODO(mbelshe): Cleanup these testing tricks.
-// If we want to use multiple connections, grab the flip session
-// up front using the original domain name.
-#undef USE_MULTIPLE_CONNECTIONS
-#undef DIVERT_URLS_TO_TEST_SERVER
-#if defined(USE_MULTIPLE_CONNECTIONS) || !defined(DIVERT_URLS_TO_TEST_SERVER)
   flip_ = FlipSession::GetFlipSession(resolve_info, session_);
-#endif
-
-// Use this to divert URLs to a test server.
-#ifdef DIVERT_URLS_TO_TEST_SERVER
-  // Fake out this session to go to our test server.
-  host = "servername";
-  port = 443;
-  resolve_info = HostResolver::RequestInfo(host, port);
-#ifndef USE_MULTIPLE_CONNECTIONS
-  flip_ = FlipSession::GetFlipSession(resolve_info, session_);
-#endif  // USE_MULTIPLE_CONNECTIONS
-
-#endif  // DIVERT_URLS_TO_TEST_SERVER
+  DCHECK(flip_);
 
   int rv = flip_->Connect(connection_group, resolve_info, request_->priority);
   DCHECK(rv == net::OK);  // The API says it will always return OK.
diff --git a/net/flip/flip_session.cc b/net/flip/flip_session.cc
index 962124a..cb0da56 100644
--- a/net/flip/flip_session.cc
+++ b/net/flip/flip_session.cc
@@ -28,7 +28,6 @@ namespace net {
 // static
 scoped_ptr<FlipSessionPool> FlipSession::session_pool_;
 bool FlipSession::use_ssl_ = true;
-int PrioritizedIOBuffer::order_ = 0;
 
 FlipSession* FlipSession::GetFlipSession(
     const HostResolver::RequestInfo& info,
@@ -114,37 +113,8 @@ void CreateFlipHeadersFromHttpRequest(
     (*headers)[name] = it.values();
   }
 
-#undef DIVERT_URLS_TO_TEST_SERVER
-#ifdef DIVERT_URLS_TO_TEST_SERVER
-  // TODO(mbelshe): Figure out how to remove this.  This is just for hooking
-  // up to a test server.
-  // For testing content on our test server, we modify the URL.
-  GURL url = info->url;
-  FilePath path(UrlToFilenameEncoder::Encode(url.spec(), FilePath()));
-
-  // We do the unusual conversion from a FilePath::StringType to
-  // an ascii string.  Recognize that StringType is a wstring on windows,
-  // so a failure is technically possible, but this is just used as a test
-  // case, so it's okay.  This code will be deleted.
-  std::string hack_url("/");
-#if defined(OS_WIN)
-  hack_url.append(WideToASCII(path.value()));
-#else
-  hack_url.append(path.value());
-#endif
-
-  // switch backslashes.  HACK
-  std::string::size_type pos(0);
-  while ((pos = hack_url.find('\\', pos)) != std::string::npos) {
-    hack_url.replace(pos, 1, "/");
-    pos += 1;
-  }
-#else
-  std::string hack_url(info->url.PathForRequest());
-#endif  // REWRITE_URLS
-
   (*headers)["method"] = info->method;
-  (*headers)["url"] = hack_url;
+  (*headers)["url"] = info->url.PathForRequest();
   (*headers)["version"] = kHttpProtocolVersion;
   (*headers)["host"] = GetHostAndOptionalPort(info->url);
   if (info->user_agent.length())
@@ -234,7 +204,7 @@ int FlipSession::CreateStream(FlipDelegate* delegate) {
       new IOBufferWithSize(length);
   memcpy(buffer->data(), syn_frame, length);
   delete[] syn_frame;
-  queue_.push(PrioritizedIOBuffer(buffer, priority));
+  queue_.push(FlipIOBuffer(buffer, priority, stream));
 
   static StatsCounter flip_requests("flip.requests");
   flip_requests.Increment();
@@ -377,6 +347,10 @@ void FlipSession::OnWriteComplete(int result) {
   LOG(INFO) << "Flip write complete (result=" << result << ")";
 
   if (result >= 0) {
+    // TODO(mbelshe) Verify that we wrote ALL the bytes we needed to.
+    //               The code current is broken in the case of a partial write.
+    DCHECK_EQ(static_cast<size_t>(result), in_flight_write_.size());
+
     // Cleanup the write which just completed.
     in_flight_write_.release();
 
@@ -437,40 +411,42 @@ void FlipSession::WriteSocket() {
   if (write_pending_)   // Another write is in progress still.
     return;
 
+  // Loop sending frames until we've sent everything or until the write
+  // returns error (or ERR_IO_PENDING).
   while (queue_.size()) {
-    const size_t kMaxSegmentSize = 1430;
-    const size_t kMaxPayload = 4 * kMaxSegmentSize;
-    size_t max_size = std::max(kMaxPayload, queue_.top().size());
-
-    size_t bytes = 0;
-    // If we have multiple IOs to do, accumulate up to 4 MSS's worth of data
-    // and send them in batch.
-    IOBufferWithSize* buffer = new IOBufferWithSize(max_size);
-    while (queue_.size() && bytes < max_size) {
-      PrioritizedIOBuffer next_buffer = queue_.top();
-
-      // Now that we're outputting the frame, we can finally compress it.
-      flip::FlipFrame* uncompressed_frame =
-          reinterpret_cast<flip::FlipFrame*>(next_buffer.buffer()->data());
-      scoped_array<flip::FlipFrame> compressed_frame(
-          flip_framer_.CompressFrame(uncompressed_frame));
-      size_t size = compressed_frame.get()->length() + sizeof(flip::FlipFrame);
-      if (bytes + size > kMaxPayload)
-        break;
-      memcpy(buffer->data() + bytes, compressed_frame.get(), size);
-      bytes += size;
-      queue_.pop();
-      next_buffer.release();
-    }
-    DCHECK(bytes > 0);
-    in_flight_write_ = PrioritizedIOBuffer(buffer, 0);
+    // Grab the next FlipFrame to send.
+    FlipIOBuffer next_buffer = queue_.top();
+    queue_.pop();
+
+    // We've deferred compression until just before we write it to the socket,
+    // which is now.
+    flip::FlipFrame* uncompressed_frame =
+        reinterpret_cast<flip::FlipFrame*>(next_buffer.buffer()->data());
+    scoped_array<flip::FlipFrame> compressed_frame(
+        flip_framer_.CompressFrame(uncompressed_frame));
+    size_t size = compressed_frame.get()->length() + sizeof(flip::FlipFrame);
+
+    DCHECK(size > 0);
+
+    // TODO(mbelshe): We have too much copying of data here.
+    IOBufferWithSize* buffer = new IOBufferWithSize(size);
+    memcpy(buffer->data(), compressed_frame.get(), size);
+
+    // Attempt to send the frame.
+    in_flight_write_ = FlipIOBuffer(buffer, 0, next_buffer.stream());
     write_pending_ = true;
-
     int rv = connection_.socket()->Write(in_flight_write_.buffer(),
-                                         bytes, &write_callback_);
+                                         size, &write_callback_);
     if (rv == net::ERR_IO_PENDING)
       break;
+
+    // We sent the frame successfully.
     OnWriteComplete(rv);
+
+    // TODO(mbelshe):  Test this error case.  Maybe we should mark the socket
+    //                 as in an error state.
+    if (rv < 0)
+      break;
   }
 }
 
diff --git a/net/flip/flip_session.h b/net/flip/flip_session.h
index 22cc578..ea895da 100644
--- a/net/flip/flip_session.h
+++ b/net/flip/flip_session.h
@@ -18,6 +18,7 @@
 #include "net/base/ssl_config_service.h"
 #include "net/base/upload_data_stream.h"
 #include "net/flip/flip_framer.h"
+#include "net/flip/flip_io_buffer.h"
 #include "net/flip/flip_protocol.h"
 #include "net/flip/flip_session_pool.h"
 #include "net/socket/client_socket.h"
@@ -86,37 +87,6 @@ class FlipDelegate {
   virtual void OnClose(int status) = 0;
 };
 
-class PrioritizedIOBuffer {
- public:
-  PrioritizedIOBuffer() : buffer_(0), priority_(0) {}
-  PrioritizedIOBuffer(IOBufferWithSize* buffer, int priority)
-      : buffer_(buffer),
-        priority_(priority),
-        position_(++order_) {
-  }
-
-  IOBuffer* buffer() const { return buffer_; }
-
-  size_t size() const { return buffer_->size(); }
-
-  void release() { buffer_ = NULL; }
-
-  int priority() { return priority_; }
-
-  // Supports sorting.
-  bool operator<(const PrioritizedIOBuffer& other) const {
-    if (priority_ != other.priority_)
-      return priority_ > other.priority_;
-    return position_ >= other.position_;
-  }
-
- private:
-  scoped_refptr<IOBufferWithSize> buffer_;
-  int priority_;
-  int position_;
-  static int order_;  // Maintains a FIFO order for equal priorities.
-};
-
 class FlipSession : public base::RefCounted<FlipSession>,
                     public flip::FlipFramerVisitorInterface {
  public:
@@ -249,12 +219,12 @@ class FlipSession : public base::RefCounted<FlipSession>,
   std::map<std::string, FlipDelegate*> pending_streams_;
 
   // As we gather data to be sent, we put it into the output queue.
-  typedef std::priority_queue<PrioritizedIOBuffer> OutputQueue;
+  typedef std::priority_queue<FlipIOBuffer> OutputQueue;
   OutputQueue queue_;
 
   // TODO(mbelshe): this is ugly!!
   // The packet we are currently sending.
-  PrioritizedIOBuffer in_flight_write_;
+  FlipIOBuffer in_flight_write_;
   bool delayed_write_pending_;
   bool write_pending_;
 
diff --git a/net/flip/flip_session_unittest.cc b/net/flip/flip_session_unittest.cc
index 90bf938..c27f478 100644
--- a/net/flip/flip_session_unittest.cc
+++ b/net/flip/flip_session_unittest.cc
@@ -3,25 +3,25 @@
 // found in the LICENSE file.
 
 #include "net/base/test_completion_callback.h"
+#include "net/flip/flip_io_buffer.h"
+#include "net/flip/flip_session.h"
 #include "net/socket/socket_test_util.h"
 #include "testing/platform_test.h"
 
-#include "net/flip/flip_session.h"
-
 namespace net {
 
 class FlipSessionTest : public PlatformTest {
  public:
 };
 
-// Test the PrioritizedIOBuffer class.
-TEST_F(FlipSessionTest, PrioritizedIOBuffer) {
-  std::priority_queue<PrioritizedIOBuffer> queue_;
+// Test the FlipIOBuffer class.
+TEST_F(FlipSessionTest, FlipIOBuffer) {
+  std::priority_queue<FlipIOBuffer> queue_;
   const size_t kQueueSize = 100;
 
   // Insert 100 items; pri 100 to 1.
   for (size_t index = 0; index < kQueueSize; ++index) {
-    PrioritizedIOBuffer buffer(NULL, kQueueSize - index);
+    FlipIOBuffer buffer(NULL, kQueueSize - index, NULL);
     queue_.push(buffer);
   }
 
@@ -30,14 +30,14 @@ TEST_F(FlipSessionTest, PrioritizedIOBuffer) {
   IOBufferWithSize* buffers[kNumDuplicates];
   for (size_t index = 0; index < kNumDuplicates; ++index) {
     buffers[index] = new IOBufferWithSize(index+1);
-    queue_.push(PrioritizedIOBuffer(buffers[index], 0));
+    queue_.push(FlipIOBuffer(buffers[index], 0, NULL));
   }
 
   EXPECT_EQ(kQueueSize + kNumDuplicates, queue_.size());
 
   // Verify the P0 items come out in FIFO order.
   for (size_t index = 0; index < kNumDuplicates; ++index) {
-    PrioritizedIOBuffer buffer = queue_.top();
+    FlipIOBuffer buffer = queue_.top();
     EXPECT_EQ(0, buffer.priority());
     EXPECT_EQ(index + 1, buffer.size());
     queue_.pop();
@@ -45,7 +45,7 @@ TEST_F(FlipSessionTest, PrioritizedIOBuffer) {
 
   int priority = 1;
   while (queue_.size()) {
-    PrioritizedIOBuffer buffer = queue_.top();
+    FlipIOBuffer buffer = queue_.top();
     EXPECT_EQ(priority++, buffer.priority());
     queue_.pop();
   }