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

1 files changed, 180 insertions, 173 deletions

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) {