Replace WebSocketFrameChunk with WebSocketFrame

net/websockets/websocket_basic_stream_test.cc

Index: net/websockets/websocket_basic_stream_test.cc
diff --git a/net/websockets/websocket_basic_stream_test.cc b/net/websockets/websocket_basic_stream_test.cc
index ec2e51a6d0ffb7e7a71897b260ad86bee26d1f84..ac2540682b3697f2b6578b9f6c658071ff65b472 100644
--- a/net/websockets/websocket_basic_stream_test.cc
+++ b/net/websockets/websocket_basic_stream_test.cc
@@ -37,31 +37,40 @@ const size_t kMultipleFramesSize = arraysize(kMultipleFrames) - 1;
 // invalid.
 const char kInvalidFrame[] = "\x81\x7E\x00\x07Invalid";
 const size_t kInvalidFrameSize = arraysize(kInvalidFrame) - 1;
+// Control frames must have the FIN bit set. This one does not.
+const char kPingFrameWithoutFin[] = "\x0A\x00";
+const size_t kPingFrameWithoutFinSize = arraysize(kPingFrameWithoutFin) - 1;
+// Control frames must have a payload of 125 bytes or less. This one has
+// a payload of 126 bytes.
+const char k126BytePong[] =
+    "\x89\x7e\x00\x7eZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ"
+    "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ";
+const size_t k126BytePongSize = arraysize(k126BytePong) - 1;
+const char kCloseFrame[] = "\x88\x09\x03\xe8occludo";
+const size_t kCloseFrameSize = arraysize(kCloseFrame) - 1;
 const char kWriteFrame[] = "\x81\x85\x00\x00\x00\x00Write";
 const size_t kWriteFrameSize = arraysize(kWriteFrame) - 1;
 const WebSocketMaskingKey kNulMaskingKey = {{'\0', '\0', '\0', '\0'}};
 
-// Generates a ScopedVector<WebSocketFrameChunk> which will have a wire format
+// Generates a ScopedVector<WebSocketFrame> which will have a wire format
 // matching kWriteFrame.
