Merge SettingsMap changes from server code

Review URL: http://codereview.chromium.org/10254021

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

4 files changed, 90 insertions, 8 deletions

diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index ee0ee22..e7f33d756 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -989,7 +989,7 @@ bool SpdyFramer::ParseSettings(const SpdySettingsControlFrame* frame,
     SpdySettingsIds id = static_cast<SpdySettingsIds>(flags_and_id.id());
     SpdySettingsFlags flags =
         static_cast<SpdySettingsFlags>(flags_and_id.flags());
-    settings->insert(std::make_pair(id, SettingsFlagsAndValue(flags, value)));
+    (*settings)[id] = SettingsFlagsAndValue(flags, value);
   }
   return true;
 }
@@ -1113,13 +1113,13 @@ SpdySettingsControlFrame* SpdyFramer::CreateSettings(
   SpdyFrameBuilder frame(SETTINGS, CONTROL_FLAG_NONE, spdy_version_,
                          frame_size);
   frame.WriteUInt32(values.size());
-  SettingsMap::const_iterator it = values.begin();
-  while (it != values.end()) {
+  for (SettingsMap::const_iterator it = values.begin();
+       it != values.end();
+       it++) {
     SettingsFlagsAndId flags_and_id(it->second.first, it->first);
     uint32 id_and_flags_wire = flags_and_id.GetWireFormat(spdy_version_);
     frame.WriteBytes(&id_and_flags_wire, 4);
     frame.WriteUInt32(it->second.second);
-    ++it;
   }
   DCHECK_EQ(frame.length(), frame_size);
   return reinterpret_cast<SpdySettingsControlFrame*>(frame.take());
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index e1ff855..690e848 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -71,7 +71,7 @@ class NET_EXPORT_PRIVATE SettingsFlagsAndId {
   uint32 id_;
 };
 
-// SpdySettingsMap has unique (flags, value) pair for given SpdySettingsIds ID.
+// SettingsMap has unique (flags, value) pair for given SpdySettingsIds ID.
 typedef std::pair<SpdySettingsFlags, uint32> SettingsFlagsAndValue;
 typedef std::map<SpdySettingsIds, SettingsFlagsAndValue> SettingsMap;
 
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index 494391d..fd80da4 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -524,6 +524,7 @@ class SpdyFramerTest
     SpdySettingsIds id = static_cast<SpdySettingsIds>(flags_and_id.id());
     SpdySettingsFlags flags =
         static_cast<SpdySettingsFlags>(flags_and_id.flags());
+    CHECK(settings->find(id) == settings->end());
     settings->insert(std::make_pair(id, SettingsFlagsAndValue(flags, value)));
   }
 
@@ -2577,6 +2578,86 @@ TEST_P(SpdyFramerTest, ReadLargeSettingsFrame) {
   EXPECT_EQ(2, visitor.settings_frame_count_);
 }
 
+// Tests handling of SETTINGS frame with duplicate entries.
+TEST_P(SpdyFramerTest, ReadDuplicateSettings) {
+  SpdyFramer framer(spdy_version_);
+
+  const unsigned char kV2FrameData[] = {
+    0x80, spdy_version_, 0x00, 0x04,
+    0x00, 0x00, 0x00, 0x1C,
+    0x00, 0x00, 0x00, 0x03,
+    0x01, 0x00, 0x00, 0x00,  // 1st Setting
+    0x00, 0x00, 0x00, 0x02,
+    0x01, 0x00, 0x00, 0x00,  // 2nd (duplicate) Setting
+    0x00, 0x00, 0x00, 0x03,
+    0x03, 0x00, 0x00, 0x00,  // 3rd (unprocessed) Setting
+    0x00, 0x00, 0x00, 0x03,
+  };
+
+  const unsigned char kV3FrameData[] = {
+    0x80, spdy_version_, 0x00, 0x04,
+    0x00, 0x00, 0x00, 0x1C,
+    0x00, 0x00, 0x00, 0x03,
+    0x00, 0x00, 0x00, 0x01,  // 1st Setting
+    0x00, 0x00, 0x00, 0x02,
+    0x00, 0x00, 0x00, 0x01,  // 2nd (duplicate) Setting
+    0x00, 0x00, 0x00, 0x03,
+    0x00, 0x00, 0x00, 0x03,  // 3rd (unprocessed) Setting
+    0x00, 0x00, 0x00, 0x03,
+  };
+
+  TestSpdyVisitor visitor(spdy_version_);
+  visitor.use_compression_ = false;
+  if (IsSpdy2()) {
+    visitor.SimulateInFramer(kV2FrameData, sizeof(kV2FrameData));
+  } else {
+    visitor.SimulateInFramer(kV3FrameData, sizeof(kV3FrameData));
+  }
+  EXPECT_EQ(1, visitor.error_count_);
+  EXPECT_EQ(1, visitor.setting_count_);
+  EXPECT_EQ(1, visitor.settings_frame_count_);
+}
+
+// Tests handling of SETTINGS frame with entries out of order.
+TEST_P(SpdyFramerTest, ReadOutOfOrderSettings) {
+  SpdyFramer framer(spdy_version_);
+
+  const unsigned char kV2FrameData[] = {
+    0x80, spdy_version_, 0x00, 0x04,
+    0x00, 0x00, 0x00, 0x1C,
+    0x00, 0x00, 0x00, 0x03,
+    0x02, 0x00, 0x00, 0x00,  // 1st Setting
+    0x00, 0x00, 0x00, 0x02,
+    0x01, 0x00, 0x00, 0x00,  // 2nd (out of order) Setting
+    0x00, 0x00, 0x00, 0x03,
+    0x03, 0x00, 0x00, 0x00,  // 3rd (unprocessed) Setting
+    0x00, 0x00, 0x00, 0x03,
+  };
+
+  const unsigned char kV3FrameData[] = {
+    0x80, spdy_version_, 0x00, 0x04,
+    0x00, 0x00, 0x00, 0x1C,
+    0x00, 0x00, 0x00, 0x03,
+    0x00, 0x00, 0x00, 0x02,  // 1st Setting
+    0x00, 0x00, 0x00, 0x02,
+    0x00, 0x00, 0x00, 0x01,  // 2nd (out of order) Setting
+    0x00, 0x00, 0x00, 0x03,
+    0x00, 0x00, 0x01, 0x03,  // 3rd (unprocessed) Setting
+    0x00, 0x00, 0x00, 0x03,
+  };
+
+  TestSpdyVisitor visitor(spdy_version_);
+  visitor.use_compression_ = false;
+  if (IsSpdy2()) {
+    visitor.SimulateInFramer(kV2FrameData, sizeof(kV2FrameData));
+  } else {
+    visitor.SimulateInFramer(kV3FrameData, sizeof(kV3FrameData));
+  }
+  EXPECT_EQ(1, visitor.error_count_);
+  EXPECT_EQ(1, visitor.setting_count_);
+  EXPECT_EQ(1, visitor.settings_frame_count_);
+}
+
 TEST_P(SpdyFramerTest, ReadCredentialFrame) {
   SpdyCredential credential;
   credential.slot = 3;
diff --git a/net/spdy/spdy_protocol_test.cc b/net/spdy/spdy_protocol_test.cc
index d4d9b1f..f71f0d2 100644
--- a/net/spdy/spdy_protocol_test.cc
+++ b/net/spdy/spdy_protocol_test.cc
@@ -245,9 +245,10 @@ TEST_P(SpdyProtocolTest, TestSpdySettingsFrame) {
     SettingsMap parsed_settings;
     EXPECT_TRUE(framer.ParseSettings(settings_frame.get(), &parsed_settings));
     EXPECT_EQ(settings.size(), parsed_settings.size());
-    SettingsMap::const_iterator it, it2;
-    for (it = parsed_settings.begin(); it != parsed_settings.end(); it++) {
-      it2 = settings.find(it->first);
+    for (SettingsMap::const_iterator it = parsed_settings.begin();
+         it != parsed_settings.end();
+         it++) {
+      SettingsMap::const_iterator it2 = settings.find(it->first);
       EXPECT_EQ(it->first, it2->first);
       SettingsFlagsAndValue parsed = it->second;
       SettingsFlagsAndValue created = it2->second;