This change simplifies SpdyHeaderBlock and SpdyHeaderStorage, now that the underlying arena implementation does not allocate memory upon construction.

gfe-relnote: Code simplifications to classes in //third_party/spdy, no functional change; not protected.
PiperOrigin-RevId: 284805587
Change-Id: I3b0cbdda821c1f2ea89b9f880d76a3132490b196

spdy/core/spdy_header_block.cc

diff --git a/spdy/core/spdy_header_block.cc b/spdy/core/spdy_header_block.cc
index 9ccc627..609f6d7 100644
--- a/spdy/core/spdy_header_block.cc
+++ b/spdy/core/spdy_header_block.cc
@@ -12,7 +12,6 @@
 #include "net/third_party/quiche/src/spdy/platform/api/spdy_estimate_memory_usage.h"
 #include "net/third_party/quiche/src/spdy/platform/api/spdy_logging.h"
 #include "net/third_party/quiche/src/spdy/platform/api/spdy_string_utils.h"
-#include "net/third_party/quiche/src/spdy/platform/api/spdy_unsafe_arena.h"
 
 namespace spdy {
 namespace {
@@ -65,6 +64,10 @@
   return *this;
 }
 
+void SpdyHeaderBlock::HeaderValue::set_storage(SpdyHeaderStorage* storage) {
+  storage_ = storage;
+}
+
 SpdyHeaderBlock::HeaderValue::~HeaderValue() = default;
 
 SpdyStringPiece SpdyHeaderBlock::HeaderValue::ConsolidatedValue() const {
@@ -132,14 +135,14 @@
   // can reclaim the memory used by the key. This makes lookup-only access to
   // SpdyHeaderBlock through operator[] memory-neutral.
   if (valid_ && lookup_result_ == block_->map_.end()) {
-    block_->GetStorage()->Rewind(key_);
+    block_->storage_.Rewind(key_);
   }
 }
 
 SpdyHeaderBlock::ValueProxy& SpdyHeaderBlock::ValueProxy::operator=(
     const SpdyStringPiece value) {
   *spdy_header_block_value_size_ += value.size();
-  SpdyHeaderStorage* storage = block_->GetStorage();
+  SpdyHeaderStorage* storage = &block_->storage_;
   if (lookup_result_ == block_->map_.end()) {
     SPDY_DVLOG(1) << "Inserting: (" << key_ << ", " << value << ")";
     lookup_result_ =
@@ -168,7 +171,10 @@
 SpdyHeaderBlock::SpdyHeaderBlock(SpdyHeaderBlock&& other)
     : map_(kInitialMapBuckets) {
   map_.swap(other.map_);
-  storage_.swap(other.storage_);
+  storage_ = std::move(other.storage_);
+  for (auto& p : map_) {
+    p.second.set_storage(&storage_);
+  }
   key_size_ = other.key_size_;
   value_size_ = other.value_size_;
 }
@@ -177,7 +183,10 @@
 
 SpdyHeaderBlock& SpdyHeaderBlock::operator=(SpdyHeaderBlock&& other) {
   map_.swap(other.map_);
-  storage_.swap(other.storage_);
+  storage_ = std::move(other.storage_);
+  for (auto& p : map_) {
+    p.second.set_storage(&storage_);
+  }
   key_size_ = other.key_size_;
   value_size_ = other.value_size_;
   return *this;
@@ -226,7 +235,7 @@
   key_size_ = 0;
   value_size_ = 0;
   map_.clear();
-  storage_.reset();
+  storage_.Clear();
 }
 
 void SpdyHeaderBlock::insert(const SpdyHeaderBlock::value_type& value) {
@@ -242,9 +251,8 @@
     SPDY_DVLOG(1) << "Updating key: " << iter->first
                   << " with value: " << value.second;
     value_size_ -= iter->second.SizeEstimate();
-    SpdyHeaderStorage* storage = GetStorage();
     iter->second =
-        HeaderValue(storage, iter->first, storage->Write(value.second));
+        HeaderValue(&storage_, iter->first, storage_.Write(value.second));
   }
 }
 
@@ -280,7 +288,7 @@
   SPDY_DVLOG(1) << "Updating key: " << iter->first
                 << "; appending value: " << value;
   value_size_ += SeparatorForKey(key).size();
-  iter->second.Append(GetStorage()->Write(value));
+  iter->second.Append(storage_.Write(value));
 }
 
 size_t SpdyHeaderBlock::EstimateMemoryUsage() const {
@@ -292,29 +300,17 @@
 void SpdyHeaderBlock::AppendHeader(const SpdyStringPiece key,
                                    const SpdyStringPiece value) {
   auto backed_key = WriteKey(key);
-  SpdyHeaderStorage* storage = GetStorage();
   map_.emplace(std::make_pair(
-      backed_key, HeaderValue(storage, backed_key, storage->Write(value))));
-}
-
-SpdyHeaderStorage* SpdyHeaderBlock::GetStorage() {
-  if (storage_ == nullptr) {
-    storage_ = std::make_unique<SpdyHeaderStorage>();
-  }
-  return storage_.get();
+      backed_key, HeaderValue(&storage_, backed_key, storage_.Write(value))));
 }
 
 SpdyStringPiece SpdyHeaderBlock::WriteKey(const SpdyStringPiece key) {
   key_size_ += key.size();
-  return GetStorage()->Write(key);
+  return storage_.Write(key);
 }
 
 size_t SpdyHeaderBlock::bytes_allocated() const {
-  if (storage_ == nullptr) {
-    return 0;
-  } else {
-    return storage_->bytes_allocated();
-  }
+  return storage_.bytes_allocated();
 }
 
 }  // namespace spdy