-ScopedVector<WebSocketFrameChunk> GenerateWriteFrame() {
-  scoped_ptr<WebSocketFrameChunk> chunk(new WebSocketFrameChunk);
+ScopedVector<WebSocketFrame> GenerateWriteFrame() {

[yhirano, 2013/09/13 10:03:17]

Why do you use ScopedVector instead of scoptr_ptr?
GenerateWriteFrame generates a frame, not multiple frames.
Or, should we rename it to GenerateWriteFrames?

[Adam Rice, 2013/09/13 14:09:53]

I realised I was always calling it in the same way, and so I made it part of the
test fixture and renamed it to PrepareWriteFrame().

+  scoped_ptr<WebSocketFrame> frame(
+      new WebSocketFrame(WebSocketFrameHeader::kOpCodeText));
   const size_t payload_size =
       kWriteFrameSize - (WebSocketFrameHeader::kBaseHeaderSize +
                          WebSocketFrameHeader::kMaskingKeyLength);
-  chunk->data = new IOBufferWithSize(payload_size);
-  memcpy(chunk->data->data(),
+  frame->data = new IOBufferWithSize(payload_size);
+  memcpy(frame->data->data(),
          kWriteFrame + kWriteFrameSize - payload_size,
          payload_size);
-  chunk->final_chunk = true;
-  scoped_ptr<WebSocketFrameHeader> header(
-      new WebSocketFrameHeader(WebSocketFrameHeader::kOpCodeText));
-  header->final = true;
-  header->masked = true;
-  header->payload_length = payload_size;
-  chunk->header = header.Pass();
-  ScopedVector<WebSocketFrameChunk> chunks;
-  chunks.push_back(chunk.release());
-  return chunks.Pass();
+  WebSocketFrameHeader& header = frame->header;
+  header.final = true;
+  header.masked = true;
+  header.payload_length = payload_size;
+  ScopedVector<WebSocketFrame> frames;
+  frames.push_back(frame.release());
+  return frames.Pass();
 }
 
 // A masking key generator function which generates the identity mask,
@@ -143,66 +152,112 @@ class WebSocketBasicStreamSocketTest : public WebSocketBasicStreamTest {
   ClientSocketPoolHistograms histograms_;
   MockTransportClientSocketPool pool_;
   CapturingBoundNetLog(bound_net_log_);
-  ScopedVector<WebSocketFrameChunk> frame_chunks_;
+  ScopedVector<WebSocketFrame> frames_;
   TestCompletionCallback cb_;
   scoped_refptr<GrowableIOBuffer> http_read_buffer_;
   std::string sub_protocol_;
   std::string extensions_;
 };
 
+// A test fixture for the common case of tests that only perform a single read.
+class WebSocketBasicStreamSocketSingleReadTest
+    : public WebSocketBasicStreamSocketTest {
+ protected:
+  void CreateRead(const MockRead& read) {
+    reads_[0] = read;
+    CreateStream(reads_, 1U, NULL, 0);
+  }
+
+  MockRead reads_[1];
+};
+
+// A test fixture for tests that perform chunked reads.
+class WebSocketBasicStreamSocketChunkedReadTest
+    : public WebSocketBasicStreamSocketTest {
+ protected:
+  // Specify the behaviour if there aren't enough chunks to use all the data. If
+  // LAST_FRAME_BIG is specified, the the rest of the data will be
+  // put in the last chunk. If LAST_FRAME_NOT_BIG is specified, then the last
+  // frame will be no bigger than the rest of the frames (but it can be smaller,
+  // if not enough data remains).
+  enum LastFrameBehaviour {
+    LAST_FRAME_BIG,
+    LAST_FRAME_NOT_BIG
+  };
+
+  // Prepares a read from |data| of |data_size|, split into |number_of_chunks|,
+  // each of |chunk_size| (except that the last chunk may be larger or
+  // smaller). All reads must be either SYNCHRONOUS or ASYNC (not a mixture),
+  // and errors cannot be simulated. Once data is exhausted, further reads will
+  // return 0 (ie. connection closed).
+  void CreateChunkedRead(IoMode mode,
+                         const char data[],
+                         size_t data_size,
+                         int chunk_size,
+                         int number_of_chunks,
+                         LastFrameBehaviour last_frame_behaviour) {
+    reads_.reset(new MockRead[number_of_chunks]);
+    const char* start = data;
+    for (int i = 0; i < number_of_chunks; ++i) {
+      int len = chunk_size;
+      if (last_frame_behaviour == LAST_FRAME_BIG && i == number_of_chunks - 1 &&
+          data + data_size - start > static_cast<size_t>(len)) {

[yhirano, 2013/09/13 10:03:17]

Is the last condition necessary?

[Adam Rice, 2013/09/13 14:09:53]

No. I have cleaned up this condition a bit.

+        len = static_cast<int>(data + data_size - start);
+      } else if (data + data_size - start < static_cast<size_t>(len)) {
+        len = static_cast<int>(data + data_size - start);
+      }
+      reads_[i] = MockRead(mode, start, len);
+      start += len;
+    }
+    CreateStream(reads_.get(), number_of_chunks, NULL, 0);
+  }
+
+  scoped_ptr<MockRead[]> reads_;
+};
+
 TEST_F(WebSocketBasicStreamSocketTest, ConstructionWorks) {
   CreateNullStream();
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, SyncReadWorks) {
-  MockRead reads[] = {MockRead(SYNCHRONOUS, kSampleFrame, kSampleFrameSize)};
-  CreateReadOnly(reads);
-  int result = stream_->ReadFrames(&frame_chunks_, cb_.callback());
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, SyncReadWorks) {
+  CreateRead(MockRead(SYNCHRONOUS, kSampleFrame, kSampleFrameSize));
+  int result = stream_->ReadFrames(&frames_, cb_.callback());
   EXPECT_EQ(OK, result);
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(GG_UINT64_C(6), frame_chunks_[0]->header->payload_length);
-  EXPECT_TRUE(frame_chunks_[0]->header->final);
-  EXPECT_TRUE(frame_chunks_[0]->final_chunk);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
+  EXPECT_TRUE(frames_[0]->header.final);
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, AsyncReadWorks) {
-  MockRead reads[] = {MockRead(ASYNC, kSampleFrame, kSampleFrameSize)};
-  CreateReadOnly(reads);
-  int result = stream_->ReadFrames(&frame_chunks_, cb_.callback());
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, AsyncReadWorks) {
+  CreateRead(MockRead(ASYNC, kSampleFrame, kSampleFrameSize));
+  int result = stream_->ReadFrames(&frames_, cb_.callback());
   ASSERT_EQ(ERR_IO_PENDING, result);
   EXPECT_EQ(OK, cb_.WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(GG_UINT64_C(6), frame_chunks_[0]->header->payload_length);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
   // Don't repeat all the tests from SyncReadWorks; just enough to be sure the
   // frame was really read.
 }
 
 // ReadFrames will not return a frame whose header has not been wholly received.
-TEST_F(WebSocketBasicStreamSocketTest, HeaderFragmentedSync) {
-  MockRead reads[] = {
-      MockRead(SYNCHRONOUS, kSampleFrame, 1),
-      MockRead(SYNCHRONOUS, kSampleFrame + 1, kSampleFrameSize - 1)};
-  CreateReadOnly(reads);
-  int result = stream_->ReadFrames(&frame_chunks_, cb_.callback());
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, HeaderFragmentedSync) {
+  CreateChunkedRead(
+      SYNCHRONOUS, kSampleFrame, kSampleFrameSize, 1, 2, LAST_FRAME_BIG);
+  int result = stream_->ReadFrames(&frames_, cb_.callback());
   ASSERT_EQ(OK, result);
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(GG_UINT64_C(6), frame_chunks_[0]->header->payload_length);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
 }
 
 // The same behaviour applies to asynchronous reads.
-TEST_F(WebSocketBasicStreamSocketTest, HeaderFragmentedAsync) {
-  MockRead reads[] = {MockRead(ASYNC, kSampleFrame, 1),
-                      MockRead(ASYNC, kSampleFrame + 1, kSampleFrameSize - 1)};
-  CreateReadOnly(reads);
-  int result = stream_->ReadFrames(&frame_chunks_, cb_.callback());
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, HeaderFragmentedAsync) {
+  CreateChunkedRead(
+      ASYNC, kSampleFrame, kSampleFrameSize, 1, 2, LAST_FRAME_BIG);
+  int result = stream_->ReadFrames(&frames_, cb_.callback());
   ASSERT_EQ(ERR_IO_PENDING, result);
   EXPECT_EQ(OK, cb_.WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(GG_UINT64_C(6), frame_chunks_[0]->header->payload_length);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
 }
 
 // If it receives an incomplete header in a synchronous call, then has to wait
@@ -211,12 +266,11 @@ TEST_F(WebSocketBasicStreamSocketTest, HeaderFragmentedSyncAsync) {
   MockRead reads[] = {MockRead(SYNCHRONOUS, kSampleFrame, 1),
                       MockRead(ASYNC, kSampleFrame + 1, kSampleFrameSize - 1)};
   CreateReadOnly(reads);
-  int result = stream_->ReadFrames(&frame_chunks_, cb_.callback());
+  int result = stream_->ReadFrames(&frames_, cb_.callback());
   ASSERT_EQ(ERR_IO_PENDING, result);
   EXPECT_EQ(OK, cb_.WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(GG_UINT64_C(6), frame_chunks_[0]->header->payload_length);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
 }
 
 // An extended header should also return ERR_IO_PENDING if it is not completely
@@ -226,112 +280,118 @@ TEST_F(WebSocketBasicStreamSocketTest, FragmentedLargeHeader) {
       MockRead(SYNCHRONOUS, kPartialLargeFrame, kLargeFrameHeaderSize - 1),
       MockRead(SYNCHRONOUS, ERR_IO_PENDING)};
   CreateReadOnly(reads);
-  EXPECT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  EXPECT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
-// A frame that does not arrive in a single read should arrive in chunks.
-TEST_F(WebSocketBasicStreamSocketTest, LargeFrameFirstChunk) {
-  MockRead reads[] = {
-      MockRead(SYNCHRONOUS, kPartialLargeFrame, kPartialLargeFrameSize)};
-  CreateReadOnly(reads);
-  EXPECT_EQ(OK, stream_->ReadFrames(&frame_chunks_, cb_.callback()));
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(kLargeFrameDeclaredPayloadSize,
-            frame_chunks_[0]->header->payload_length);
-  EXPECT_TRUE(frame_chunks_[0]->header->final);
-  EXPECT_FALSE(frame_chunks_[0]->final_chunk);
+// A frame that does not arrive in a single read should be broken into separate
+// frames.
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, LargeFrameFirstChunk) {
+  CreateRead(MockRead(SYNCHRONOUS, kPartialLargeFrame, kPartialLargeFrameSize));
+  EXPECT_EQ(OK, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_FALSE(frames_[0]->header.final);
   EXPECT_EQ(kPartialLargeFrameSize - kLargeFrameHeaderSize,
-            static_cast<size_t>(frame_chunks_[0]->data->size()));
+            static_cast<size_t>(frames_[0]->header.payload_length));
 }
 
-// If only the header arrives, we should get a zero-byte chunk.
+// If only the header of a data frame arrives, we should not receive a frame and
+// be told to wait. WebSocketBasicStream does two reads in this case, as after
+// the first read it has no frames to return.
 TEST_F(WebSocketBasicStreamSocketTest, HeaderOnlyChunk) {
   MockRead reads[] = {
-      MockRead(SYNCHRONOUS, kPartialLargeFrame, kLargeFrameHeaderSize)};
+      MockRead(SYNCHRONOUS, kPartialLargeFrame, kLargeFrameHeaderSize),
+      MockRead(SYNCHRONOUS, ERR_IO_PENDING)};
   CreateReadOnly(reads);
-  EXPECT_EQ(OK, stream_->ReadFrames(&frame_chunks_, cb_.callback()));
-  ASSERT_EQ(1U, frame_chunks_.size());
-  EXPECT_FALSE(frame_chunks_[0]->final_chunk);
-  EXPECT_TRUE(frame_chunks_[0]->data.get() == NULL);
+  EXPECT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(0U, frames_.size());
 }
 
-// The second and subsequent chunks of a frame have no header.
-TEST_F(WebSocketBasicStreamSocketTest, LargeFrameTwoChunks) {
-  static const size_t kChunkSize = 16;
+// If the header and the body of a data frame arrive seperately, we should only
+// see one frame.
+TEST_F(WebSocketBasicStreamSocketTest, HeaderBodySeparated) {
   MockRead reads[] = {
-      MockRead(ASYNC, kPartialLargeFrame, kChunkSize),
-      MockRead(ASYNC, kPartialLargeFrame + kChunkSize, kChunkSize)};
+      MockRead(SYNCHRONOUS, kPartialLargeFrame, kLargeFrameHeaderSize),
+      MockRead(ASYNC,
+               kPartialLargeFrame + kLargeFrameHeaderSize,
+               kPartialLargeFrameSize - kLargeFrameHeaderSize)};
   CreateReadOnly(reads);
+  EXPECT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_EQ(OK, cb_.WaitForResult());
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(kPartialLargeFrameSize - kLargeFrameHeaderSize,
+            frames_[0]->header.payload_length);
+}
+
+// Every frame has a header with a correct payload_length field.
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, LargeFrameTwoChunks) {
+  const size_t kChunkSize = 16;
+  CreateChunkedRead(ASYNC,
+                    kPartialLargeFrame,
+                    kPartialLargeFrameSize,
+                    kChunkSize,
+                    2,
+                    LAST_FRAME_NOT_BIG);
   TestCompletionCallback cb[2];
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb[0].callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb[0].callback()));
   EXPECT_EQ(OK, cb[0].WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(kChunkSize - kLargeFrameHeaderSize,
+            frames_[0]->header.payload_length);
 
-  frame_chunks_.clear();
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb[1].callback()));
+  frames_.clear();
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb[1].callback()));
   EXPECT_EQ(OK, cb[1].WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_FALSE(frame_chunks_[0]->header);
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(kChunkSize, frames_[0]->header.payload_length);
 }
 
