Optimized HostPortPair::ToString and CookieMonster::GetAllCookiesForURLWithOptions.

net/http/http_server_properties_impl.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/http/http_server_properties_impl.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #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"
 
 namespace net {
 
 namespace {
 
 const uint64 kBrokenAlternateProtocolDelaySecs = 300;
 
 }  // namespace
@@ skipping 102 matching lines @@
       spdy_server_list->Append(new base::StringValue(spdy_server_host_port));
       ++count;
     }
   }
 }
 
 // static
 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;
 }
 
 static const AlternateProtocolInfo* g_forced_alternate_protocol = NULL;
 
 // static
 void HttpServerPropertiesImpl::ForceAlternateProtocol(
     const AlternateProtocolInfo& info) {
   // Note: we're going to leak this.
   if (g_forced_alternate_protocol)
@@ skipping 350 matching lines @@
   base::TimeDelta delay = when > now ? when - now : base::TimeDelta();
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(
           &HttpServerPropertiesImpl::ExpireBrokenAlternateProtocolMappings,
           weak_ptr_factory_.GetWeakPtr()),
       delay);
 }
 
 }  // namespace net