Save pipelining capabilities for the most used hosts between sessions.

net/http/http_server_properties.h

Index: net/http/http_server_properties.h
diff --git a/net/http/http_server_properties.h b/net/http/http_server_properties.h
index 51171c23c21de6f48ef870c847e4fbc791eee663..d6cdcd8d54af6ccb2a4f51dfa7731ee1b9846f56 100644
--- a/net/http/http_server_properties.h
+++ b/net/http/http_server_properties.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_export.h"
+#include "net/http/http_pipelined_host.h"

[mmenke, 2011/12/02 21:52:10]

You might want to move the Capability enumeration into its own file, so you
don't have to include this here.  While this doesn't include too much,
url_request_context.h does include this file, and it's used all over the place.

Alternatively, you could just use ints instead of HttpPipelinedHost::Capability.

Or you could keep it as is, I don't feel particularly strongly about this, just
thought I'd mention it.

[James Simonsen, 2011/12/03 03:16:00]

Done. Yeah, seems like the right thing to do. The interface files aren't too
heavy now, but it'd be easy for them to grow without knowing the cost.

 #include "net/spdy/spdy_framer.h"  // TODO(willchan): Reconsider this.
 
 namespace net {
@@ -35,6 +36,8 @@ struct NET_EXPORT PortAlternateProtocolPair {
 
 typedef std::map<HostPortPair, PortAlternateProtocolPair> AlternateProtocolMap;
 typedef std::map<HostPortPair, spdy::SpdySettings> SpdySettingsMap;
+typedef std::map<HostPortPair,
+        HttpPipelinedHost::Capability> PipelineCapabilityMap;

[mmenke, 2011/12/02 21:52:10]

Since changing how HttpPipelinedHost::Capability maps meaning to integers now
breaks things, I suggest you hard code values in the enumeration, and add a
comment that the exact values matter.

[James Simonsen, 2011/12/03 03:16:00]

Done.

 
 extern const char kAlternateProtocolHeader[];
 extern const char* const kAlternateProtocolStrings[NUM_ALTERNATE_PROTOCOLS];
@@ -95,6 +98,17 @@ class NET_EXPORT HttpServerProperties {
   // Returns all persistent SpdySettings.
   virtual const SpdySettingsMap& spdy_settings_map() const = 0;
 
+  virtual HttpPipelinedHost::Capability GetPipelineCapability(
+      const HostPortPair& origin) = 0;
+
+  virtual void SetPipelineCapability(
+      const HostPortPair& origin,
+      HttpPipelinedHost::Capability capability) = 0;
+
+  virtual void ClearPipelineCapabilities() = 0;

[mmenke, 2011/12/02 21:52:10]

Was thinking we wouldn't need this, but may want it for unit testing.

Also, the SPDY equivalent is called when the network changes, since the meaning
of addresses may change.  We should probably do the same, though it seems a
little odd to me given that we keep it across browser sessions, between which
the network may change.

[James Simonsen, 2011/12/03 03:16:00]

It seems it's mainly used for when the user clears their history. Technically,
pipelining knowledge would be part of that too.
I was picturing a separate layer for that.

If the user's network changes, it's possible they now have an invisible proxy
that breaks pipelining. In that case, we should just disable pipelining for the
entire network session rather than trying to track it per host.

The plan was to forcefully pipeline a couple of background test requests to
chromium.org any time the network changes. If those pass, then we deem the
session capable of pipelining and enable per-host pipelining (aka what we have
now).

Anyway, it's on my list to do that, assuming we find some broken proxies in the
wild.

+
+  virtual PipelineCapabilityMap GetPipelineCapabilityMap() const = 0;

[ramant (doing other things), 2011/12/03 00:31:43]

nit: GetPipelineCapabilityMap => pipeline_capability_map

[mmenke, 2011/12/03 00:38:34]

This is not a simple accessor.  It converts an MRUCache to an std::map.

+
  private:
   DISALLOW_COPY_AND_ASSIGN(HttpServerProperties);
 };