SPDY - replaced SpdySettings (list) with SettingsMap

net/spdy/spdy_protocol_test.cc

Index: net/spdy/spdy_protocol_test.cc
===================================================================
--- net/spdy/spdy_protocol_test.cc	(revision 131881)
+++ net/spdy/spdy_protocol_test.cc	(working copy)
@@ -215,7 +215,7 @@
   SpdyFramer framer(spdy_version_);
 
   // Create a settings frame with no settings.
-  SpdySettings settings;
+  SettingsMap settings;
   scoped_ptr<SpdySettingsControlFrame> settings_frame(
       framer.CreateSettings(settings));
   EXPECT_EQ(framer.protocol_version(), settings_frame->version());
@@ -225,34 +225,65 @@
 
   // We'll add several different ID/Flag combinations and then verify
   // that they encode and decode properly.
-  SettingsFlagsAndId ids[] = {
+  SettingsFlagsAndId spdy2_ids[] = {
+    // id = 0, flags = 0.
     SettingsFlagsAndId::FromWireFormat(spdy_version_, 0x00000000),
+    // id = 1, flags = 0xff.
+    SettingsFlagsAndId::FromWireFormat(spdy_version_, 0xff000001),
+    // id = 2, flags = 1.
+    SettingsFlagsAndId::FromWireFormat(spdy_version_, 0x01000002),
+    // id = 9, flags = 6.
+    SettingsFlagsAndId(6, 9),
+    // id = 0x00ffffff, flags = 0xff.
     SettingsFlagsAndId::FromWireFormat(spdy_version_, 0xffffffff),
+  };
+
+  SettingsFlagsAndId spdy3_ids[] = {
+    // id = 0, flags = 0.
+    SettingsFlagsAndId::FromWireFormat(spdy_version_, 0x00000000),
+    // id = 1, flags = 2.
+    SettingsFlagsAndId::FromWireFormat(spdy_version_, 0x01000002),
+    // id = 9, flags = 6.
+    SettingsFlagsAndId(6, 9),
+    // id = 0xff, flags = 1.
     SettingsFlagsAndId::FromWireFormat(spdy_version_, 0xff000001),
+    // id = 0x00ffffff, flags = 0xff.
     SettingsFlagsAndId::FromWireFormat(spdy_version_, 0xffffffff),
-    SettingsFlagsAndId::FromWireFormat(spdy_version_, 0x01000002),
-    SettingsFlagsAndId(3, 1)
   };

[Ryan Hamilton, 2012/04/12 17:11:21]

How come the spdy2 and spdy3 settings are separated into two different chunks
here?

[ramant (doing other things), 2012/04/12 18:35:59]

In the map, settings are stored in sorted order based on id. The id's are
different from spdy2/spdy3 (because FromWireFormat is different between spdy/2
and spdy/3)

For example: for "0x01000002" is spdy/2, id is 2 and flags is 1 and in spdy/3 id
is 1 and flags is 2.

Implementing two different arrays, made it easier to keep hasan's exiting test
code/cases. 

Will talk to him if he wants to rewrite this test.

[Ryan Hamilton, 2012/04/17 02:32:49]

Perhaps I'm missing something, but it looks like both arrays are constructed
with the same SettingsFlagsAndId::FromWireFormat() invocations, albeit in a
different order.  Is the order significant?
*nod*  I'm content to defer to whatever you guys agree to.

[ramant (doing other things), 2012/04/17 18:48:19]

(sorry if already known)

We get data out of "ids" array and store it in a Map(where it is stored in
sorted by "id").

CreateSettings used to take a list and now it is taking a Map. Because it is
taking a Map, the settings are created in a sorted order (by "id").

Then we call ParseSettings, which returns the settings  in a Map (sorted by
"id"). It would be in the same order as "settings" Map that is passed to
CreateSettings.

Prior to this change, ParseSettings, used to return the settings in a list and
thus it would return in the same order as they were created.  

Then we compare the data returned by ParsedSettings with what is in the "ids".

So we compare the returned data in the following order by id.

id:
0
0,1
0,1,9
0,1,9,0xff
0,1,9,0xff,0x00ffffff

Because of the above, stored the data in the "ids" array sorted by "id".

As we had talked, changed the code to compare the std::maps.

Deleted duplicate entries in "ids".

 
-  for (size_t index = 0; index < arraysize(ids); ++index) {
-    settings.insert(settings.end(), std::make_pair(ids[index], index));
+  SettingsFlagsAndId* ids;
+  size_t settings_size;
+  if (IsSpdy2()) {
+    ids = &spdy2_ids[0];
+    settings_size = arraysize(spdy2_ids);
+  } else {
+    ids = &spdy3_ids[0];
+    settings_size = arraysize(spdy3_ids);
+  }
+
+  for (size_t index = 0; index < settings_size; ++index) {
+    SettingsFlagsAndId flags_and_id = ids[index];
+    SpdySettingsIds id = static_cast<SpdySettingsIds>(flags_and_id.id());
+    SpdySettingsFlags flags =
+        static_cast<SpdySettingsFlags>(flags_and_id.flags());
+    settings[id] = SettingsFlagsAndValue(flags, index);
     settings_frame.reset(framer.CreateSettings(settings));
     EXPECT_EQ(framer.protocol_version(), settings_frame->version());
     EXPECT_TRUE(settings_frame->is_control_frame());
     EXPECT_EQ(SETTINGS, settings_frame->type());
     EXPECT_EQ(index + 1, settings_frame->num_entries());
 
-    SpdySettings parsed_settings;
+    SettingsMap parsed_settings;
     EXPECT_TRUE(framer.ParseSettings(settings_frame.get(), &parsed_settings));
     EXPECT_EQ(settings.size(), parsed_settings.size());
-    SpdySettings::const_iterator it = parsed_settings.begin();
+    SettingsMap::const_iterator it = parsed_settings.begin();
     int pos = 0;
     while (it != parsed_settings.end()) {
-      SettingsFlagsAndId parsed = it->first;
-      uint32 value = it->second;
-      EXPECT_EQ(ids[pos].flags(), parsed.flags());
-      EXPECT_EQ(ids[pos].id(), parsed.id());
-      EXPECT_EQ(static_cast<uint32>(pos), value);
+      SpdySettingsIds parsed_id = it->first;
+      SettingsFlagsAndValue parsed = it->second;
+      EXPECT_EQ(ids[pos].flags(), static_cast<uint8>(parsed.first));
+      EXPECT_EQ(ids[pos].id(), static_cast<uint32>(parsed_id));
+      EXPECT_EQ(static_cast<uint32>(pos), parsed.second);
       ++it;
       ++pos;
     }