-// Only the final chunk of a frame has final_chunk set.
-TEST_F(WebSocketBasicStreamSocketTest, OnlyFinalChunkIsFinal) {
+// Only the final frame of a fragmented message has |final| bit set.
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, OnlyFinalChunkIsFinal) {
   static const size_t kFirstChunkSize = 4;
-  MockRead reads[] = {MockRead(ASYNC, kSampleFrame, kFirstChunkSize),
-                      MockRead(ASYNC,
-                               kSampleFrame + kFirstChunkSize,
-                               kSampleFrameSize - kFirstChunkSize)};
-  CreateReadOnly(reads);
+  CreateChunkedRead(ASYNC,
+                    kSampleFrame,
+                    kSampleFrameSize,
+                    kFirstChunkSize,
+                    2,
+                    LAST_FRAME_BIG);
   TestCompletionCallback cb[2];
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb[0].callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb[0].callback()));
   EXPECT_EQ(OK, cb[0].WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_FALSE(frame_chunks_[0]->final_chunk);
+  ASSERT_EQ(1U, frames_.size());
+  ASSERT_FALSE(frames_[0]->header.final);
 
-  frame_chunks_.clear();
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb[1].callback()));
+  frames_.clear();
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb[1].callback()));
   EXPECT_EQ(OK, cb[1].WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->final_chunk);
