Several improvements for Structured Header Dictionary

- Add Dictionary::at(StringPiece), which allows looking up items on a
  constant Dictionary object or reference. Also add at(size_t) and const
  variants for indexed accesses.
- Always initialize all fields. Currently the
  ParameterizedMember::member_is_inner_list is left uninitialized by its
  default constructor.

Bug: 1061139
Change-Id: Id3e523e0948ab1e93a917c89b6cc62955cc07bb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2103397
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752349}

net/http/structured_headers.cc

@@ -11,6 +11,7 @@
 #include "base/base64.h"
 #include "base/containers/flat_set.h"
 #include "base/containers/span.h"
+#include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 
@@ -828,19 +829,46 @@ std::vector<DictionaryMember>::const_iterator Dictionary::end() const {
 ParameterizedMember& Dictionary::operator[](std::size_t idx) {
   return members_[idx].second;
 }
+const ParameterizedMember& Dictionary::operator[](std::size_t idx) const {
+  return members_[idx].second;
+}
+ParameterizedMember& Dictionary::at(std::size_t idx) {
+  return (*this)[idx];
+}
+const ParameterizedMember& Dictionary::at(std::size_t idx) const {
+  return (*this)[idx];
+}
 ParameterizedMember& Dictionary::operator[](base::StringPiece key) {
-  for (auto& member : members_) {
-    if (member.first == key)
-      return member.second;
-  }
+  auto it =
+      std::find_if(members_.begin(), members_.end(),
+                   [key](const auto& member) { return member.first == key; });
+  if (it != members_.end())
+    return it->second;
   return (*(members_.insert(members_.end(), make_pair(std::string(key),
                                                       ParameterizedMember()))))
       .second;
 }
-std::size_t Dictionary::size() {
+ParameterizedMember& Dictionary::at(base::StringPiece key) {
+  auto it =
+      std::find_if(members_.begin(), members_.end(),
+                   [key](const auto& member) { return member.first == key; });
+  DCHECK(it != members_.end()) << "Provided key not found in dictionary";
+  return it->second;
+}
+const ParameterizedMember& Dictionary::at(base::StringPiece key) const {
+  auto it =
+      std::find_if(members_.begin(), members_.end(),
+                   [key](const auto& member) { return member.first == key; });
+  DCHECK(it != members_.end()) << "Provided key not found in dictionary";
+  return it->second;
+}
+bool Dictionary::empty() const {
+  return members_.empty();
+}
+std::size_t Dictionary::size() const {
   return members_.size();
 }
-bool Dictionary::contains(base::StringPiece key) {
+bool Dictionary::contains(base::StringPiece key) const {
   for (auto& member : members_) {
     if (member.first == key)
       return true;

net/http/structured_headers.h

@@ -164,7 +164,7 @@ inline bool operator!=(const ParameterizedItem& lhs,
 struct NET_EXPORT ParameterizedMember {
   std::vector<ParameterizedItem> member;
   // If false, then |member| should only hold one Item.
-  bool member_is_inner_list;
+  bool member_is_inner_list = false;
 
   Parameters params;
 
@@ -204,10 +204,24 @@ class NET_EXPORT Dictionary {
   const_iterator begin() const;
   iterator end();
   const_iterator end() const;
+
+  // operator[](size_t) and at(size_t) will both abort the program in case of
+  // out of bounds access.
   ParameterizedMember& operator[](std::size_t idx);
+  const ParameterizedMember& operator[](std::size_t idx) const;
+  ParameterizedMember& at(std::size_t idx);
+  const ParameterizedMember& at(std::size_t idx) const;
+
+  // Consistent with std::map, if |key| does not exist in the Dictionary, then
+  // operator[](base::StringPiece) will create an entry for it, but at() will
+  // abort the entire program.
   ParameterizedMember& operator[](base::StringPiece key);
-  std::size_t size();
-  bool contains(base::StringPiece key);
+  ParameterizedMember& at(base::StringPiece key);
+  const ParameterizedMember& at(base::StringPiece key) const;
+
+  bool empty() const;
+  std::size_t size() const;
+  bool contains(base::StringPiece key) const;
   friend bool operator==(const Dictionary& lhs, const Dictionary& rhs);
   friend bool operator!=(const Dictionary& lhs, const Dictionary& rhs);
 

net/http/structured_headers_unittest.cc

@@ -1132,6 +1132,71 @@ TEST(StructuredHeaderTest, SerializeDictionary) {
   }
 }
 
+TEST(StructuredHeaderTest, DictionaryConstructors) {
+  const std::string key0 = "key0";
+  const std::string key1 = "key1";
+  const ParameterizedMember member0{Item("Applepie"), {}};
+  const ParameterizedMember member1{Item("hello", Item::kByteSequenceType), {}};
+
+  Dictionary dict;
+  EXPECT_TRUE(dict.empty());
+  EXPECT_EQ(0U, dict.size());
+  dict[key0] = member0;
+  EXPECT_FALSE(dict.empty());
+  EXPECT_EQ(1U, dict.size());
+
+  const Dictionary dict_copy = dict;
+  EXPECT_FALSE(dict_copy.empty());
+  EXPECT_EQ(1U, dict_copy.size());
+  EXPECT_EQ(dict, dict_copy);
+
+  const Dictionary dict_init{{{key0, member0}, {key1, member1}}};
+  EXPECT_FALSE(dict_init.empty());
+  EXPECT_EQ(2U, dict_init.size());
+  EXPECT_EQ(member0, dict_init.at(key0));
+  EXPECT_EQ(member1, dict_init.at(key1));
+}
+
+TEST(StructuredHeaderTest, DictionaryAccessors) {
+  const std::string key0 = "key0";
+  const std::string key1 = "key1";
+
+  const ParameterizedMember nonempty_member0{Item("Applepie"), {}};
+  const ParameterizedMember nonempty_member1{
+      Item("hello", Item::kByteSequenceType), {}};
+  const ParameterizedMember empty_member;
+
+  Dictionary dict{{{key0, nonempty_member0}}};
+  EXPECT_TRUE(dict.contains(key0));
+  EXPECT_EQ(nonempty_member0, dict[key0]);
+  EXPECT_EQ(&dict[key0], &dict.at(key0));
+  EXPECT_EQ(&dict[key0], &dict[0]);
+  EXPECT_EQ(&dict[key0], &dict.at(0));
+
+  // Even if the key does not yet exist in |dict|, operator[]() should
+  // automatically create an empty entry.
+  ASSERT_FALSE(dict.contains(key1));
+  ParameterizedMember& member1 = dict[key1];
+  EXPECT_TRUE(dict.contains(key1));
+  EXPECT_EQ(empty_member, member1);
+  EXPECT_EQ(&member1, &dict[key1]);
+  EXPECT_EQ(&member1, &dict.at(key1));
+  EXPECT_EQ(&member1, &dict[1]);
+  EXPECT_EQ(&member1, &dict.at(1));
+
+  member1 = nonempty_member1;
+  EXPECT_EQ(nonempty_member1, dict[key1]);
+  EXPECT_EQ(&dict[key1], &dict.at(key1));
+  EXPECT_EQ(&dict[key1], &dict[1]);
+  EXPECT_EQ(&dict[key1], &dict.at(1));
+
+  // at(StringPiece) and indexed accessors have const overloads.
+  const Dictionary& dict_ref = dict;
+  EXPECT_EQ(&member1, &dict_ref.at(key1));
+  EXPECT_EQ(&member1, &dict_ref[1]);
+  EXPECT_EQ(&member1, &dict_ref.at(1));
+}
+
 TEST(StructuredHeaderTest, UnserializableDictionary) {
   static const struct UnserializableDictionary {
     const char* name;