Revert 31252 - 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/366014

TBR=mbelshe@google.com
Review URL: http://codereview.chromium.org/372028

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

1 files changed, 57 insertions, 205 deletions

diff --git a/net/flip/flip_protocol.h b/net/flip/flip_protocol.h
index 7c0c659..8a585bc 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,22 +116,29 @@ 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
 };
 
-// The basic FLIP Frame.
-struct FlipFrameBlock {
+// 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:
   union {
     struct {
       uint16 version_;
@@ -142,229 +149,74 @@ struct FlipFrameBlock {
     } data_;
   };
   FlagsAndLength flags_length_;
-};
-
-// 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);
-};
+} FlipFrame;
 
 // A Data Frame.
-class FlipDataFrame : public FlipFrame {
- public:
-  FlipDataFrame() : FlipFrame(size()) {}
-  FlipDataFrame(char* data, bool needs_delete)
-      : FlipFrame(data, needs_delete) {}
-  virtual ~FlipDataFrame() {}
-
+typedef struct : public FlipFrame {
   FlipStreamId stream_id() const {
-    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);
+      return ntohl(data_.stream_id_) & kStreamIdMask;
   }
-
-  static size_t size() { return FlipFrame::size(); }
- private:
-  DISALLOW_COPY_AND_ASSIGN(FlipDataFrame);
-};
+  void set_stream_id(FlipStreamId id) { data_.stream_id_ = htonl(id); }
+} FlipDataFrame;
 
 // A Control Frame.
-class FlipControlFrame : public FlipFrame {
- public:
-  explicit FlipControlFrame(size_t size) : FlipFrame(size) {}
-  FlipControlFrame(char* data, bool needs_delete)
-      : FlipFrame(data, needs_delete) {}
-  virtual ~FlipControlFrame() {}
-
+typedef struct : public FlipFrame {
   uint16 version() const {
     const int kVersionMask = 0x7fff;
-    return ntohs(block()->control_.version_) & kVersionMask;
+    return ntohs(control_.version_) & kVersionMask;
   }
   FlipControlType type() const {
-    uint16 type = ntohs(block()->control_.type_);
+    uint16 type = ntohs(control_.type_);
     DCHECK(type >= SYN_STREAM && type <= NOOP);
     return static_cast<FlipControlType>(type);
   }
-  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); }
+  FlipStreamId stream_id() const { return ntohl(stream_id_) & kStreamIdMask; }
 
  private:
-  struct FlipControlFrameBlock* block() const {
-    return static_cast<FlipControlFrameBlock*>(frame_);
-  }
-  DISALLOW_COPY_AND_ASSIGN(FlipControlFrame);
-};
+  FlipStreamId stream_id_;
+} FlipControlFrame;
 
 // A SYN_STREAM frame.
-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; }
-
+typedef struct FlipSynStreamControlFrame : public FlipControlFrame {
+  uint8 priority() const { return (priority_ & kPriorityMask) >> 6; }
   // The number of bytes in the header block beyond the frame header length.
-  int header_block_len() const {
-    return length() - (size() - FlipFrame::size());
-  }
-
+  int header_block_len() const { return length() - kHeaderBlockOffset; }
   const char* header_block() const {
-    return reinterpret_cast<const char*>(block()) + size();
+    return reinterpret_cast<const char*>(this) +
+        sizeof(FlipFrame) + kHeaderBlockOffset;
   }
-
-  static size_t size() { return sizeof(FlipSynStreamControlFrameBlock); }
-
  private:
-  struct FlipSynStreamControlFrameBlock* block() const {
-    return static_cast<FlipSynStreamControlFrameBlock*>(frame_);
-  }
-  DISALLOW_COPY_AND_ASSIGN(FlipSynStreamControlFrame);
-};
+  // Offset from the end of the FlipControlFrame to the FlipHeaderBlock.
+  static const int kHeaderBlockOffset = 6;
+  uint8 priority_;
+  uint8 unused_;
+  // Variable FlipHeaderBlock here.
+} FlipSynStreamControlFrame;
 
 // A SYN_REPLY frame.
-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());
-  }
-
+typedef struct FlipSynReplyControlFrame : public FlipControlFrame {
+  int header_block_len() const { return length() - kHeaderBlockOffset; }
   const char* header_block() const {
-    return reinterpret_cast<const char*>(block()) + size();
+    return reinterpret_cast<const char*>(this) +
+        sizeof(FlipFrame) + kHeaderBlockOffset;
   }
 
-  static size_t size() { return sizeof(FlipSynReplyControlFrameBlock); }
-
  private:
-  struct FlipSynReplyControlFrameBlock* block() const {
-    return static_cast<FlipSynReplyControlFrameBlock*>(frame_);
-  }
-  DISALLOW_COPY_AND_ASSIGN(FlipSynReplyControlFrame);
-};
+  // Offset from the end of the FlipControlFrame to the FlipHeaderBlock.
+  static const int kHeaderBlockOffset = 6;
+  uint16 unused_;
+  // Variable FlipHeaderBlock here.
+} FlipSynReplyControlFrame;
 
 // A FIN_STREAM frame.
-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); }
-
+typedef struct FlipFinStreamControlFrame : public FlipControlFrame {
+  uint32 status() const { return ntohl(status_); }
+  void set_status(int id) { status_ = htonl(id); }
  private:
-  struct FlipFinStreamControlFrameBlock* block() const {
-    return static_cast<FlipFinStreamControlFrameBlock*>(frame_);
-  }
-  DISALLOW_COPY_AND_ASSIGN(FlipFinStreamControlFrame);
-};
+  uint32 status_;
+} FlipFinStreamControlFrame;
 
 }  // namespace flip
 
 #endif  // NET_FLIP_FLIP_PROTOCOL_H_
+