SPDY - don't persist SpdySettings until we convert it to a map.

net/spdy/spdy_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_session.h"
 
 #include <map>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 1282 matching lines @@
 
   scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
   stream->OnDataReceived(data, len);
 }
 
 void SpdySession::OnSetting(spdy::SpdySettingsIds id,
                             uint8 flags,
                             uint32 value) {
   HandleSetting(id, value);
   spdy::SettingsFlagsAndId flags_and_id(flags, id);
-  http_server_properties_->SetSpdySetting(
-      host_port_pair(), std::make_pair(flags_and_id, value));
+  // TODO(rtenneti): persist SpdySetting.

[willchan no longer on Chromium, 2012/03/15 03:13:54]

Wait, why do we have to change this? We can still set the SpdySetting, right? It
just can't be persisted?

[ramant (doing other things), 2012/03/15 03:46:30]

SetSpdySetting is doing the copy of memory. It is possible over a period of time
(for plus.google.com) we get lot of settings update and we keep appending and
thus the list can grow. Will check in a fix that doesn't grow the memory
tomorrow. Hope this approach is ok.

+  // http_server_properties_->SetSpdySetting(
+  //    host_port_pair(), std::make_pair(flags_and_id, value));
 
   received_settings_ = true;
 
   // Log the settings.
   spdy::SpdySettings settings;
   settings.insert(settings.end(), std::make_pair(flags_and_id, value));
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_RECV_SETTINGS,
       make_scoped_refptr(new NetLogSpdySettingsParameter(settings)));
 }
@@ skipping 351 matching lines @@
         cwnd = ApplyCwndFieldTrialPolicy(val);
         UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SpdySettingsCwndSent",
                                     cwnd,
                                     1, 200, 100);
         if (cwnd != val) {
           spdy::SettingsFlagsAndId new_id(spdy::SETTINGS_FLAG_PLEASE_PERSIST,
                                           id);
           i->second = cwnd;
           i->first = new_id;
           spdy::SpdySetting setting(new_id, val);
-          http_server_properties_->SetSpdySetting(host_port_pair(), setting);
+          // TODO(rtenneti): Persist SpdySetting.

[willchan no longer on Chromium, 2012/03/15 03:13:54]

Do we need to change this too?

[ramant (doing other things), 2012/03/15 03:46:30]

This is where the bug was. In the previous version, we used to break and we used
to set only one setting update. For sorting purposes, I used the same loop and
it had the side effect of calling SetSpdySetting for every element (and most of
the elements were CWD setting).

None of this code will get executed because GetSpdySettings will be empty.
Wanted to be 100% safe. Will implement persisting (and saving of the settings
tonight/tomorrow).

What do you think of this approach (being very conservative).

+          // http_server_properties_->SetSpdySetting(host_port_pair(), setting);
           unique_settings[id] = setting;
           continue;
         }
     }
     unique_settings[id] = *i;
   }
 
   HandleSettings(settings);
 
   net_log_.AddEvent(
@@ skipping 247 matching lines @@
 SSLClientSocket* SpdySession::GetSSLClientSocket() const {
   if (!is_secure_)
     return NULL;
   SSLClientSocket* ssl_socket =
       reinterpret_cast<SSLClientSocket*>(connection_->socket());
   DCHECK(ssl_socket);
   return ssl_socket;
 }
 
 }  // namespace net