Rework the FlipProtocol to separate the C-like structs from the

methods.  Without this refactoring, we didn't have a clean way
to allocate and deallocate FlipFrames.  Now we can use the
scoped_ptr cleanly.

Summary of misc changes:
  * Merged in some small changes from the GFE side.
  * flip_protocol.h Changes substantially.  We now have separate
    structs and classes.  No longer can you cast from one frame
    type to another.  All FlipFrame classes derive from FlipFrame.
    A FlipFrame owns a buffer which is used for the frame, and
    when you create the Frame, you can specify whether the FlipFrame
    will self-clean its buffer or not.  This makes it cheap to
    instantiate a FlipFrame class on the stack and use it temporarily
    for accessing fields without having to do any copies or allocations.
  * You can't use sizeof(FlipFrame) anymore - that is now a class.
    Added a static "size()" method to each FlipFrame type for
    declaring its real size.
  * Refactored a couple of routines in flip_framer.  These were
    previously in a huge state machine (ProcessInput), just copied
    the code into subroutines.
  * Added flip_protocol_test to the mix from the gfe side.  Much of
    this is a refactoring from flip_framer_test.
  * Eliminated reinterpret_casts as much as I could and got rid of
    all uses of scoped_array for FlipFrames.


BUG=none
TEST=all flip tests reworked


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

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

8 files changed, 652 insertions, 360 deletions