+  ASSERT_EQ(1U, frames_.size());
+  ASSERT_TRUE(frames_[0]->header.final);
 }
 
 // Multiple frames that arrive together should be parsed correctly.
-TEST_F(WebSocketBasicStreamSocketTest, ThreeFramesTogether) {
-  MockRead reads[] = {
-      MockRead(SYNCHRONOUS, kMultipleFrames, kMultipleFramesSize)};
-  CreateReadOnly(reads);
-
-  ASSERT_EQ(OK, stream_->ReadFrames(&frame_chunks_, cb_.callback()));
-  ASSERT_EQ(3U, frame_chunks_.size());
-  EXPECT_TRUE(frame_chunks_[0]->final_chunk);
-  EXPECT_TRUE(frame_chunks_[1]->final_chunk);
-  EXPECT_TRUE(frame_chunks_[2]->final_chunk);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, ThreeFramesTogether) {
+  CreateRead(MockRead(SYNCHRONOUS, kMultipleFrames, kMultipleFramesSize));
+
+  ASSERT_EQ(OK, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(3U, frames_.size());
+  EXPECT_TRUE(frames_[0]->header.final);
+  EXPECT_TRUE(frames_[1]->header.final);
+  EXPECT_TRUE(frames_[2]->header.final);
 }
 
 // ERR_CONNECTION_CLOSED must be returned on close.
-TEST_F(WebSocketBasicStreamSocketTest, SyncClose) {
-  MockRead reads[] = {MockRead(SYNCHRONOUS, "", 0)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, SyncClose) {
+  CreateRead(MockRead(SYNCHRONOUS, "", 0));
 
   EXPECT_EQ(ERR_CONNECTION_CLOSED,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+            stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, AsyncClose) {
-  MockRead reads[] = {MockRead(ASYNC, "", 0)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, AsyncClose) {
+  CreateRead(MockRead(ASYNC, "", 0));
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   EXPECT_EQ(ERR_CONNECTION_CLOSED, cb_.WaitForResult());
 }
 
@@ -339,53 +399,52 @@ TEST_F(WebSocketBasicStreamSocketTest, AsyncClose) {
 // ERR_CONNECTION_CLOSED. This is not expected to happen on an established
 // connection; a Read of size 0 is the expected behaviour. The key point of this
 // test is to confirm that ReadFrames() behaviour is identical in both cases.
-TEST_F(WebSocketBasicStreamSocketTest, SyncCloseWithErr) {
-  MockRead reads[] = {MockRead(SYNCHRONOUS, ERR_CONNECTION_CLOSED)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, SyncCloseWithErr) {
+  CreateRead(MockRead(SYNCHRONOUS, ERR_CONNECTION_CLOSED));
 
   EXPECT_EQ(ERR_CONNECTION_CLOSED,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+            stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, AsyncCloseWithErr) {
-  MockRead reads[] = {MockRead(ASYNC, ERR_CONNECTION_CLOSED)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, AsyncCloseWithErr) {
+  CreateRead(MockRead(ASYNC, ERR_CONNECTION_CLOSED));
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   EXPECT_EQ(ERR_CONNECTION_CLOSED, cb_.WaitForResult());
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, SyncErrorsPassedThrough) {
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, SyncErrorsPassedThrough) {
   // ERR_INSUFFICIENT_RESOURCES here represents an arbitrary error that
   // WebSocketBasicStream gives no special handling to.
-  MockRead reads[] = {MockRead(SYNCHRONOUS, ERR_INSUFFICIENT_RESOURCES)};
-  CreateReadOnly(reads);
+  CreateRead(MockRead(SYNCHRONOUS, ERR_INSUFFICIENT_RESOURCES));
 
   EXPECT_EQ(ERR_INSUFFICIENT_RESOURCES,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+            stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, AsyncErrorsPassedThrough) {
-  MockRead reads[] = {MockRead(ASYNC, ERR_INSUFFICIENT_RESOURCES)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, AsyncErrorsPassedThrough) {
+  CreateRead(MockRead(ASYNC, ERR_INSUFFICIENT_RESOURCES));
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   EXPECT_EQ(ERR_INSUFFICIENT_RESOURCES, cb_.WaitForResult());
 }
 
 // If we get a frame followed by a close, we should receive them separately.
-TEST_F(WebSocketBasicStreamSocketTest, CloseAfterFrame) {
-  MockRead reads[] = {MockRead(SYNCHRONOUS, kSampleFrame, kSampleFrameSize),
-                      MockRead(SYNCHRONOUS, "", 0)};
-  CreateReadOnly(reads);
-
-  EXPECT_EQ(OK, stream_->ReadFrames(&frame_chunks_, cb_.callback()));
-  EXPECT_EQ(1U, frame_chunks_.size());
-  frame_chunks_.clear();
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, CloseAfterFrame) {
+  // The chunk size equals the data size, so the second chunk is 0 size, closing
+  // the connection.
+  CreateChunkedRead(SYNCHRONOUS,
+                    kSampleFrame,
+                    kSampleFrameSize,
+                    kSampleFrameSize,
+                    2,
+                    LAST_FRAME_NOT_BIG);
+
+  EXPECT_EQ(OK, stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_EQ(1U, frames_.size());
+  frames_.clear();
   EXPECT_EQ(ERR_CONNECTION_CLOSED,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+            stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
 // Synchronous close after an async frame header is handled by a different code
@@ -395,8 +454,7 @@ TEST_F(WebSocketBasicStreamSocketTest, AsyncCloseAfterIncompleteHeader) {
                       MockRead(SYNCHRONOUS, "", 0)};
   CreateReadOnly(reads);
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   ASSERT_EQ(ERR_CONNECTION_CLOSED, cb_.WaitForResult());
 }
 
@@ -407,8 +465,7 @@ TEST_F(WebSocketBasicStreamSocketTest, AsyncErrCloseAfterIncompleteHeader) {
                       MockRead(SYNCHRONOUS, ERR_CONNECTION_CLOSED)};
   CreateReadOnly(reads);
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   ASSERT_EQ(ERR_CONNECTION_CLOSED, cb_.WaitForResult());
 }
 
@@ -418,65 +475,140 @@ TEST_F(WebSocketBasicStreamSocketTest, HttpReadBufferIsUsed) {
   SetHttpReadBuffer(kSampleFrame, kSampleFrameSize);
   CreateNullStream();
 
-  EXPECT_EQ(OK, stream_->ReadFrames(&frame_chunks_, cb_.callback()));
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->data);
-  EXPECT_EQ(6, frame_chunks_[0]->data->size());
+  EXPECT_EQ(OK, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(1U, frames_.size());
+  ASSERT_TRUE(frames_[0]->data);
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
 }
 
 // Check that a frame whose header partially arrived at the end of the response
 // headers works correctly.
-TEST_F(WebSocketBasicStreamSocketTest, PartialFrameHeaderInHttpResponse) {
+TEST_F(WebSocketBasicStreamSocketSingleReadTest,
+       PartialFrameHeaderInHttpResponse) {
   SetHttpReadBuffer(kSampleFrame, 1);
-  MockRead reads[] = {MockRead(ASYNC, kSampleFrame + 1, kSampleFrameSize - 1)};
-  CreateReadOnly(reads);
+  CreateRead(MockRead(ASYNC, kSampleFrame + 1, kSampleFrameSize - 1));
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   EXPECT_EQ(OK, cb_.WaitForResult());
-  ASSERT_EQ(1U, frame_chunks_.size());
-  ASSERT_TRUE(frame_chunks_[0]->data);
-  EXPECT_EQ(6, frame_chunks_[0]->data->size());
-  ASSERT_TRUE(frame_chunks_[0]->header);
-  EXPECT_EQ(WebSocketFrameHeader::kOpCodeText,
-            frame_chunks_[0]->header->opcode);
+  ASSERT_EQ(1U, frames_.size());
+  ASSERT_TRUE(frames_[0]->data);
+  EXPECT_EQ(GG_UINT64_C(6), frames_[0]->header.payload_length);
+  EXPECT_EQ(WebSocketFrameHeader::kOpCodeText, frames_[0]->header.opcode);
 }
 
 // Check that an invalid frame results in an error.
-TEST_F(WebSocketBasicStreamSocketTest, SyncInvalidFrame) {
-  MockRead reads[] = {MockRead(SYNCHRONOUS, kInvalidFrame, kInvalidFrameSize)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, SyncInvalidFrame) {
+  CreateRead(MockRead(SYNCHRONOUS, kInvalidFrame, kInvalidFrameSize));
 
   EXPECT_EQ(ERR_WS_PROTOCOL_ERROR,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+            stream_->ReadFrames(&frames_, cb_.callback()));
 }
 
-TEST_F(WebSocketBasicStreamSocketTest, AsyncInvalidFrame) {
-  MockRead reads[] = {MockRead(ASYNC, kInvalidFrame, kInvalidFrameSize)};
-  CreateReadOnly(reads);
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, AsyncInvalidFrame) {
+  CreateRead(MockRead(ASYNC, kInvalidFrame, kInvalidFrameSize));
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->ReadFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
   EXPECT_EQ(ERR_WS_PROTOCOL_ERROR, cb_.WaitForResult());
 }
 
+// A control frame without a FIN flag is invalid and should not be passed
+// through to higher layers. RFC6455 5.5 "All control frames ... MUST NOT be
+// fragmented."
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, ControlFrameWithoutFin) {
+  CreateRead(
+      MockRead(SYNCHRONOUS, kPingFrameWithoutFin, kPingFrameWithoutFinSize));
+
+  ASSERT_EQ(ERR_WS_PROTOCOL_ERROR,
+            stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_TRUE(frames_.empty());
+}
+
+// A control frame over 125 characters is invalid. RFC6455 5.5 "All control
+// frames MUST have a payload length of 125 bytes or less". Since we use a
+// 125-byte buffer to assemble fragmented control frames, we need to detect this
+// error before attempting to assemble the fragments.
+TEST_F(WebSocketBasicStreamSocketSingleReadTest, OverlongControlFrame) {
+  CreateRead(MockRead(SYNCHRONOUS, k126BytePong, k126BytePongSize));
+
+  EXPECT_EQ(ERR_WS_PROTOCOL_ERROR,
+            stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_TRUE(frames_.empty());
+}
+
+// A control frame over 125 characters should still be rejected if it is split
+// into multiple chunks.
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, SplitOverlongControlFrame) {
+  const size_t kFirstChunkSize = 16;
+  CreateChunkedRead(SYNCHRONOUS,
+                    k126BytePong,
+                    k126BytePongSize,
+                    kFirstChunkSize,
+                    2,
+                    LAST_FRAME_BIG);
+
+  EXPECT_EQ(ERR_WS_PROTOCOL_ERROR,
+            stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_TRUE(frames_.empty());
+}
+
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest,
+       AsyncSplitOverlongControlFrame) {
+  const size_t kFirstChunkSize = 16;
+  CreateChunkedRead(ASYNC,
+                    k126BytePong,
+                    k126BytePongSize,
+                    kFirstChunkSize,
+                    2,
+                    LAST_FRAME_BIG);
+
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
+  EXPECT_EQ(ERR_WS_PROTOCOL_ERROR, cb_.WaitForResult());
+  // The caller should not call ReadFrames() again after receiving an error
+  // other than ERR_IO_PENDING.
+  EXPECT_TRUE(frames_.empty());
+}
+
+// In the synchronous case, ReadFrames assembles the whole control frame before
+// returning.
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, SyncControlFrameAssembly) {
+  const size_t kChunkSize = 3;
+  CreateChunkedRead(
+      SYNCHRONOUS, kCloseFrame, kCloseFrameSize, kChunkSize, 3, LAST_FRAME_BIG);
+
+  ASSERT_EQ(OK, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(WebSocketFrameHeader::kOpCodeClose, frames_[0]->header.opcode);
+}
+
+// In the asynchronous case, the callback is not called until the control frame
+// has been completely assembled.
+TEST_F(WebSocketBasicStreamSocketChunkedReadTest, AsyncControlFrameAssembly) {
+  const size_t kChunkSize = 3;
+  CreateChunkedRead(
+      ASYNC, kCloseFrame, kCloseFrameSize, kChunkSize, 3, LAST_FRAME_BIG);
+
+  ASSERT_EQ(ERR_IO_PENDING, stream_->ReadFrames(&frames_, cb_.callback()));
+  ASSERT_EQ(OK, cb_.WaitForResult());
+  ASSERT_EQ(1U, frames_.size());
+  EXPECT_EQ(WebSocketFrameHeader::kOpCodeClose, frames_[0]->header.opcode);
+}
+
 // Check that writing a frame all at once works.
 TEST_F(WebSocketBasicStreamSocketTest, WriteAtOnce) {
   MockWrite writes[] = {MockWrite(SYNCHRONOUS, kWriteFrame, kWriteFrameSize)};
   CreateWriteOnly(writes);
-  frame_chunks_ = GenerateWriteFrame();
+  frames_ = GenerateWriteFrame();
 
-  EXPECT_EQ(OK, stream_->WriteFrames(&frame_chunks_, cb_.callback()));
+  EXPECT_EQ(OK, stream_->WriteFrames(&frames_, cb_.callback()));
 }
 
 // Check that completely async writing works.
 TEST_F(WebSocketBasicStreamSocketTest, AsyncWriteAtOnce) {
   MockWrite writes[] = {MockWrite(ASYNC, kWriteFrame, kWriteFrameSize)};
   CreateWriteOnly(writes);
-  frame_chunks_ = GenerateWriteFrame();
+  frames_ = GenerateWriteFrame();
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->WriteFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->WriteFrames(&frames_, cb_.callback()));
   EXPECT_EQ(OK, cb_.WaitForResult());
 }
 
@@ -488,10 +620,9 @@ TEST_F(WebSocketBasicStreamSocketTest, WriteInBits) {
                         MockWrite(ASYNC, kWriteFrame + 4, 4),
                         MockWrite(ASYNC, kWriteFrame + 8, kWriteFrameSize - 8)};
   CreateWriteOnly(writes);
-  frame_chunks_ = GenerateWriteFrame();
+  frames_ = GenerateWriteFrame();
 
-  ASSERT_EQ(ERR_IO_PENDING,
-            stream_->WriteFrames(&frame_chunks_, cb_.callback()));
+  ASSERT_EQ(ERR_IO_PENDING, stream_->WriteFrames(&frames_, cb_.callback()));
   EXPECT_EQ(OK, cb_.WaitForResult());
 }