SPDY - persist SPDY settings.

chrome/browser/net/http_server_properties_manager.cc

Index: chrome/browser/net/http_server_properties_manager.cc
===================================================================
--- chrome/browser/net/http_server_properties_manager.cc	(revision 127896)
+++ chrome/browser/net/http_server_properties_manager.cc	(working copy)
@@ -140,31 +140,20 @@
   return http_server_properties_impl_->alternate_protocol_map();
 }
 
-const spdy::SpdySettings&
+const spdy::SettingsMap&
 HttpServerPropertiesManager::GetSpdySettings(
     const net::HostPortPair& host_port_pair) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   return http_server_properties_impl_->GetSpdySettings(host_port_pair);
 }
 
-// Saves settings for a host.
-bool HttpServerPropertiesManager::SetSpdySettings(
-    const net::HostPortPair& host_port_pair,
-    const spdy::SpdySettings& settings) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  bool persist = http_server_properties_impl_->SetSpdySettings(
-      host_port_pair, settings);
-  if (persist)
-    ScheduleUpdatePrefsOnIO();
-  return persist;
-}
-
 bool HttpServerPropertiesManager::SetSpdySetting(
     const net::HostPortPair& host_port_pair,
-    const spdy::SpdySetting& setting) {
+    uint32 id,
+    const spdy::SettingsFlagsAndValue& flags_and_value) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   bool persist = http_server_properties_impl_->SetSpdySetting(
-      host_port_pair, setting);
+      host_port_pair, id, flags_and_value);
   if (persist)
     ScheduleUpdatePrefsOnIO();
   return persist;
@@ -278,9 +267,49 @@
       spdy_servers->push_back(server_str);
     }
 
-    // TODO(rtenneti): Implement reading of SpdySettings.
+    // Get SpdySettings.
     DCHECK(!ContainsKey(*spdy_settings_map, server));
+    base::ListValue* spdy_settings_list = NULL;
+    if (server_pref_dict->GetListWithoutPathExpansion(
+        "settings", &spdy_settings_list)) {
+      spdy::SettingsMap settings_map;
 
+      for (base::ListValue::const_iterator list_it =
+           spdy_settings_list->begin();
+           list_it != spdy_settings_list->end(); ++list_it) {
+        if ((*list_it)->GetType() != Value::TYPE_DICTIONARY) {

[Ryan Hamilton, 2012/03/21 16:24:41]

Do I understand correctly that we're persisting this as a list of dicts?  How
come?

[ramant (doing other things), 2012/03/23 04:11:25]

Correct. 

Didn't change the format in this CL. The old version persisted Settings as a
List (server used to send duplicates). 

The following is the discussion willchan and I had earlier (remember 
http://codereview.chromium.org/8423028/#msg15

Will add a TODO to add version and change the format of the persisted settings
(remove the extra List).

[Ryan Hamilton, 2012/03/23 15:25:38]

Sounds good.

+          DVLOG(1) << "Malformed SpdySettingsList for server: " << server_str;
+          NOTREACHED();
+          continue;
+        }
+
+        const base::DictionaryValue* spdy_setting_dict =
+            static_cast<const base::DictionaryValue*>(*list_it);
+
+        int id = 0;
+        if (!spdy_setting_dict->GetIntegerWithoutPathExpansion("id", &id)) {
+          DVLOG(1) << "Malformed id in SpdySettings for server: " << server_str;
+          NOTREACHED();
+          continue;
+        }
+
+        int value = 0;
+        if (!spdy_setting_dict->GetIntegerWithoutPathExpansion("value",
+                                                               &value)) {
+          DVLOG(1) << "Malformed value in SpdySettings for server: " <<
+              server_str;
+          NOTREACHED();
+          continue;
+        }
+
+        spdy::SettingsFlagsAndValue flags_and_value(
+            spdy::SETTINGS_FLAG_PERSISTED, value);
+        settings_map[id] = flags_and_value;
+      }
+
+      (*spdy_settings_map)[server] = settings_map;
+    }
+
     int pipeline_capability = net::PIPELINE_UNKNOWN;
     if ((server_pref_dict->GetInteger(
          "pipeline_capability", &pipeline_capability)) &&
@@ -423,20 +452,20 @@
 struct ServerPref {
   ServerPref()
       : supports_spdy(false),
-        settings(NULL),
+        settings_map(NULL),
         alternate_protocol(NULL),
         pipeline_capability(net::PIPELINE_UNKNOWN) {
   }
   ServerPref(bool supports_spdy,
-             const spdy::SpdySettings* settings,
+             const spdy::SettingsMap* settings_map,
              const net::PortAlternateProtocolPair* alternate_protocol)
       : supports_spdy(supports_spdy),
-        settings(settings),
+        settings_map(settings_map),
         alternate_protocol(alternate_protocol),
         pipeline_capability(net::PIPELINE_UNKNOWN) {
   }
   bool supports_spdy;
-  const spdy::SpdySettings* settings;
+  const spdy::SettingsMap* settings_map;
   const net::PortAlternateProtocolPair* alternate_protocol;
   net::HttpPipelinedHostCapability pipeline_capability;
 };
@@ -480,7 +509,7 @@
       ServerPref server_pref(false, &map_it->second, NULL);
       server_pref_map[server] = server_pref;
     } else {
-      it->second.settings = &map_it->second;
+      it->second.settings_map = &map_it->second;
     }
   }
 
@@ -535,7 +564,21 @@
     // Save supports_spdy.
     server_pref_dict->SetBoolean("supports_spdy", server_pref.supports_spdy);
 
-    // TODO(rtenneti): Implement save SpdySettings.
+    // Save SPDY settings.
+    if (server_pref.settings_map) {
+      base::ListValue* spdy_settings_list = new ListValue();

[Ryan Hamilton, 2012/03/21 16:24:41]

Do you need to worry about reading old values?  Did we persist settings
differently in previous versions?

[ramant (doing other things), 2012/03/23 04:11:25]

The format for spdy_settings is same as before. Format hasn't changed.

+      for (spdy::SettingsMap::const_iterator it =
+           server_pref.settings_map->begin();
+           it != server_pref.settings_map->end(); ++it) {
+        uint32 id = it->first;
+        uint32 value = it->second.second;
+        base::DictionaryValue* spdy_setting_dict = new base::DictionaryValue;
+        spdy_setting_dict->SetInteger("id", id);
+        spdy_setting_dict->SetInteger("value", value);
+        spdy_settings_list->Append(spdy_setting_dict);
+      }
+      server_pref_dict->Set("settings", spdy_settings_list);
+    }
 
     // Save alternate_protocol.
     if (server_pref.alternate_protocol) {