diff --git a/net/flip/flip_frame_builder.h b/net/flip/flip_frame_builder.h
index fa59c45..32b6035 100644
--- a/net/flip/flip_frame_builder.h
+++ b/net/flip/flip_frame_builder.h
@@ -43,14 +43,9 @@ class FlipFrameBuilder {
   // Returns the size of the FlipFrameBuilder's data.
   int length() const { return length_; }
 
-  // Returns the data for this FlipFrameBuilder.
-  const FlipFrame* data() const {
-    return reinterpret_cast<FlipFrame*>(buffer_);
-  }
-
   // Takes the buffer from the FlipFrameBuilder.
   FlipFrame* take() {
-    FlipFrame* rv = reinterpret_cast<FlipFrame*>(buffer_);
+    FlipFrame* rv = new FlipFrame(buffer_, true);
     buffer_ = NULL;
     capacity_ = 0;
     length_ = 0;
diff --git a/net/flip/flip_framer.cc b/net/flip/flip_framer.cc
index c0136d0..f60752c 100644
--- a/net/flip/flip_framer.cc
+++ b/net/flip/flip_framer.cc
@@ -105,8 +105,8 @@ size_t FlipFramer::BytesSafeToRead() const {
     case FLIP_RESET:
       return 0;
     case FLIP_READING_COMMON_HEADER:
-      DCHECK(current_frame_len_ < sizeof(FlipFrame));
-      return sizeof(FlipFrame) - current_frame_len_;
+      DCHECK(current_frame_len_ < FlipFrame::size());
+      return FlipFrame::size() - current_frame_len_;
     case FLIP_INTERPRET_CONTROL_FRAME_COMMON_HEADER:
       return 0;
     case FLIP_CONTROL_FRAME_PAYLOAD:
@@ -151,11 +151,6 @@ size_t FlipFramer::ProcessInput(const char* data, size_t len) {
 
   size_t original_len = len;
   while (len != 0) {
-    FlipControlFrame* current_control_frame =
-        reinterpret_cast<FlipControlFrame*>(current_frame_buffer_);
-    FlipDataFrame* current_data_frame =
-        reinterpret_cast<FlipDataFrame*>(current_frame_buffer_);
-
     switch (state_) {
       case FLIP_ERROR:
       case FLIP_DONE:
@@ -178,52 +173,7 @@ size_t FlipFramer::ProcessInput(const char* data, size_t len) {
       // I felt it was a nice partitioning, however (which probably indicates
       // that it should be refactored into its own function!)
       case FLIP_INTERPRET_CONTROL_FRAME_COMMON_HEADER:
-        DCHECK(error_code_ == 0);
-        DCHECK(current_frame_len_ >= sizeof(FlipFrame));
-        // Do some sanity checking on the control frame sizes.
-        switch (current_control_frame->type()) {
-          case SYN_STREAM:
-            // NOTE: sizeof(FlipSynStreamControlFrame) is not accurate.
-            if (current_control_frame->length() <
-                sizeof(FlipSynStreamControlFrame) - sizeof(FlipControlFrame))
-              set_error(FLIP_INVALID_CONTROL_FRAME);
-            break;
-          case SYN_REPLY:
-            if (current_control_frame->length() <
-                sizeof(FlipSynReplyControlFrame) - sizeof(FlipControlFrame))
-              set_error(FLIP_INVALID_CONTROL_FRAME);
-            break;
-          case FIN_STREAM:
-            if (current_control_frame->length() !=
-                sizeof(FlipFinStreamControlFrame) - sizeof(FlipFrame))
-              set_error(FLIP_INVALID_CONTROL_FRAME);
-            break;
-          case NOOP:
-            // NOP.  Swallow it.
-            CHANGE_STATE(FLIP_AUTO_RESET);
-            continue;
-          default:
-            set_error(FLIP_UNKNOWN_CONTROL_TYPE);
-            break;
-        }
-
-        // We only support version 1 of this protocol.
-        if (current_control_frame->version() != kFlipProtocolVersion)
-          set_error(FLIP_UNSUPPORTED_VERSION);
-
-        if (error_code_) {
-          CHANGE_STATE(FLIP_ERROR);
-          goto bottom;
-        }
-
-        remaining_control_payload_ = current_control_frame->length();
-        if (remaining_control_payload_ > kControlFrameBufferMaxSize) {
-          set_error(FLIP_CONTROL_PAYLOAD_TOO_LARGE);
-          CHANGE_STATE(FLIP_ERROR);
-          goto bottom;
-        }
-        ExpandControlFrameBuffer(remaining_control_payload_);
-        CHANGE_STATE(FLIP_CONTROL_FRAME_PAYLOAD);
+        ProcessControlFrameHeader();
         continue;
 
       case FLIP_CONTROL_FRAME_PAYLOAD: {
@@ -235,60 +185,12 @@ size_t FlipFramer::ProcessInput(const char* data, size_t len) {
       case FLIP_IGNORE_REMAINING_PAYLOAD:
         // control frame has too-large payload
         // intentional fallthrough
-      case FLIP_FORWARD_STREAM_FRAME:
-        if (remaining_payload_) {
-          size_t amount_to_forward = std::min(remaining_payload_, len);
-          if (amount_to_forward && state_ != FLIP_IGNORE_REMAINING_PAYLOAD) {
-            const FlipDataFrame* data_frame =
-                reinterpret_cast<const FlipDataFrame*>(current_data_frame);
-            if (data_frame->flags() & DATA_FLAG_COMPRESSED) {
-              // TODO(mbelshe): Assert that the decompressor is init'ed.
-              if (!InitializeDecompressor())
-                return NULL;
-
-              size_t decompressed_max_size = amount_to_forward * 100;
-              scoped_array<char> decompressed(new char[decompressed_max_size]);
-              decompressor_->next_in = reinterpret_cast<Bytef*>(
-                  const_cast<char*>(data));
-              decompressor_->avail_in = amount_to_forward;
-              decompressor_->next_out =
-                  reinterpret_cast<Bytef*>(decompressed.get());
-              decompressor_->avail_out = decompressed_max_size;
-
-              int rv = inflate(decompressor_.get(), Z_SYNC_FLUSH);
-              if (rv != Z_OK) {
-                set_error(FLIP_DECOMPRESS_FAILURE);
-                goto bottom;
-              }
-              size_t decompressed_size = decompressed_max_size -
-                                      decompressor_->avail_out;
-              // Only inform the visitor if there is data.
-              if (decompressed_size)
-                visitor_->OnStreamFrameData(current_data_frame->stream_id(),
-                                            decompressed.get(),
-                                            decompressed_size);
-              amount_to_forward -= decompressor_->avail_in;
-            } else {
-              // The data frame was not compressed.
-              // Only inform the visitor if there is data.
-              if (amount_to_forward)
-                visitor_->OnStreamFrameData(current_data_frame->stream_id(),
-                                            data, amount_to_forward);
-            }
-          }
-          data += amount_to_forward;
-          len -= amount_to_forward;
-          remaining_payload_ -= amount_to_forward;
-          // If the FIN flag is set, and there is no more data in this data
-          // frame, inform the visitor of EOF via a 0-length data frame.
-          if (!remaining_payload_ &&
-              current_data_frame->flags() & DATA_FLAG_FIN)
-            visitor_->OnStreamFrameData(current_data_frame->stream_id(), NULL,
-                                       0);
-        } else {
-          CHANGE_STATE(FLIP_AUTO_RESET);
-        }
+      case FLIP_FORWARD_STREAM_FRAME: {
+        int bytes_read = ProcessDataFramePayload(data, len);
+        len -= bytes_read;
+        data += bytes_read;
         continue;
+      }
       default:
         break;
     }
@@ -303,12 +205,11 @@ size_t FlipFramer::ProcessCommonHeader(const char* data, size_t len) {
   DCHECK(state_ == FLIP_READING_COMMON_HEADER);
 
   int original_len = len;
-  FlipDataFrame* current_frame =
-      reinterpret_cast<FlipDataFrame*>(current_frame_buffer_);
+  FlipFrame current_frame(current_frame_buffer_, false);
 
   do {
-    if (current_frame_len_ < sizeof(FlipFrame)) {
-      size_t bytes_desired = sizeof(FlipFrame) - current_frame_len_;
+    if (current_frame_len_ < FlipFrame::size()) {
+      size_t bytes_desired = FlipFrame::size() - current_frame_len_;
       size_t bytes_to_append = std::min(bytes_desired, len);
       char* header_buffer = current_frame_buffer_;
       memcpy(&header_buffer[current_frame_len_], data, bytes_to_append);
@@ -316,18 +217,20 @@ size_t FlipFramer::ProcessCommonHeader(const char* data, size_t len) {
       data += bytes_to_append;
       len -= bytes_to_append;
       // Check for an empty data frame.
-      if (current_frame_len_ == sizeof(FlipFrame) &&
-          !current_frame->is_control_frame() &&
-          current_frame->length() == 0) {
-        if (current_frame->flags() & CONTROL_FLAG_FIN)
-          visitor_->OnStreamFrameData(current_frame->stream_id(), NULL, 0);
+      if (current_frame_len_ == FlipFrame::size() &&
+          !current_frame.is_control_frame() &&
+          current_frame.length() == 0) {
+        if (current_frame.flags() & CONTROL_FLAG_FIN) {
+          FlipDataFrame data_frame(current_frame_buffer_, false);
+          visitor_->OnStreamFrameData(data_frame.stream_id(), NULL, 0);
+        }
         CHANGE_STATE(FLIP_RESET);
       }
       break;
     }
-    remaining_payload_ = current_frame->length();
+    remaining_payload_ = current_frame.length();
     // if we're here, then we have the common header all received.
-    if (!current_frame->is_control_frame())
+    if (!current_frame.is_control_frame())
       CHANGE_STATE(FLIP_FORWARD_STREAM_FRAME);
     else
       CHANGE_STATE(FLIP_INTERPRET_CONTROL_FRAME_COMMON_HEADER);
@@ -336,6 +239,55 @@ size_t FlipFramer::ProcessCommonHeader(const char* data, size_t len) {
   return original_len - len;
 }
 
+void FlipFramer::ProcessControlFrameHeader() {
+  FlipControlFrame current_control_frame(current_frame_buffer_, false);
+  DCHECK_EQ(FLIP_NO_ERROR, error_code_);
+  DCHECK_LE(FlipFrame::size(), current_frame_len_);
+  // Do some sanity checking on the control frame sizes.
+  switch (current_control_frame.type()) {
+    case SYN_STREAM:
+      if (current_control_frame.length() <
+          FlipSynStreamControlFrame::size() - FlipControlFrame::size())
+        set_error(FLIP_INVALID_CONTROL_FRAME);
+      break;
+    case SYN_REPLY:
+      if (current_control_frame.length() <
+          FlipSynReplyControlFrame::size() - FlipControlFrame::size())
+        set_error(FLIP_INVALID_CONTROL_FRAME);
+      break;
+    case FIN_STREAM:
+      if (current_control_frame.length() !=
+          FlipFinStreamControlFrame::size() - FlipFrame::size())
+        set_error(FLIP_INVALID_CONTROL_FRAME);
+      break;
+    case NOOP:
+      // NOP.  Swallow it.
+      CHANGE_STATE(FLIP_AUTO_RESET);
+      return;
+    default:
+      set_error(FLIP_UNKNOWN_CONTROL_TYPE);
+      break;
+  }
+
+  // We only support version 1 of this protocol.
+  if (current_control_frame.version() != kFlipProtocolVersion)
+    set_error(FLIP_UNSUPPORTED_VERSION);
+
+  if (error_code_) {
+    CHANGE_STATE(FLIP_ERROR);
+    return;
+  }
+
+  remaining_control_payload_ = current_control_frame.length();
+  if (remaining_control_payload_ > kControlFrameBufferMaxSize) {
+    set_error(FLIP_CONTROL_PAYLOAD_TOO_LARGE);
+    CHANGE_STATE(FLIP_ERROR);
+    return;
+  }
+  ExpandControlFrameBuffer(remaining_control_payload_);
+  CHANGE_STATE(FLIP_CONTROL_FRAME_PAYLOAD);
+}
+
 size_t FlipFramer::ProcessControlFramePayload(const char* data, size_t len) {
   size_t original_len = len;
   do {
@@ -351,26 +303,85 @@ size_t FlipFramer::ProcessControlFramePayload(const char* data, size_t len) {
       if (remaining_control_payload_)
         break;
     }
-    FlipControlFrame* control_frame =
-        reinterpret_cast<FlipControlFrame*>(current_frame_buffer_);
-    visitor_->OnControl(control_frame);
+    FlipControlFrame control_frame(current_frame_buffer_, false);
+    visitor_->OnControl(&control_frame);
 
     // If this is a FIN, tell the caller.
-    if (control_frame->type() == SYN_REPLY &&
-        control_frame->flags() & CONTROL_FLAG_FIN)
-      visitor_->OnStreamFrameData(control_frame->stream_id(), NULL, 0);
+    if (control_frame.type() == SYN_REPLY &&
+        control_frame.flags() & CONTROL_FLAG_FIN)
+      visitor_->OnStreamFrameData(control_frame.stream_id(), NULL, 0);
 
     CHANGE_STATE(FLIP_IGNORE_REMAINING_PAYLOAD);
   } while (false);
   return original_len - len;
 }
 
+size_t FlipFramer::ProcessDataFramePayload(const char* data, size_t len) {
+  size_t original_len = len;
+
+  FlipDataFrame current_data_frame(current_frame_buffer_, false);
+  if (remaining_payload_) {
+    size_t amount_to_forward = std::min(remaining_payload_, len);
+    if (amount_to_forward && state_ != FLIP_IGNORE_REMAINING_PAYLOAD) {
+      if (current_data_frame.flags() & DATA_FLAG_COMPRESSED) {
+        // TODO(mbelshe): Assert that the decompressor is init'ed.
+        if (!InitializeDecompressor())
+          return NULL;
+
+        size_t decompressed_max_size = amount_to_forward * 100;
+        scoped_ptr<char> decompressed(new char[decompressed_max_size]);
+        decompressor_->next_in = reinterpret_cast<Bytef*>(
+            const_cast<char*>(data));
+        decompressor_->avail_in = amount_to_forward;
+        decompressor_->next_out =
+            reinterpret_cast<Bytef*>(decompressed.get());
+        decompressor_->avail_out = decompressed_max_size;
+
+        int rv = inflate(decompressor_.get(), Z_SYNC_FLUSH);
+        if (rv != Z_OK) {
+          set_error(FLIP_DECOMPRESS_FAILURE);
+          goto bottom;
+        }
+        size_t decompressed_size = decompressed_max_size -
+                                   decompressor_->avail_out;
+        // Only inform the visitor if there is data.
+        if (decompressed_size)
+          visitor_->OnStreamFrameData(current_data_frame.stream_id(),
+                                      decompressed.get(),
+                                      decompressed_size);
+        amount_to_forward -= decompressor_->avail_in;
+      } else {
+        // The data frame was not compressed.
+        // Only inform the visitor if there is data.
+        if (amount_to_forward)
+          visitor_->OnStreamFrameData(current_data_frame.stream_id(),
+                                      data, amount_to_forward);
+      }
+    }
+    data += amount_to_forward;
+    len -= amount_to_forward;
+    remaining_payload_ -= amount_to_forward;
+
+    // If the FIN flag is set, and there is no more data in this data
+    // frame, inform the visitor of EOF via a 0-length data frame.
+    if (!remaining_payload_ &&
+        current_data_frame.flags() & DATA_FLAG_FIN)
+      visitor_->OnStreamFrameData(current_data_frame.stream_id(), NULL,
+                                  0);
+  } else {
+    CHANGE_STATE(FLIP_AUTO_RESET);
+  }
+
+ bottom:
+  return original_len - len;
+}
+
 void FlipFramer::ExpandControlFrameBuffer(size_t size) {
   DCHECK(size < kControlFrameBufferMaxSize);
   if (size < current_frame_capacity_)
     return;
 
-  int alloc_size = size + sizeof(FlipFrame);
+  int alloc_size = size + FlipFrame::size();
   char* new_buffer = new char[alloc_size];
   memcpy(new_buffer, current_frame_buffer_, current_frame_len_);
   current_frame_capacity_ = alloc_size;
@@ -379,17 +390,18 @@ void FlipFramer::ExpandControlFrameBuffer(size_t size) {
 
 bool FlipFramer::ParseHeaderBlock(const FlipFrame* frame,
                                   FlipHeaderBlock* block) {
-  uint32 type = reinterpret_cast<const FlipControlFrame*>(frame)->type();
+  FlipControlFrame control_frame(frame->data(), false);
+  uint32 type = control_frame.type();
   if (type != SYN_STREAM && type != SYN_REPLY)
     return false;
 
   // Find the header data within the control frame.
-  scoped_array<FlipSynStreamControlFrame> control_frame(
-    reinterpret_cast<FlipSynStreamControlFrame*>(DecompressFrame(frame)));
-  if (!control_frame.get())
+  scoped_ptr<FlipFrame> decompressed_frame(DecompressFrame(frame));
+  if (!decompressed_frame.get())
     return false;
-  const char *header_data = control_frame.get()->header_block();
-  int header_length = control_frame.get()->header_block_len();
+  FlipSynStreamControlFrame syn_frame(decompressed_frame->data(), false);
+  const char *header_data = syn_frame.header_block();
+  int header_length = syn_frame.header_block_len();
 
   FlipFrameBuilder builder(header_data, header_length);
   void* iter = NULL;
@@ -438,21 +450,18 @@ FlipSynStreamControlFrame* FlipFramer::CreateSynStream(
   }
 
   // Write the length and flags.
-  size_t length = frame.length() - sizeof(FlipFrame);
+  size_t length = frame.length() - FlipFrame::size();
   DCHECK(length < static_cast<size_t>(kLengthMask));
   FlagsAndLength flags_length;
   flags_length.length_ = htonl(static_cast<uint32>(length));
   flags_length.flags_[0] = flags;
   frame.WriteBytesToOffset(4, &flags_length, sizeof(flags_length));
 
-  if (compressed) {
-    FlipSynStreamControlFrame* new_frame =
-        reinterpret_cast<FlipSynStreamControlFrame*>(
-        CompressFrame(frame.data()));
-    return new_frame;
-  }
-
-  return reinterpret_cast<FlipSynStreamControlFrame*>(frame.take());
+  scoped_ptr<FlipFrame> syn_frame(frame.take());
+  if (compressed)
+    return reinterpret_cast<FlipSynStreamControlFrame*>(
+        CompressFrame(syn_frame.get()));
+  return reinterpret_cast<FlipSynStreamControlFrame*>(syn_frame.release());
 }
 
 /* static */
@@ -487,17 +496,18 @@ FlipSynReplyControlFrame* FlipFramer::CreateSynReply(FlipStreamId stream_id,
   }
 
   // Write the length
-  size_t length = frame.length() - sizeof(FlipFrame);
+  size_t length = frame.length() - FlipFrame::size();
   DCHECK(length < static_cast<size_t>(kLengthMask));
   FlagsAndLength flags_length;
   flags_length.length_ = htonl(static_cast<uint32>(length));
   flags_length.flags_[0] = flags;
   frame.WriteBytesToOffset(4, &flags_length, sizeof(flags_length));
 
+  scoped_ptr<FlipFrame> reply_frame(frame.take());
   if (compressed)
     return reinterpret_cast<FlipSynReplyControlFrame*>(
-        CompressFrame(frame.data()));
-  return reinterpret_cast<FlipSynReplyControlFrame*>(frame.take());
+        CompressFrame(reply_frame.get()));
+  return reinterpret_cast<FlipSynReplyControlFrame*>(reply_frame.release());
 }
 
 FlipDataFrame* FlipFramer::CreateDataFrame(FlipStreamId stream_id,
@@ -514,9 +524,10 @@ FlipDataFrame* FlipFramer::CreateDataFrame(FlipStreamId stream_id,
   frame.WriteBytes(&flags_length, sizeof(flags_length));
 
   frame.WriteBytes(data, len);
+  scoped_ptr<FlipFrame> data_frame(frame.take());
   if (flags & DATA_FLAG_COMPRESSED)
-    return reinterpret_cast<FlipDataFrame*>(CompressFrame(frame.data()));
-  return reinterpret_cast<FlipDataFrame*>(frame.take());
+    return reinterpret_cast<FlipDataFrame*>(CompressFrame(data_frame.get()));
+  return reinterpret_cast<FlipDataFrame*>(data_frame.release());
 }
 
 /* static */
@@ -589,7 +600,7 @@ bool FlipFramer::InitializeDecompressor() {
 bool FlipFramer::GetFrameBoundaries(const FlipFrame* frame,
                                     int* payload_length,
                                     int* header_length,
-                                    const unsigned char** payload) const {
+                                    const char** payload) const {
   if (frame->is_control_frame()) {
     const FlipControlFrame* control_frame =
         reinterpret_cast<const FlipControlFrame*>(frame);
@@ -600,10 +611,8 @@ bool FlipFramer::GetFrameBoundaries(const FlipFrame* frame,
           const FlipSynStreamControlFrame *syn_frame =
               reinterpret_cast<const FlipSynStreamControlFrame*>(frame);
           *payload_length = syn_frame->header_block_len();
-          *header_length = sizeof(FlipFrame) + syn_frame->length() -
-              syn_frame->header_block_len();
-          *payload = reinterpret_cast<const unsigned char*>(frame) +
-              *header_length;
+          *header_length = syn_frame->size();
+          *payload = frame->data() + *header_length;
         }
         break;
       default:
@@ -611,13 +620,12 @@ bool FlipFramer::GetFrameBoundaries(const FlipFrame* frame,
         return false;  // We can't compress this frame!
     }
   } else {
-    *header_length = sizeof(FlipFrame);
+    *header_length = FlipFrame::size();
     *payload_length = frame->length();
-    *payload = reinterpret_cast<const unsigned char*>(frame) +
-        sizeof(FlipFrame);
+    *payload = frame->data() + FlipFrame::size();
   }
   DCHECK(static_cast<size_t>(*header_length) <=
-      sizeof(FlipFrame) + *payload_length);
+      FlipFrame::size() + *payload_length);
   return true;
 }
 
@@ -625,7 +633,7 @@ bool FlipFramer::GetFrameBoundaries(const FlipFrame* frame,
 FlipFrame* FlipFramer::CompressFrame(const FlipFrame* frame) {
   int payload_length;
   int header_length;
-  const unsigned char* payload;
+  const char* payload;
 
   static StatsCounter pre_compress_bytes("flip.PreCompressSize");
   static StatsCounter post_compress_bytes("flip.PostCompressSize");
@@ -644,13 +652,13 @@ FlipFrame* FlipFramer::CompressFrame(const FlipFrame* frame) {
   // Create an output frame.
   int compressed_max_size = deflateBound(compressor_.get(), payload_length);
   int new_frame_size = header_length + compressed_max_size;
-  FlipFrame* new_frame =
-      reinterpret_cast<FlipFrame*>(new char[new_frame_size]);
-  memcpy(new_frame, frame, header_length);
+  FlipFrame* new_frame = new FlipFrame(new_frame_size);
+  memcpy(new_frame->data(), frame->data(), frame->length() + FlipFrame::size());
 
-  compressor_->next_in = const_cast<Bytef*>(payload);
+  compressor_->next_in = reinterpret_cast<Bytef*>(const_cast<char*>(payload));
   compressor_->avail_in = payload_length;
-  compressor_->next_out = reinterpret_cast<Bytef*>(new_frame) + header_length;
+  compressor_->next_out = reinterpret_cast<Bytef*>(new_frame->data()) +
+                          header_length;
   compressor_->avail_out = compressed_max_size;
 
   // Data packets have a 'compressed flag
@@ -667,7 +675,7 @@ FlipFrame* FlipFramer::CompressFrame(const FlipFrame* frame) {
   }
 
   int compressed_size = compressed_max_size - compressor_->avail_out;
-  new_frame->set_length(header_length + compressed_size - sizeof(FlipFrame));
+  new_frame->set_length(header_length + compressed_size - FlipFrame::size());
 
   pre_compress_bytes.Add(payload_length);
   post_compress_bytes.Add(new_frame->length());
@@ -678,7 +686,7 @@ FlipFrame* FlipFramer::CompressFrame(const FlipFrame* frame) {
 FlipFrame* FlipFramer::DecompressFrame(const FlipFrame* frame) {
   int payload_length;
   int header_length;
-  const unsigned char* payload;
+  const char* payload;
 
   static StatsCounter pre_decompress_bytes("flip.PreDeCompressSize");
   static StatsCounter post_decompress_bytes("flip.PostDeCompressSize");
@@ -705,13 +713,12 @@ FlipFrame* FlipFramer::DecompressFrame(const FlipFrame* frame) {
   // the input data.
   int decompressed_max_size = kControlFrameBufferInitialSize;
   int new_frame_size = header_length + decompressed_max_size;
-  FlipFrame* new_frame =
-      reinterpret_cast<FlipFrame*>(new char[new_frame_size]);
-  memcpy(new_frame, frame, header_length);
+  FlipFrame* new_frame = new FlipFrame(new_frame_size);
+  memcpy(new_frame->data(), frame->data(), frame->length() + FlipFrame::size());
 
-  decompressor_->next_in = const_cast<Bytef*>(payload);
+  decompressor_->next_in = reinterpret_cast<Bytef*>(const_cast<char*>(payload));
   decompressor_->avail_in = payload_length;
-  decompressor_->next_out = reinterpret_cast<Bytef*>(new_frame) +
+  decompressor_->next_out = reinterpret_cast<Bytef*>(new_frame->data()) +
       header_length;
   decompressor_->avail_out = decompressed_max_size;
 
@@ -737,7 +744,7 @@ FlipFrame* FlipFramer::DecompressFrame(const FlipFrame* frame) {
   }
 
   int decompressed_size = decompressed_max_size - decompressor_->avail_out;
-  new_frame->set_length(header_length + decompressed_size - sizeof(FlipFrame));
+  new_frame->set_length(header_length + decompressed_size - FlipFrame::size());
 
   pre_decompress_bytes.Add(frame->length());
   post_decompress_bytes.Add(new_frame->length());
@@ -746,10 +753,10 @@ FlipFrame* FlipFramer::DecompressFrame(const FlipFrame* frame) {
 }
 
 FlipFrame* FlipFramer::DuplicateFrame(const FlipFrame* frame) {
-  int size = sizeof(FlipFrame) + frame->length();
-  char* new_frame = new char[size];
-  memcpy(new_frame, frame, size);
-  return reinterpret_cast<FlipFrame*>(new_frame);
+  int size = FlipFrame::size() + frame->length();
+  FlipFrame* new_frame = new FlipFrame(size);
+  memcpy(new_frame->data(), frame->data(), size);
+  return new_frame;
 }
 
 void FlipFramer::set_enable_compression(bool value) {
diff --git a/net/flip/flip_framer.h b/net/flip/flip_framer.h
index d3cae28..b333176 100644
--- a/net/flip/flip_framer.h
+++ b/net/flip/flip_framer.h
@@ -30,7 +30,11 @@ namespace flip {
 
 class FlipFramer;
 class FlipFramerTest;
+
+namespace test {
 class TestFlipVisitor;
+void FramerSetEnableCompressionHelper(FlipFramer* framer, bool compress);
+}  // namespace test
 
 // A datastructure for holding a set of headers from either a
 // SYN_STREAM or SYN_REPLY frame.
@@ -58,9 +62,6 @@ class FlipFramerVisitorInterface {
   virtual void OnStreamFrameData(flip::FlipStreamId stream_id,
                                  const char* data,
                                  size_t len) = 0;
-
-  // TODO(fenix): Implement me!
-  virtual void OnLameDuck() = 0;
 };
 
 class FlipFramer {
@@ -200,9 +201,11 @@ class FlipFramer {
 
  protected:
   FRIEND_TEST(FlipFramerTest, HeaderBlockBarfsOnOutOfOrderHeaders);
-  friend class flip::TestFlipVisitor;
   friend class net::FlipNetworkTransactionTest;
   friend class net::HttpNetworkLayer;  // This is temporary for the server.
+  friend class test::TestFlipVisitor;
+  friend void test::FramerSetEnableCompressionHelper(FlipFramer* framer,
+                                                     bool compress);
 
   // For ease of testing we can tweak compression on/off.
   void set_enable_compression(bool value);
@@ -212,7 +215,9 @@ class FlipFramer {
   // Internal breakout from ProcessInput.  Returns the number of bytes
   // consumed from the data.
   size_t ProcessCommonHeader(const char* data, size_t len);
+  void ProcessControlFrameHeader();
   size_t ProcessControlFramePayload(const char* data, size_t len);
+  size_t ProcessDataFramePayload(const char* data, size_t len);
 
   // Initialize the ZLib state.
   bool InitializeCompressor();
@@ -230,8 +235,7 @@ class FlipFramer {
   // Given a frame, breakdown the variable payload length, the static header
   // header length, and variable payload pointer.
   bool GetFrameBoundaries(const FlipFrame* frame, int* payload_length,
-                          int* header_length,
-                          const unsigned char** payload) const;
+                          int* header_length, const char** payload) const;
 
   FlipState state_;
   FlipError error_code_;
diff --git a/net/flip/flip_framer_test.cc b/net/flip/flip_framer_test.cc
index bee0432..921e052 100644
--- a/net/flip/flip_framer_test.cc
+++ b/net/flip/flip_framer_test.cc
@@ -2,9 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
 #include <iostream>
-#include "base/scoped_ptr.h"
 
+#include "base/scoped_ptr.h"
 #include "flip_framer.h"  // cross-google3 directory naming.
 #include "flip_protocol.h"
 #include "flip_frame_builder.h"
@@ -12,14 +13,15 @@
 
 namespace flip {
 
-class FlipFramerTest : public PlatformTest {
- public:
-  virtual void TearDown() {}
-};
+namespace test {
+
+void FramerSetEnableCompressionHelper(FlipFramer* framer, bool compress) {
+  framer->set_enable_compression(compress);
+}
 
 class TestFlipVisitor : public FlipFramerVisitorInterface  {
  public:
-  explicit TestFlipVisitor()
+  TestFlipVisitor()
     : error_count_(0),
       syn_frame_count_(0),
       syn_reply_frame_count_(0),
@@ -37,10 +39,9 @@ class TestFlipVisitor : public FlipFramerVisitorInterface  {
                          const char* data,
                          size_t len) {
     if (len == 0)
-      zero_length_data_frame_count_++;
+      ++zero_length_data_frame_count_;
 
     data_bytes_ += len;
-#ifdef TEST_LOGGING
     std::cerr << "OnStreamFrameData(" << stream_id << ", \"";
     if (len > 0) {
       for (size_t i = 0 ; i < len; ++i) {
@@ -48,7 +49,6 @@ class TestFlipVisitor : public FlipFramerVisitorInterface  {
       }
     }
     std::cerr << "\", " << len << ")\n";
-#endif  // TEST_LOGGING
   }
 
   void OnControl(const FlipControlFrame* frame) {
@@ -56,14 +56,12 @@ class TestFlipVisitor : public FlipFramerVisitorInterface  {
     bool parsed_headers = false;
     switch (frame->type()) {
       case SYN_STREAM:
-        parsed_headers = framer_.ParseHeaderBlock(
-            reinterpret_cast<const FlipFrame*>(frame), &headers);
+        parsed_headers = framer_.ParseHeaderBlock(frame, &headers);
         DCHECK(parsed_headers);
         syn_frame_count_++;
         break;
       case SYN_REPLY:
-        parsed_headers = framer_.ParseHeaderBlock(
-            reinterpret_cast<const FlipFrame*>(frame), &headers);
+        parsed_headers = framer_.ParseHeaderBlock(frame, &headers);
         DCHECK(parsed_headers);
         syn_reply_frame_count_++;
         break;
@@ -74,10 +72,7 @@ class TestFlipVisitor : public FlipFramerVisitorInterface  {
         DCHECK(false);  // Error!
     }
     if (frame->flags() & CONTROL_FLAG_FIN)
-      fin_flag_count_++;
-  }
-
-  void OnLameDuck() {
+      ++fin_flag_count_;
   }
 
   // Convenience function which runs a framer simulation with particular input.
@@ -113,75 +108,27 @@ class TestFlipVisitor : public FlipFramerVisitorInterface  {
   int zero_length_data_frame_count_;  // The count of zero-length data frames.
 };
 
-// Test our protocol constants
-TEST_F(FlipFramerTest, ProtocolConstants) {
-  EXPECT_EQ(8u, sizeof(FlipFrame));
-  EXPECT_EQ(8u, sizeof(FlipDataFrame));
-  EXPECT_EQ(12u, sizeof(FlipControlFrame));
-  EXPECT_EQ(16u, sizeof(FlipSynStreamControlFrame));
-  EXPECT_EQ(16u, sizeof(FlipSynReplyControlFrame));
-  EXPECT_EQ(16u, sizeof(FlipFinStreamControlFrame));
-  EXPECT_EQ(1, SYN_STREAM);
-  EXPECT_EQ(2, SYN_REPLY);
-  EXPECT_EQ(3, FIN_STREAM);
-}
+}  // namespace test
 
-// Test some of the protocol helper functions
-TEST_F(FlipFramerTest, FrameStructs) {
-  FlipFrame frame;
-  memset(&frame, 0, sizeof(frame));
-  frame.set_length(12345);
-  frame.set_flags(10);
-  EXPECT_EQ(12345u, frame.length());
-  EXPECT_EQ(10u, frame.flags());
-  EXPECT_EQ(false, frame.is_control_frame());
-
-  memset(&frame, 0, sizeof(frame));
-  frame.set_length(0);
-  frame.set_flags(10);
-  EXPECT_EQ(0u, frame.length());
-  EXPECT_EQ(10u, frame.flags());
-  EXPECT_EQ(false, frame.is_control_frame());
-}
+}  // namespace flip
 
-TEST_F(FlipFramerTest, DataFrameStructs) {
-  FlipDataFrame data_frame;
-  memset(&data_frame, 0, sizeof(data_frame));
-  data_frame.set_stream_id(12345);
-  EXPECT_EQ(12345u, data_frame.stream_id());
-}
+using flip::FlipFrame;
+using flip::FlipFrameBuilder;
+using flip::FlipFramer;
+using flip::FlipHeaderBlock;
+using flip::FlipSynStreamControlFrame;
+using flip::kControlFlagMask;
+using flip::CONTROL_FLAG_NONE;
+using flip::SYN_STREAM;
+using flip::test::FramerSetEnableCompressionHelper;
+using flip::test::TestFlipVisitor;
 
-TEST_F(FlipFramerTest, ControlFrameStructs) {
-  FlipFramer framer;
-  FlipHeaderBlock headers;
+namespace {
 
-  scoped_array<FlipSynStreamControlFrame> syn_frame(
-      framer.CreateSynStream(123, 2, CONTROL_FLAG_NONE, false, &headers));
-  EXPECT_EQ(kFlipProtocolVersion, syn_frame.get()->version());
-  EXPECT_EQ(true, syn_frame.get()->is_control_frame());
-  EXPECT_EQ(SYN_STREAM, syn_frame.get()->type());
-  EXPECT_EQ(123u, syn_frame.get()->stream_id());
-  EXPECT_EQ(2u, syn_frame.get()->priority());
-  EXPECT_EQ(2, syn_frame.get()->header_block_len());
-
-  scoped_array<FlipSynReplyControlFrame> syn_reply(
-      framer.CreateSynReply(123, CONTROL_FLAG_NONE, false, &headers));
-  EXPECT_EQ(kFlipProtocolVersion, syn_reply.get()->version());
-  EXPECT_EQ(true, syn_reply.get()->is_control_frame());
-  EXPECT_EQ(SYN_REPLY, syn_reply.get()->type());
-  EXPECT_EQ(123u, syn_reply.get()->stream_id());
-  EXPECT_EQ(2, syn_reply.get()->header_block_len());
-
-  scoped_array<FlipFinStreamControlFrame> fin_frame(
-      framer.CreateFinStream(123, 444));
-  EXPECT_EQ(kFlipProtocolVersion, fin_frame.get()->version());
-  EXPECT_EQ(true, fin_frame.get()->is_control_frame());
-  EXPECT_EQ(FIN_STREAM, fin_frame.get()->type());
-  EXPECT_EQ(123u, fin_frame.get()->stream_id());
-  EXPECT_EQ(444u, fin_frame.get()->status());
-  fin_frame.get()->set_status(555);
-  EXPECT_EQ(555u, fin_frame.get()->status());
-}
+class FlipFramerTest : public PlatformTest {
+ public:
+  virtual void TearDown() {}
+};
 
 // Test that we can encode and decode a FlipHeaderBlock.
 TEST_F(FlipFramerTest, HeaderBlock) {
@@ -191,13 +138,12 @@ TEST_F(FlipFramerTest, HeaderBlock) {
   FlipFramer framer;
 
   // Encode the header block into a SynStream frame.
-  scoped_array<FlipSynStreamControlFrame> frame(
+  scoped_ptr<FlipSynStreamControlFrame> frame(
       framer.CreateSynStream(1, 1, CONTROL_FLAG_NONE, true, &headers));
   EXPECT_TRUE(frame.get() != NULL);
 
   FlipHeaderBlock new_headers;
-  FlipFrame* control_frame = reinterpret_cast<FlipFrame*>(frame.get());
-  framer.ParseHeaderBlock(control_frame, &new_headers);
+  framer.ParseHeaderBlock(frame.get(), &new_headers);
 
   EXPECT_EQ(headers.size(), new_headers.size());
   EXPECT_EQ(headers["alpha"], new_headers["alpha"]);
@@ -220,13 +166,13 @@ TEST_F(FlipFramerTest, HeaderBlockBarfsOnOutOfOrderHeaders) {
   frame.WriteString("alpha");
   frame.WriteString("alpha");
   // write the length
-  frame.WriteUInt32ToOffset(4, frame.length() - sizeof(FlipFrame));
+  frame.WriteUInt32ToOffset(4, frame.length() - FlipFrame::size());
 
   FlipHeaderBlock new_headers;
-  const FlipFrame* control_frame = frame.data();
+  scoped_ptr<FlipFrame> control_frame(frame.take());
   FlipFramer framer;
-  framer.set_enable_compression(false);
-  EXPECT_FALSE(framer.ParseHeaderBlock(control_frame, &new_headers));
+  FramerSetEnableCompressionHelper(&framer, false);
+  EXPECT_FALSE(framer.ParseHeaderBlock(control_frame.get(), &new_headers));
 }
 
 TEST_F(FlipFramerTest, BasicCompression) {
@@ -239,26 +185,45 @@ TEST_F(FlipFramerTest, BasicCompression) {
   headers["content-length"] = "12";
 
   FlipFramer framer;
-  scoped_array<FlipSynStreamControlFrame>
+  FramerSetEnableCompressionHelper(&framer, true);
+  scoped_ptr<FlipSynStreamControlFrame>
       frame1(framer.CreateSynStream(1, 1, CONTROL_FLAG_NONE, true, &headers));
-  scoped_array<FlipSynStreamControlFrame>
+  scoped_ptr<FlipSynStreamControlFrame>
       frame2(framer.CreateSynStream(1, 1, CONTROL_FLAG_NONE, true, &headers));
 
   // Expect the second frame to be more compact than the first.
-  EXPECT_LE(frame2.get()->length(), frame1.get()->length());
+  EXPECT_LE(frame2->length(), frame1->length());
 
   // Decompress the first frame
-  scoped_array<FlipFrame> frame3(
-      framer.DecompressFrame(reinterpret_cast<FlipFrame*>(frame1.get())));
+  scoped_ptr<FlipFrame> frame3(framer.DecompressFrame(frame1.get()));
 
   // Decompress the second frame
-  scoped_array<FlipFrame> frame4(
-      framer.DecompressFrame(reinterpret_cast<FlipFrame*>(frame2.get())));
+  scoped_ptr<FlipFrame> frame4(framer.DecompressFrame(frame2.get()));
 
   // Expect frames 3 & 4 to be the same.
   EXPECT_EQ(0,
-      memcmp(frame3.get(), frame4.get(),
-      sizeof(FlipFrame) + frame3.get()->length()));
+      memcmp(frame3->data(), frame4->data(),
+      FlipFrame::size() + frame3->length()));
+}
+
+TEST_F(FlipFramerTest, DecompressUncompressedFrame) {
+  FlipHeaderBlock headers;
+  headers["server"] = "FlipServer 1.0";
+  headers["date"] = "Mon 12 Jan 2009 12:12:12 PST";
+  headers["status"] = "200";
+  headers["version"] = "HTTP/1.1";
+  headers["content-type"] = "text/html";
+  headers["content-length"] = "12";
+
+  FlipFramer framer;
+  FramerSetEnableCompressionHelper(&framer, true);
+  scoped_ptr<FlipSynStreamControlFrame>
+      frame1(framer.CreateSynStream(1, 1, CONTROL_FLAG_NONE, false, &headers));
+
+  // Decompress the frame
+  scoped_ptr<FlipFrame> frame2(framer.DecompressFrame(frame1.get()));
+
+  EXPECT_EQ(NULL, frame2.get());
 }
 
 TEST_F(FlipFramerTest, Basic) {
@@ -386,6 +351,5 @@ TEST_F(FlipFramerTest, FinOnSynReplyFrame) {
   EXPECT_EQ(1, visitor.zero_length_data_frame_count_);
 }
 
-}  // namespace flip
-
+}  // namespace
 
diff --git a/net/flip/flip_protocol.h b/net/flip/flip_protocol.h
index 8a585bc..7c0c659 100644
--- a/net/flip/flip_protocol.h
+++ b/net/flip/flip_protocol.h
@@ -12,11 +12,11 @@
 #else
 #include <arpa/inet.h>
 #endif
+
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "flip_bitmasks.h"  // cross-google3 directory naming.
 
-
 //  Data Frame Format
 //  +----------------------------------+
 //  |0|       Stream-ID (31bits)       |
@@ -116,29 +116,22 @@ typedef uint32 FlipStreamId;
 #define FLIP_PRIORITY_LOWEST 3
 #define FLIP_PRIORITY_HIGHEST 0
 
+// -------------------------------------------------------------------------
+// These structures mirror the protocol structure definitions.
+
+// For the control data structures, we pack so that sizes match the
+// protocol over-the-wire sizes.
+#pragma pack(push)
+#pragma pack(1)
+
 // A special structure for the 8 bit flags and 24 bit length fields.
 union FlagsAndLength {
-  uint8 flags_[4]; // 8 bits
-  uint32 length_;  // 24 bits
+  uint8 flags_[4];  // 8 bits
+  uint32 length_;   // 24 bits
 };
 
-// All Flip Frame types derive from the FlipFrame struct.
-typedef struct {
-  uint8 flags() const { return flags_length_.flags_[0]; }
-  void set_flags(uint8 flags) { flags_length_.flags_[0] = flags; }
-
-  uint32 length() const { return ntohl(flags_length_.length_) & kLengthMask; }
-  void set_length(uint32 length) {
-    DCHECK((length & ~kLengthMask) == 0);
-    length = htonl(length & kLengthMask);
-    flags_length_.length_ = (flags() << 6) | length;
-  }
-
-  bool is_control_frame() const {
-    return (ntohs(control_.version_) & kControlFlagMask) == kControlFlagMask;
-  }
-
- protected:
+// The basic FLIP Frame.
+struct FlipFrameBlock {
   union {
     struct {
       uint16 version_;
@@ -149,74 +142,229 @@ typedef struct {
     } data_;
   };
   FlagsAndLength flags_length_;
-} FlipFrame;
+};
+
+// A Control Frame.
+struct FlipControlFrameBlock : FlipFrameBlock {
+  FlipStreamId stream_id_;
+};
+
+// A SYN_STREAM Control Frame.
+struct FlipSynStreamControlFrameBlock : FlipControlFrameBlock {
+  uint8 priority_;
+  uint8 unused_;
+};
+
+// A SYN_REPLY Control Frame.
+struct FlipSynReplyControlFrameBlock : FlipControlFrameBlock {
+  uint16 unused_;
+};
+
+// A FNI_STREAM Control Frame.
+struct FlipFinStreamControlFrameBlock : FlipControlFrameBlock {
+  uint32 status_;
+};
+
+#pragma pack(pop)
+
+// -------------------------------------------------------------------------
+// Wrapper classes for various FLIP frames.
+
+// All Flip Frame types derive from this FlipFrame class.
+class FlipFrame {
+ public:
+  // Create a FlipFrame for a given sized buffer.
+  explicit FlipFrame(size_t size) : frame_(NULL), needs_delete_(true) {
+    DCHECK_GE(size, sizeof(struct FlipFrameBlock));
+    char* buffer = new char[size];
+    memset(buffer, 0, size);
+    frame_ = reinterpret_cast<struct FlipFrameBlock*>(buffer);
+  }
+
+  // Create a FlipFrame using a pre-created buffer.
+  // If |needs_delete| is true, this class takes ownership of the buffer
+  // and will delete it on cleanup.  The buffer must have been created using
+  // new char[].
+  // If |needs_delete| is false, the caller retains ownership
+  // of the buffer and will keep the buffer alive longer than |this|.  In other
+  // words, this class does NOT create a copy of the buffer.
+  FlipFrame(char* data, bool needs_delete)
+      : frame_(reinterpret_cast<struct FlipFrameBlock*>(data)),
+        needs_delete_(needs_delete) {
+    DCHECK(frame_);
+  }
+
+  virtual ~FlipFrame() {
+    if (needs_delete_) {
+      char* buffer = reinterpret_cast<char*>(frame_);
+      delete [] buffer;
+    }
+    frame_ = NULL;
+  }
+
+  // Provide access to the frame bytes
+  char* data() const { return reinterpret_cast<char*>(frame_); }
+
+  uint8 flags() const { return frame_->flags_length_.flags_[0]; }
+  void set_flags(uint8 flags) { frame_->flags_length_.flags_[0] = flags; }
+
+  uint32 length() const {
+    return ntohl(frame_->flags_length_.length_) & kLengthMask;
+  }
+
+  void set_length(uint32 length) {
+    DCHECK_EQ(0u, (length & ~kLengthMask));
+    length = htonl(length & kLengthMask);
+    frame_->flags_length_.length_ = flags() | length;
+  }
+
+  bool is_control_frame() const {
+    return (ntohs(frame_->control_.version_) & kControlFlagMask) ==
+        kControlFlagMask;
+  }
+
+  static size_t size() { return sizeof(struct FlipFrameBlock); }
+
+ protected:
+  FlipFrameBlock* frame_;
+
+ private:
+  bool needs_delete_;
+  DISALLOW_COPY_AND_ASSIGN(FlipFrame);
+};
 
 // A Data Frame.
-typedef struct : public FlipFrame {
+class FlipDataFrame : public FlipFrame {
+ public:
+  FlipDataFrame() : FlipFrame(size()) {}
+  FlipDataFrame(char* data, bool needs_delete)
+      : FlipFrame(data, needs_delete) {}
+  virtual ~FlipDataFrame() {}
+
   FlipStreamId stream_id() const {
-      return ntohl(data_.stream_id_) & kStreamIdMask;
+    return ntohl(frame_->data_.stream_id_) & kStreamIdMask;
+  }
+
+  // Note that setting the stream id sets the control bit to false.
+  // As stream id should always be set, this means the control bit
+  // should always be set correctly.
+  void set_stream_id(FlipStreamId id) {
+    DCHECK_EQ(0u, (id & ~kStreamIdMask));
+    frame_->data_.stream_id_ = htonl(id & kStreamIdMask);
   }
-  void set_stream_id(FlipStreamId id) { data_.stream_id_ = htonl(id); }
-} FlipDataFrame;
+
+  static size_t size() { return FlipFrame::size(); }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FlipDataFrame);
+};
 
 // A Control Frame.
-typedef struct : public FlipFrame {
+class FlipControlFrame : public FlipFrame {
+ public:
+  explicit FlipControlFrame(size_t size) : FlipFrame(size) {}
+  FlipControlFrame(char* data, bool needs_delete)
+      : FlipFrame(data, needs_delete) {}
+  virtual ~FlipControlFrame() {}
+
   uint16 version() const {
     const int kVersionMask = 0x7fff;
-    return ntohs(control_.version_) & kVersionMask;
+    return ntohs(block()->control_.version_) & kVersionMask;
   }
   FlipControlType type() const {
-    uint16 type = ntohs(control_.type_);
+    uint16 type = ntohs(block()->control_.type_);
     DCHECK(type >= SYN_STREAM && type <= NOOP);
     return static_cast<FlipControlType>(type);
   }
-  FlipStreamId stream_id() const { return ntohl(stream_id_) & kStreamIdMask; }
+  FlipStreamId stream_id() const {
+    return ntohl(block()->stream_id_) & kStreamIdMask;
+  }
+
+  void set_stream_id(FlipStreamId id) {
+    block()->stream_id_ = htonl(id & kStreamIdMask);
+  }
+
+  static size_t size() { return sizeof(FlipControlFrameBlock); }
 
  private:
-  FlipStreamId stream_id_;
-} FlipControlFrame;
+  struct FlipControlFrameBlock* block() const {
+    return static_cast<FlipControlFrameBlock*>(frame_);
+  }
+  DISALLOW_COPY_AND_ASSIGN(FlipControlFrame);
+};
 
 // A SYN_STREAM frame.
-typedef struct FlipSynStreamControlFrame : public FlipControlFrame {
-  uint8 priority() const { return (priority_ & kPriorityMask) >> 6; }
+class FlipSynStreamControlFrame : public FlipControlFrame {
+ public:
+  FlipSynStreamControlFrame() : FlipControlFrame(size()) {}
+  FlipSynStreamControlFrame(char* data, bool needs_delete)
+      : FlipControlFrame(data, needs_delete) {}
+  virtual ~FlipSynStreamControlFrame() {}
+
+  uint8 priority() const { return (block()->priority_ & kPriorityMask) >> 6; }
+
   // The number of bytes in the header block beyond the frame header length.
-  int header_block_len() const { return length() - kHeaderBlockOffset; }
+  int header_block_len() const {
+    return length() - (size() - FlipFrame::size());
+  }
+
   const char* header_block() const {
-    return reinterpret_cast<const char*>(this) +
-        sizeof(FlipFrame) + kHeaderBlockOffset;
+    return reinterpret_cast<const char*>(block()) + size();
   }
+
+  static size_t size() { return sizeof(FlipSynStreamControlFrameBlock); }
+
  private:
-  // Offset from the end of the FlipControlFrame to the FlipHeaderBlock.
-  static const int kHeaderBlockOffset = 6;
-  uint8 priority_;
-  uint8 unused_;
-  // Variable FlipHeaderBlock here.
-} FlipSynStreamControlFrame;
+  struct FlipSynStreamControlFrameBlock* block() const {
+    return static_cast<FlipSynStreamControlFrameBlock*>(frame_);
+  }
+  DISALLOW_COPY_AND_ASSIGN(FlipSynStreamControlFrame);
+};
 
 // A SYN_REPLY frame.
-typedef struct FlipSynReplyControlFrame : public FlipControlFrame {
-  int header_block_len() const { return length() - kHeaderBlockOffset; }
+class FlipSynReplyControlFrame : public FlipControlFrame {
+ public:
+  FlipSynReplyControlFrame() : FlipControlFrame(size()) {}
+  FlipSynReplyControlFrame(char* data, bool needs_delete)
+      : FlipControlFrame(data, needs_delete) {}
+  virtual ~FlipSynReplyControlFrame() {}
+
+  int header_block_len() const {
+    return length() - (size() - FlipFrame::size());
+  }
+
   const char* header_block() const {
-    return reinterpret_cast<const char*>(this) +
-        sizeof(FlipFrame) + kHeaderBlockOffset;
+    return reinterpret_cast<const char*>(block()) + size();
   }
 
+  static size_t size() { return sizeof(FlipSynReplyControlFrameBlock); }
+
  private:
-  // Offset from the end of the FlipControlFrame to the FlipHeaderBlock.
-  static const int kHeaderBlockOffset = 6;
-  uint16 unused_;
-  // Variable FlipHeaderBlock here.
-} FlipSynReplyControlFrame;
+  struct FlipSynReplyControlFrameBlock* block() const {
+    return static_cast<FlipSynReplyControlFrameBlock*>(frame_);
+  }
+  DISALLOW_COPY_AND_ASSIGN(FlipSynReplyControlFrame);
+};
 
 // A FIN_STREAM frame.
-typedef struct FlipFinStreamControlFrame : public FlipControlFrame {
-  uint32 status() const { return ntohl(status_); }
-  void set_status(int id) { status_ = htonl(id); }
+class FlipFinStreamControlFrame : public FlipControlFrame {
+ public:
+  FlipFinStreamControlFrame() : FlipControlFrame(size()) {}
+  FlipFinStreamControlFrame(char* data, bool needs_delete)
+      : FlipControlFrame(data, needs_delete) {}
+  virtual ~FlipFinStreamControlFrame() {}
+
+  uint32 status() const { return ntohl(block()->status_); }
+  void set_status(uint32 status) { block()->status_ = htonl(status); }
+
+  static size_t size() { return sizeof(FlipFinStreamControlFrameBlock); }
+
  private:
-  uint32 status_;
-} FlipFinStreamControlFrame;
+  struct FlipFinStreamControlFrameBlock* block() const {
+    return static_cast<FlipFinStreamControlFrameBlock*>(frame_);
+  }
+  DISALLOW_COPY_AND_ASSIGN(FlipFinStreamControlFrame);
+};
 
 }  // namespace flip
 
 #endif  // NET_FLIP_FLIP_PROTOCOL_H_
-
diff --git a/net/flip/flip_protocol_test.cc b/net/flip/flip_protocol_test.cc
new file mode 100644
index 0000000..fb78b63
--- /dev/null
+++ b/net/flip/flip_protocol_test.cc
@@ -0,0 +1,182 @@
+// Copyright (c) 2009 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.
+
+#include "flip_protocol.h"
+
+#include "base/scoped_ptr.h"
+#include "flip_bitmasks.h"   // shared with server.
+#include "flip_framer.h"
+#ifdef _WIN32
+#include "testing/platform_test.h"
+#else
+#include "testing/base/public/gunit.h"
+#endif
+
+using flip::FlipDataFrame;
+using flip::FlipFrame;
+using flip::FlipControlFrame;
+using flip::FlipSynStreamControlFrame;
+using flip::FlipSynReplyControlFrame;
+using flip::FlipFinStreamControlFrame;
+using flip::FlipFramer;
+using flip::FlipHeaderBlock;
+using flip::FlagsAndLength;
+using flip::kLengthMask;
+using flip::kStreamIdMask;
+using flip::kFlipProtocolVersion;
+using flip::SYN_STREAM;
+using flip::SYN_REPLY;
+using flip::FIN_STREAM;
+using flip::CONTROL_FLAG_FIN;
+using flip::CONTROL_FLAG_NONE;
+
+namespace {
+
+// Test our protocol constants
+TEST(FlipProtocolTest, ProtocolConstants) {
+  EXPECT_EQ(8u, FlipFrame::size());
+  EXPECT_EQ(8u, FlipDataFrame::size());
+  EXPECT_EQ(12u, FlipControlFrame::size());
+  EXPECT_EQ(14u, FlipSynStreamControlFrame::size());
+  EXPECT_EQ(14u, FlipSynReplyControlFrame::size());
+  EXPECT_EQ(16u, FlipFinStreamControlFrame::size());
+  EXPECT_EQ(4u, sizeof(FlagsAndLength));
+  EXPECT_EQ(1, SYN_STREAM);
+  EXPECT_EQ(2, SYN_REPLY);
+  EXPECT_EQ(3, FIN_STREAM);
+}
+
+// Test some of the protocol helper functions
+TEST(FlipProtocolTest, FrameStructs) {
+  FlipFrame frame(FlipFrame::size());
+  frame.set_length(12345);
+  frame.set_flags(10);
+  EXPECT_EQ(12345u, frame.length());
+  EXPECT_EQ(10u, frame.flags());
+  EXPECT_EQ(false, frame.is_control_frame());
+
+  frame.set_length(0);
+  frame.set_flags(10);
+  EXPECT_EQ(0u, frame.length());
+  EXPECT_EQ(10u, frame.flags());
+  EXPECT_EQ(false, frame.is_control_frame());
+}
+
+TEST(FlipProtocolTest, DataFrameStructs) {
+  FlipDataFrame data_frame;
+  data_frame.set_stream_id(12345);
+  EXPECT_EQ(12345u, data_frame.stream_id());
+}
+
+TEST(FlipProtocolTest, ControlFrameStructs) {
+  FlipFramer framer;
+  FlipHeaderBlock headers;
+
+  scoped_ptr<FlipSynStreamControlFrame> syn_frame(
+      framer.CreateSynStream(123, 2, CONTROL_FLAG_FIN, false, &headers));
+  EXPECT_EQ(kFlipProtocolVersion, syn_frame->version());
+  EXPECT_EQ(true, syn_frame->is_control_frame());
+  EXPECT_EQ(SYN_STREAM, syn_frame->type());
+  EXPECT_EQ(123u, syn_frame->stream_id());
+  EXPECT_EQ(2u, syn_frame->priority());
+  EXPECT_EQ(2, syn_frame->header_block_len());
+  EXPECT_EQ(1u, syn_frame->flags());
+
+  scoped_ptr<FlipSynReplyControlFrame> syn_reply(
+      framer.CreateSynReply(123, CONTROL_FLAG_NONE, false, &headers));
+  EXPECT_EQ(kFlipProtocolVersion, syn_reply->version());
+  EXPECT_EQ(true, syn_reply->is_control_frame());
+  EXPECT_EQ(SYN_REPLY, syn_reply->type());
+  EXPECT_EQ(123u, syn_reply->stream_id());
+  EXPECT_EQ(2, syn_reply->header_block_len());
+  EXPECT_EQ(0, syn_reply->flags());
+
+  scoped_ptr<FlipFinStreamControlFrame> fin_frame(
+      framer.CreateFinStream(123, 444));
+  EXPECT_EQ(kFlipProtocolVersion, fin_frame->version());
+  EXPECT_EQ(true, fin_frame->is_control_frame());
+  EXPECT_EQ(FIN_STREAM, fin_frame->type());
+  EXPECT_EQ(123u, fin_frame->stream_id());
+  EXPECT_EQ(444u, fin_frame->status());
+  fin_frame->set_status(555);
+  EXPECT_EQ(555u, fin_frame->status());
+  EXPECT_EQ(0, fin_frame->flags());
+}
+
+TEST(FlipProtocolTest, TestDataFrame) {
+  FlipDataFrame frame;
+
+  // Set the stream ID to various values.
+  frame.set_stream_id(0);
+  EXPECT_EQ(0, frame.stream_id());
+  EXPECT_FALSE(frame.is_control_frame());
+  frame.set_stream_id(~0 & kStreamIdMask);
+  EXPECT_EQ(~0 & kStreamIdMask, frame.stream_id());
+  EXPECT_FALSE(frame.is_control_frame());
+
+  // Set length to various values.  Make sure that when you set_length(x),
+  // length() == x.  Also make sure the flags are unaltered.
+  memset(frame.data(), '1', FlipDataFrame::size());
+  int8 flags = frame.flags();
+  frame.set_length(0);
+  EXPECT_EQ(0, frame.length());
+  EXPECT_EQ(flags, frame.flags());
+  frame.set_length(kLengthMask);
+  EXPECT_EQ(kLengthMask, frame.length());
+  EXPECT_EQ(flags, frame.flags());
+  frame.set_length(5);
+  EXPECT_EQ(5, frame.length());
+  EXPECT_EQ(flags, frame.flags());
+
+  // Set flags to various values.  Make sure that when you set_flags(x),
+  // flags() == x.  Also make sure the length is unaltered.
+  memset(frame.data(), '1', FlipDataFrame::size());
+  uint32 length = frame.length();
+  frame.set_flags(0);
+  EXPECT_EQ(0, frame.flags());
+  EXPECT_EQ(length, frame.length());
+  int8 all_flags = ~0;
+  frame.set_flags(all_flags);
+  flags = frame.flags();
+  EXPECT_EQ(all_flags, flags);
+  EXPECT_EQ(length, frame.length());
+  frame.set_flags(5);
+  EXPECT_EQ(5, frame.flags());
+  EXPECT_EQ(length, frame.length());
+}
+
+// Make sure that overflows both die in debug mode, and do not cause problems
+// in opt mode.  Note:  Chrome doesn't die on DCHECK failures, so the
+// EXPECT_DEBUG_DEATH doesn't work.
+TEST(FlipProtocolDeathTest, TestDataFrame) {
+  FlipDataFrame frame;
+
+  frame.set_stream_id(0);
+#ifndef WIN32
+  EXPECT_DEBUG_DEATH(frame.set_stream_id(~0), "");
+#endif
+  EXPECT_FALSE(frame.is_control_frame());
+
+  frame.set_flags(0);
+#ifndef WIN32
+  EXPECT_DEBUG_DEATH(frame.set_length(~0), "");
+#endif
+  EXPECT_EQ(0, frame.flags());
+}
+
+TEST(FlipProtocolDeathTest, TestFlipControlFrame) {
+  FlipControlFrame frame(FlipControlFrame::size());
+  memset(frame.data(), '1', FlipControlFrame::size());
+
+  // Set the stream ID to various values.
+  frame.set_stream_id(0);
+  EXPECT_EQ(0, frame.stream_id());
+  EXPECT_FALSE(frame.is_control_frame());
+  frame.set_stream_id(~0 & kStreamIdMask);
+  EXPECT_EQ(~0 & kStreamIdMask, frame.stream_id());
+  EXPECT_FALSE(frame.is_control_frame());
+}
+
+}  // namespace
+
diff --git a/net/flip/flip_session.cc b/net/flip/flip_session.cc
index 72bd387..1f594eb 100644
--- a/net/flip/flip_session.cc
+++ b/net/flip/flip_session.cc
@@ -188,13 +188,12 @@ int FlipSession::CreateStream(FlipDelegate* delegate) {
     flags = flip::CONTROL_FLAG_FIN;
 
   // Create a SYN_STREAM packet and add to the output queue.
-  flip::FlipSynStreamControlFrame* syn_frame =
-      flip_framer_.CreateSynStream(stream_id, priority, flags, false, &headers);
-  int length = sizeof(flip::FlipFrame) + syn_frame->length();
-  IOBufferWithSize* buffer =
-      new IOBufferWithSize(length);
-  memcpy(buffer->data(), syn_frame, length);
-  delete[] syn_frame;
+  scoped_ptr<flip::FlipSynStreamControlFrame> syn_frame(
+      flip_framer_.CreateSynStream(stream_id, priority, flags, false,
+                                   &headers));
+  int length = flip::FlipFrame::size() + syn_frame->length();
+  IOBufferWithSize* buffer = new IOBufferWithSize(length);
+  memcpy(buffer->data(), syn_frame->data(), length);
   queue_.push(FlipIOBuffer(buffer, priority, stream));
 
   static StatsCounter flip_requests("flip.requests");
@@ -411,17 +410,16 @@ void FlipSession::WriteSocket() {
 
     // 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);
+    flip::FlipFrame uncompressed_frame(next_buffer.buffer()->data(), false);
+    scoped_ptr<flip::FlipFrame> compressed_frame(
+        flip_framer_.CompressFrame(&uncompressed_frame));
+    size_t size = compressed_frame.get()->length() + flip::FlipFrame::size();
 
     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);
+    memcpy(buffer->data(), compressed_frame->data(), size);
 
     // Attempt to send the frame.
     in_flight_write_ = FlipIOBuffer(buffer, 0, next_buffer.stream());
@@ -645,8 +643,7 @@ void FlipSession::OnControl(const flip::FlipControlFrame* frame) {
   flip::FlipHeaderBlock headers;
   uint32 type = frame->type();
   if (type == flip::SYN_STREAM || type == flip::SYN_REPLY) {
-    if (!flip_framer_.ParseHeaderBlock(
-        reinterpret_cast<const flip::FlipFrame*>(frame), &headers)) {
+    if (!flip_framer_.ParseHeaderBlock(frame, &headers)) {
       LOG(WARNING) << "Could not parse Flip Control Frame Header";
       return;
     }
@@ -698,8 +695,4 @@ void FlipSession::OnFin(const flip::FlipFinStreamControlFrame* frame) {
   }
 }
 
-void FlipSession::OnLameDuck() {
-  NOTIMPLEMENTED();
-}
-
 }  // namespace net
diff --git a/net/flip/flip_session.h b/net/flip/flip_session.h
index a76630d..7c7ad78 100644
--- a/net/flip/flip_session.h
+++ b/net/flip/flip_session.h
@@ -143,7 +143,6 @@ class FlipSession : public base::RefCounted<FlipSession>,
                                  const char* data,
                                  size_t len);
   virtual void OnControl(const flip::FlipControlFrame* frame);
-  virtual void OnLameDuck();
 
   // Control frame handlers.
   void OnSyn(const flip::FlipSynStreamControlFrame* frame,