spdy/core/spdy_header_block.h

diff --git a/spdy/core/spdy_header_block.h b/spdy/core/spdy_header_block.h
index ff5f4c9..bd575f4 100644
--- a/spdy/core/spdy_header_block.h
+++ b/spdy/core/spdy_header_block.h
@@ -8,7 +8,6 @@
 #include <stddef.h>
 
 #include <list>
-#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -51,6 +50,8 @@
     HeaderValue(HeaderValue&& other);
     HeaderValue& operator=(HeaderValue&& other);
 
+    void set_storage(SpdyHeaderStorage* storage);
+
     // Copies are not.
     HeaderValue(const HeaderValue& other) = delete;
     HeaderValue& operator=(const HeaderValue& other) = delete;
@@ -225,14 +226,12 @@
   friend class test::SpdyHeaderBlockPeer;
 
   void AppendHeader(const SpdyStringPiece key, const SpdyStringPiece value);
-  SpdyHeaderStorage* GetStorage();
   SpdyStringPiece WriteKey(const SpdyStringPiece key);
   size_t bytes_allocated() const;
 
-  // SpdyStringPieces held by |map_| point to memory owned by |*storage_|.
-  // |storage_| might be nullptr as long as |map_| is empty.
+  // SpdyStringPieces held by |map_| point to memory owned by |storage_|.
   MapType map_;
-  std::unique_ptr<SpdyHeaderStorage> storage_;
+  SpdyHeaderStorage storage_;
 
   size_t key_size_ = 0;
   size_t value_size_ = 0;

spdy/core/spdy_header_block_test.cc

diff --git a/spdy/core/spdy_header_block_test.cc b/spdy/core/spdy_header_block_test.cc
index a744a3b..774854b 100644
--- a/spdy/core/spdy_header_block_test.cc
+++ b/spdy/core/spdy_header_block_test.cc
@@ -118,6 +118,13 @@
   EXPECT_NE(block1, block2);
 }
 
+SpdyHeaderBlock ReturnTestHeaderBlock() {
+  SpdyHeaderBlock block;
+  block["foo"] = "bar";
+  block.insert(std::make_pair("foo2", "baz"));
+  return block;
+}
+
 // Test that certain methods do not crash on moved-from instances.
 TEST(SpdyHeaderBlockTest, MovedFromIsValid) {
   SpdyHeaderBlock block1;
@@ -139,6 +146,11 @@
 
   block1["foo"] = "bar";
   EXPECT_THAT(block1, ElementsAre(Pair("foo", "bar")));
+
+  SpdyHeaderBlock block5 = ReturnTestHeaderBlock();
+  block5.AppendValueOrAddHeader("foo", "bar2");
+  EXPECT_THAT(block5, ElementsAre(Pair("foo", std::string("bar\0bar2", 8)),
+                                  Pair("foo2", "baz")));
 }
 
 // This test verifies that headers can be appended to no matter how they were

spdy/core/spdy_header_storage.h

diff --git a/spdy/core/spdy_header_storage.h b/spdy/core/spdy_header_storage.h
index fd31555..fc559ec 100644
--- a/spdy/core/spdy_header_storage.h
+++ b/spdy/core/spdy_header_storage.h
@@ -23,6 +23,9 @@
   SpdyHeaderStorage(const SpdyHeaderStorage&) = delete;
   SpdyHeaderStorage& operator=(const SpdyHeaderStorage&) = delete;
 
+  SpdyHeaderStorage(SpdyHeaderStorage&& other) = default;
+  SpdyHeaderStorage& operator=(SpdyHeaderStorage&& other) = default;
+
   SpdyStringPiece Write(SpdyStringPiece s);
 
   // If |s| points to the most recent allocation from arena_, the arena will