Consistently suggest ephemeral port for QUIC client

net/quic/port_suggester.cc

Index: net/quic/port_suggester.cc
diff --git a/net/quic/port_suggester.cc b/net/quic/port_suggester.cc
new file mode 100644
index 0000000000000000000000000000000000000000..adc1c0552435e5566e9bde3d196621f4ee30faee
--- /dev/null
+++ b/net/quic/port_suggester.cc
@@ -0,0 +1,44 @@
+// Copyright 2013 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/quic/port_suggester.h"
+
+#include "base/logging.h"
+#include "net/base/host_port_pair.h"
+
+namespace net {
+
+PortSuggester::PortSuggester(const HostPortPair& server,
+                             uint64 per_profile_randomness)
+    : call_count_(0),
+      previous_suggestion_(-1) {
+  unsigned char hash_bytes[base::kSHA1Length];
+  base::SHA1HashBytes(
+      reinterpret_cast<const unsigned char*>(server.host().data()),
+      server.host().length(), hash_bytes);
+  memcpy(&seed_, hash_bytes, sizeof(seed_));
+  seed_ ^= per_profile_randomness ^ server.port();
+}
+
+int PortSuggester::SuggestPort(int min, int max) {
+  // Sometimes our suggestion can't be used, so we ensure that if additional
+  // calls are made, then each call (probably) provides a new suggestion.
+  if (++call_count_ > 1) {
+    // Evolve the seed.
+    unsigned char hash_bytes[base::kSHA1Length];
+    base::SHA1HashBytes(reinterpret_cast<const unsigned char *>(&seed_),
+                        sizeof(seed_), hash_bytes);
+    memcpy(&seed_, hash_bytes, sizeof(seed_));
+  }
+  DCHECK_LE(min, max);
+  DCHECK_GT(min, 0);
+  int range = max - min + 1;
+  // Ports (and hence the extent of the |range|) are generally under 2^16, so
+  // the tiny non-uniformity in the pseudo-random distribution is not
+  // significant.
+  previous_suggestion_ = static_cast<int>(seed_ % range) + min;

[Ryan Hamilton, 2013/12/07 23:21:55]

I'm not sure that I understand the need for previous_suggestion_.  Can you
simply do:

return (seed_ % range) + min;

This avoids the need for previous_suggestion_ at all? 

edit:  Oh, I see, it's used in the DCHECK in quic_stream_factory.cc.  

nit: If you decide to keep the unit test, you might consider implementing
previous_suggestion() as: 
  return (seed_ % range) + min; 
instead of dedicating a member variable to this function which is only used in a
DCHECK (and hence not in the production binary).  You could return
previous_suggestion(); as the last line from SuggestPort to reuse the code.

[jar (doing other things), 2013/12/08 01:38:36]

Min and max are not members... so the calculation can't be repeated separately.

The method will be optimized out (as unused) in production code... which should
indeed cause the slot to be optimized out as never referenced.  That said, the
object is dynamic, and pretty short lived, so the the extra 4 bytes won't be a
big issue.

[Ryan Hamilton, 2013/12/08 04:00:29]

Oh, right!  Of course, that's obvious now that you point it out.
*nod*

+  return previous_suggestion_;
+}
+
+}  // namespace net