Take3:

Rework the FlipProtocol to separate the Clike 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 selfclean 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/377014

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

8 files changed, 654 insertions, 362 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..e1760a4 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
@@ -662,12 +670,12 @@ FlipFrame* FlipFramer::CompressFrame(const FlipFrame* frame) {
   int rv = deflate(compressor_.get(), Z_SYNC_FLUSH);
   if (rv != Z_OK) {  // How can we know that it compressed everything?
     // This shouldn't happen, right?
-    free(new_frame);
+    delete new_frame;
     return NULL;
   }
 
   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;
 
@@ -726,7 +733,7 @@ FlipFrame* FlipFramer::DecompressFrame(const FlipFrame* frame) {
     }
   }
   if (rv != Z_OK) {  // How can we know that it decompressed everything?
-    free(new_frame);
+    delete new_frame;
     return NULL;
   }
 
@@ -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,