Optimized HostPortPair::ToString and CookieMonster::GetAllCookiesForURLWithOptions.

net/http/http_server_properties_impl.cc

Index: net/http/http_server_properties_impl.cc
diff --git a/net/http/http_server_properties_impl.cc b/net/http/http_server_properties_impl.cc
index 1bf541bf0ea68607dc384d7f2f5c1bb266db827b..f1efa195fc6f374ec4bf65bb3b89c5f15008e3ec 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -9,6 +9,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 
@@ -131,9 +132,10 @@ void HttpServerPropertiesImpl::GetSpdyServerList(
 std::string HttpServerPropertiesImpl::GetFlattenedSpdyServer(
     const net::HostPortPair& host_port_pair) {
   std::string spdy_server;
-  spdy_server.append(host_port_pair.host());
+  spdy_server.reserve(host_port_pair.host().length() + 7);
+  spdy_server.assign(host_port_pair.host());
   spdy_server.append(":");
-  base::StringAppendF(&spdy_server, "%d", host_port_pair.port());
+  spdy_server.append(base::IntToString(host_port_pair.port()));

[Ryan Sleevi, 2014/12/01 16:28:51]

Not LGTM.

I don't like line 135 because, as a programming pattern, it requires me to
carefully reason whether .length() is safe to add +7 to it and what the
implications are if it isn't. My code review pattern matching is to hone in on
this, and any code full of surprises, especially of a "is it secure or not", is
bad.

I'm not sure why this code does append, append, StringAppendF, compared with
HostPortPair's StringPrintF,
https://code.google.com/p/chromium/codesearch#chromium/src/net/base/host_port...
, but I feel like if you're going to change this method, changing it to match
HostPortPair (with the special case of what I presume is bracketed IPv6 vs
unbracketed; if there isn't a special case, it should be using HostPortPair
ToString()) is a far easier change to reason about.

[Ryan Hamilton, 2014/12/01 20:14:53]

I agree with everything sleevi said. I'd be inclined to simply use
host_port_pair.ToString() instead of GetFlattenedSpdyServer in the two places
that it is called.

   return spdy_server;
 }