Update HPACK implementation to draft 05

In particular:

 - Switch to 1-based indexing.
 - Use 0 instead of -1 as the "invalid index" value.
 - Add "host" header to the static table.
 - Add HpackEncodingContext::GetMutableEntryCount() function.

This lands server change 60493594 by akalin.

Review URL: https://codereview.chromium.org/145353017

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

net/spdy/hpack_constants.h

@@ -8,7 +8,7 @@
 #include "base/basictypes.h"
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 namespace net {

net/spdy/hpack_encoder.h

@@ -16,7 +16,7 @@
 namespace net {
 
 // An HpackEncoder encodes header sets as outlined in
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 class NET_EXPORT_PRIVATE HpackEncoder {
  public:

net/spdy/hpack_encoding_context.h

@@ -16,7 +16,7 @@
 #include "net/spdy/hpack_header_table.h"
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 namespace net {
@@ -38,9 +38,13 @@ class NET_EXPORT_PRIVATE HpackEncodingContext {
 
   ~HpackEncodingContext();
 
+  uint32 GetMutableEntryCount() const;
+
   uint32 GetEntryCount() const;
 
-  // For all accessor below, index must be < GetEntryCount().
+  // For all read accessors below, index must be >= 1 and <=
+  // GetEntryCount().  For all mutating accessors below, index must be
+  // >= 1 and <= GetMutableEntryCount().
 
   // The StringPieces returned by Get{Name,Value}At() live as long as
   // the next call to SetMaxSize() or the Process*() functions.
@@ -74,23 +78,23 @@ class NET_EXPORT_PRIVATE HpackEncodingContext {
 
   // Tries to update the encoding context given an indexed header
   // opcode for the given index as described in 3.2.1. new_index is
-  // filled in with the index of a mutable entry, or -1 if one was not
+  // filled in with the index of a mutable entry, or 0 if one was not
   // able to be created. removed_referenced_indices is filled in with
   // the indices of all entries removed from the reference set.
   bool ProcessIndexedHeader(uint32 index,
-                            int32* new_index,
+                            uint32* new_index,
                             std::vector<uint32>* removed_referenced_indices);
 
   // Tries to update the encoding context given a literal header with
   // incremental indexing opcode for the given name and value as
   // described in 3.2.1. index is filled in with the index of the new
-  // entry if the header was successfully indexed, or -1 if
+  // entry if the header was successfully indexed, or 0 if
   // not. removed_referenced_indices is filled in with the indices of
   // all entries removed from the reference set.
   bool ProcessLiteralHeaderWithIncrementalIndexing(
       base::StringPiece name,
       base::StringPiece value,
-      int32* index,
+      uint32* index,
       std::vector<uint32>* removed_referenced_indices);
 
  private:

net/spdy/hpack_encoding_context_test.cc

@@ -18,7 +18,7 @@ namespace {
 TEST(HpackEncodingContextTest, IndexedHeaderInvalid) {
   HpackEncodingContext encoding_context;
 
-  int32 new_index = -1;
+  uint32 new_index = 0;
   std::vector<uint32> removed_referenced_indices;
   EXPECT_FALSE(
       encoding_context.ProcessIndexedHeader(kuint32max,
@@ -32,38 +32,42 @@ TEST(HpackEncodingContextTest, IndexedHeaderInvalid) {
 TEST(HpackEncodingContextTest, IndexedHeaderStatic) {
   HpackEncodingContext encoding_context;
 
-  std::string name = encoding_context.GetNameAt(1).as_string();
-  std::string value = encoding_context.GetValueAt(1).as_string();
-  EXPECT_NE(name, encoding_context.GetNameAt(0));
-  EXPECT_NE(value, encoding_context.GetValueAt(0));
+  std::string name = encoding_context.GetNameAt(2).as_string();
+  std::string value = encoding_context.GetValueAt(2).as_string();
+  EXPECT_EQ(0u, encoding_context.GetMutableEntryCount());
+  EXPECT_NE(name, encoding_context.GetNameAt(1));
+  EXPECT_NE(value, encoding_context.GetValueAt(1));
 
   {
-    int32 new_index = -1;
+    uint32 new_index = 0;
     std::vector<uint32> removed_referenced_indices;
     EXPECT_TRUE(
-        encoding_context.ProcessIndexedHeader(1,
+        encoding_context.ProcessIndexedHeader(2,
                                               &new_index,
                                               &removed_referenced_indices));
-    EXPECT_EQ(0, new_index);
+    EXPECT_EQ(1u, new_index);
     EXPECT_TRUE(removed_referenced_indices.empty());
   }
-  EXPECT_EQ(name, encoding_context.GetNameAt(0));
-  EXPECT_EQ(value, encoding_context.GetValueAt(0));
-  EXPECT_TRUE(encoding_context.IsReferencedAt(0));
+  EXPECT_EQ(1u, encoding_context.GetMutableEntryCount());
+  EXPECT_EQ(name, encoding_context.GetNameAt(1));
+  EXPECT_EQ(value, encoding_context.GetValueAt(1));
+  EXPECT_TRUE(encoding_context.IsReferencedAt(1));
 
   {
-    int32 new_index = -1;
+    uint32 new_index = 0;
     std::vector<uint32> removed_referenced_indices;
     EXPECT_TRUE(
-        encoding_context.ProcessIndexedHeader(0,
+        encoding_context.ProcessIndexedHeader(1,
                                               &new_index,
                                               &removed_referenced_indices));
-    EXPECT_EQ(0, new_index);
+    EXPECT_EQ(1u, new_index);
     EXPECT_TRUE(removed_referenced_indices.empty());
   }
-  EXPECT_EQ(name, encoding_context.GetNameAt(0));
-  EXPECT_EQ(value, encoding_context.GetValueAt(0));
-  EXPECT_FALSE(encoding_context.IsReferencedAt(0));
+  EXPECT_EQ(1u, encoding_context.GetMutableEntryCount());
+  EXPECT_EQ(name, encoding_context.GetNameAt(1));
+  EXPECT_EQ(value, encoding_context.GetValueAt(1));
+  EXPECT_LE(1u, encoding_context.GetMutableEntryCount());
+  EXPECT_FALSE(encoding_context.IsReferencedAt(1));
 }
 
 // Try to process an indexed header with an index for a static header
@@ -73,14 +77,15 @@ TEST(HpackEncodingContextTest, IndexedHeaderStaticCopyDoesNotFit) {
   HpackEncodingContext encoding_context;
   encoding_context.SetMaxSize(0);
 
-  int32 new_index = -1;
+  uint32 new_index = 0;
   std::vector<uint32> removed_referenced_indices;
   EXPECT_TRUE(
       encoding_context.ProcessIndexedHeader(1,
                                             &new_index,
                                             &removed_referenced_indices));
-  EXPECT_EQ(-1, new_index);
+  EXPECT_EQ(0u, new_index);
   EXPECT_TRUE(removed_referenced_indices.empty());
+  EXPECT_EQ(0u, encoding_context.GetMutableEntryCount());
 }
 
 // NOTE: It's too onerous to try to test invalid input to
@@ -92,16 +97,17 @@ TEST(HpackEncodingContextTest, IndexedHeaderStaticCopyDoesNotFit) {
 TEST(HpackEncodingContextTest, LiteralHeaderIncrementalIndexing) {
   HpackEncodingContext encoding_context;
 
-  int32 index = -1;
+  uint32 index = 0;
   std::vector<uint32> removed_referenced_indices;
   EXPECT_TRUE(
       encoding_context.ProcessLiteralHeaderWithIncrementalIndexing(
           "name", "value", &index, &removed_referenced_indices));
-  EXPECT_EQ(0, index);
+  EXPECT_EQ(1u, index);
   EXPECT_TRUE(removed_referenced_indices.empty());
-  EXPECT_EQ("name", encoding_context.GetNameAt(0).as_string());
-  EXPECT_EQ("value", encoding_context.GetValueAt(0).as_string());
-  EXPECT_TRUE(encoding_context.IsReferencedAt(0));
+  EXPECT_EQ("name", encoding_context.GetNameAt(1).as_string());
+  EXPECT_EQ("value", encoding_context.GetValueAt(1).as_string());
+  EXPECT_TRUE(encoding_context.IsReferencedAt(1));
+  EXPECT_EQ(1u, encoding_context.GetMutableEntryCount());
 }
 
 // Try to process a literal header with incremental indexing that is
@@ -111,13 +117,14 @@ TEST(HpackEncodingContextTest, LiteralHeaderIncrementalIndexingDoesNotFit) {
   HpackEncodingContext encoding_context;
   encoding_context.SetMaxSize(0);
 
-  int32 index = -1;
+  uint32 index = 0;
   std::vector<uint32> removed_referenced_indices;
   EXPECT_TRUE(
       encoding_context.ProcessLiteralHeaderWithIncrementalIndexing(
           "name", "value", &index, &removed_referenced_indices));
-  EXPECT_EQ(-1, index);
+  EXPECT_EQ(0u, index);
   EXPECT_TRUE(removed_referenced_indices.empty());
+  EXPECT_EQ(0u, encoding_context.GetMutableEntryCount());
 }
 
 }  // namespace

net/spdy/hpack_entry.h

@@ -18,7 +18,7 @@
 namespace net {
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 // A structure for an entry in the header table (3.1.2) and the

net/spdy/hpack_header_table.cc

@@ -20,13 +20,15 @@ uint32 HpackHeaderTable::GetEntryCount() const {
 }
 
 const HpackEntry& HpackHeaderTable::GetEntry(uint32 index) const {
-  CHECK_LT(index, GetEntryCount());
-  return entries_[index];
+  CHECK_GE(index, 1u);
+  CHECK_LE(index, GetEntryCount());
+  return entries_[index-1];
 }
 
 HpackEntry* HpackHeaderTable::GetMutableEntry(uint32 index) {
-  CHECK_LT(index, GetEntryCount());
-  return &entries_[index];
+  CHECK_GE(index, 1u);
+  CHECK_LE(index, GetEntryCount());
+  return &entries_[index-1];
 }
 
 void HpackHeaderTable::SetMaxSize(uint32 max_size) {
@@ -40,9 +42,9 @@ void HpackHeaderTable::SetMaxSize(uint32 max_size) {
 
 void HpackHeaderTable::TryAddEntry(
     const HpackEntry& entry,
-    int32* index,
+    uint32* index,
     std::vector<uint32>* removed_referenced_indices) {
-  *index = -1;
+  *index = 0;
   removed_referenced_indices->clear();
 
   // The algorithm used here is described in 3.3.3. We're assuming
@@ -53,9 +55,10 @@ void HpackHeaderTable::TryAddEntry(
     // The conditional implies the difference can fit in 32 bits.
     target_size = size_t_max_size - entry.Size();
   }
-  while ((static_cast<size_t>(size_) > target_size) && !entries_.empty()) {
+  while (static_cast<size_t>(size_) > target_size) {
+    DCHECK(!entries_.empty());
     if (entries_.back().IsReferenced()) {
-      removed_referenced_indices->push_back(entries_.size() - 1);
+      removed_referenced_indices->push_back(entries_.size());
     }
     size_ -= entries_.back().Size();
     entries_.pop_back();
@@ -66,7 +69,7 @@ void HpackHeaderTable::TryAddEntry(
     // condition of the if.
     DCHECK_LE(static_cast<size_t>(size_) + entry.Size(), size_t_max_size);
     size_ += entry.Size();
-    *index = 0;
+    *index = 1;
     entries_.push_front(entry);
   }
 }

net/spdy/hpack_header_table.h

@@ -17,7 +17,7 @@
 namespace net {
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 // A data structure for both the header table (described in 3.1.2) and
@@ -36,10 +36,10 @@ class NET_EXPORT_PRIVATE HpackHeaderTable {
   // Returns the total number of entries.
   uint32 GetEntryCount() const;
 
-  // The given index must be >= 0 and < GetEntryCount().
+  // The given index must be >= 1 and <= GetEntryCount().
   const HpackEntry& GetEntry(uint32 index) const;
 
-  // The given index must be >= 0 and < GetEntryCount().
+  // The given index must be >= 1 and <= GetEntryCount().
   HpackEntry* GetMutableEntry(uint32 index);
 
   // Sets the maximum size of the header table, evicting entries if
@@ -49,12 +49,12 @@ class NET_EXPORT_PRIVATE HpackHeaderTable {
   // The given entry must not be one from the header table, since it
   // may get evicted. Tries to add the given entry to the header
   // table, evicting entries if necessary as described in 3.3.3. index
-  // will be filled in with the index of the added entry, or -1 if the
+  // will be filled in with the index of the added entry, or 0 if the
   // entry could not be added. removed_referenced_indices will be
   // filled in with the indices of any removed entries that were in
   // the reference set.
   void TryAddEntry(const HpackEntry& entry,
-                   int32* index,
+                   uint32* index,
                    std::vector<uint32>* removed_referenced_indices);
 
  private:

net/spdy/hpack_header_table_test.cc

@@ -55,10 +55,10 @@ void AddEntriesExpectNoEviction(const HpackEntryVector& entries,
   unsigned start_entry_count = header_table->GetEntryCount();
   for (HpackEntryVector::const_iterator it = entries.begin();
        it != entries.end(); ++it) {
-    int32 index = -1;
+    uint32 index = 0;
     std::vector<uint32> removed_referenced_indices;
     header_table->TryAddEntry(*it, &index, &removed_referenced_indices);
-    EXPECT_EQ(0, index);
+    EXPECT_EQ(1u, index);
     EXPECT_TRUE(removed_referenced_indices.empty());
     EXPECT_EQ(start_entry_count + (it - entries.begin()) + 1u,
               header_table->GetEntryCount());
@@ -66,7 +66,7 @@ void AddEntriesExpectNoEviction(const HpackEntryVector& entries,
 
   for (HpackEntryVector::const_iterator it = entries.begin();
        it != entries.end(); ++it) {
-    uint32 index = header_table->GetEntryCount() - (it - entries.begin()) - 1;
+    uint32 index = header_table->GetEntryCount() - (it - entries.begin());
     HpackEntry entry = header_table->GetEntry(index);
     EXPECT_TRUE(it->Equals(entry))
         << "it = " << it->GetDebugString() << " != entry = "
@@ -78,7 +78,7 @@ void AddEntriesExpectNoEviction(const HpackEntryVector& entries,
 // table's reference set.
 std::set<uint32> GetReferenceSet(const HpackHeaderTable& header_table) {
   std::set<uint32> reference_set;
-  for (uint32 i = 0; i < header_table.GetEntryCount(); ++i) {
+  for (uint32 i = 1; i <= header_table.GetEntryCount(); ++i) {
     if (header_table.GetEntry(i).IsReferenced()) {
       reference_set.insert(i);
     }
@@ -136,7 +136,7 @@ TEST(HpackHeaderTableTest, SetMaxSizeZeroClearsReferenceSet) {
   AddEntriesExpectNoEviction(entries, &header_table);
 
   std::set<uint32> expected_reference_set;
-  for (uint32 i = 0; i < header_table.GetEntryCount(); ++i) {
+  for (uint32 i = 1; i <= header_table.GetEntryCount(); ++i) {
     header_table.GetMutableEntry(i)->SetReferenced(true);
     expected_reference_set.insert(i);
   }
@@ -157,7 +157,7 @@ TEST(HpackHeaderTableTest, TryAddEntryEviction) {
   AddEntriesExpectNoEviction(entries, &header_table);
 
   EXPECT_EQ(entries.size(), header_table.GetEntryCount());
-  HpackEntry first_entry = header_table.GetEntry(0);
+  HpackEntry first_entry = header_table.GetEntry(1);
   HpackEntry long_entry =
       MakeEntryOfSize(header_table.size() - first_entry.Size());
 
@@ -165,24 +165,24 @@ TEST(HpackHeaderTableTest, TryAddEntryEviction) {
   EXPECT_EQ(entries.size(), header_table.GetEntryCount());
 
   std::set<uint32> expected_reference_set;
-  for (uint32 i = 1; i < header_table.GetEntryCount(); ++i) {
+  for (uint32 i = 2; i <= header_table.GetEntryCount(); ++i) {
     header_table.GetMutableEntry(i)->SetReferenced(true);
     expected_reference_set.insert(i);
   }
   EXPECT_EQ(expected_reference_set, GetReferenceSet(header_table));
 
-  int32 index = -1;
+  uint32 index = 0;
   std::vector<uint32> removed_referenced_indices;
   header_table.TryAddEntry(long_entry, &index, &removed_referenced_indices);
 
-  EXPECT_EQ(0, index);
+  EXPECT_EQ(1u, index);
   EXPECT_EQ(expected_reference_set,
             std::set<uint32>(removed_referenced_indices.begin(),
                              removed_referenced_indices.end()));
   EXPECT_TRUE(GetReferenceSet(header_table).empty());
   EXPECT_EQ(2u, header_table.GetEntryCount());
-  EXPECT_TRUE(header_table.GetEntry(0).Equals(long_entry));
-  EXPECT_TRUE(header_table.GetEntry(1).Equals(first_entry));
+  EXPECT_TRUE(header_table.GetEntry(1).Equals(long_entry));
+  EXPECT_TRUE(header_table.GetEntry(2).Equals(first_entry));
 }
 
 // Fill a header table with entries, and then add an entry bigger than
@@ -197,17 +197,17 @@ TEST(HpackHeaderTableTest, TryAddTooLargeEntry) {
   EXPECT_EQ(entries.size(), header_table.GetEntryCount());
 
   std::set<uint32> expected_removed_referenced_indices;
-  for (uint32 i = 0; i < header_table.GetEntryCount(); ++i) {
+  for (uint32 i = 1; i <= header_table.GetEntryCount(); ++i) {
     header_table.GetMutableEntry(i)->SetReferenced(true);
     expected_removed_referenced_indices.insert(i);
   }
 
   HpackEntry long_entry = MakeEntryOfSize(header_table.size() + 1);
-  int32 index = -1;
+  uint32 index = 0;
   std::vector<uint32> removed_referenced_indices;
   header_table.TryAddEntry(long_entry, &index, &removed_referenced_indices);
 
-  EXPECT_EQ(-1, index);
+  EXPECT_EQ(0u, index);
   EXPECT_EQ(expected_removed_referenced_indices,
             std::set<uint32>(removed_referenced_indices.begin(),
                              removed_referenced_indices.end()));

net/spdy/hpack_output_stream.h

@@ -15,7 +15,7 @@
 #include "net/spdy/hpack_encoding_context.h"
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 namespace net {

net/spdy/hpack_string_util.h

@@ -11,7 +11,7 @@
 namespace net {
 
 // All section references below are to
-// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04
+// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05
 // .
 
 // A constant-time StringPiece comparison function, as suggested