SSLClientSessionCache: Log number of times Lookup is called per Session.

net/ssl/ssl_client_session_cache.h

Index: net/ssl/ssl_client_session_cache.h
diff --git a/net/ssl/ssl_client_session_cache.h b/net/ssl/ssl_client_session_cache.h
index 4a82010badda3722a6d318ff74951016a155bfc2..3c62d918e1afb80a07a91612c0b8eee89715b9d9 100644
--- a/net/ssl/ssl_client_session_cache.h
+++ b/net/ssl/ssl_client_session_cache.h
@@ -46,8 +46,17 @@ class NET_EXPORT SSLClientSessionCache : public base::MemoryCoordinatorClient {
   size_t size() const;
 
   // Returns the session associated with |cache_key| and moves it to the front
-  // of the MRU list. Returns nullptr if there is none.
-  bssl::UniquePtr<SSL_SESSION> Lookup(const std::string& cache_key);
+  // of the MRU list. Returns nullptr if there is none. If |count| is non-null,
+  // |*count| will contain the number of times this session has been looked up
+  // (including this call).
+  bssl::UniquePtr<SSL_SESSION> Lookup(const std::string& cache_key,
+                                      size_t* count);
+
+  // Resets the count returned by Lookup to 0. The SSL_SESSION returned by
+  // Lookup must be passed in, and the count will only be reset if the sessions
+  // are the same. This should be called when Lookup returns a non-null session
+  // and that session is used in a handshake.
+  void ResetLookupCount(const std::string& cache_key, SSL_SESSION* session);
 
   // Inserts |session| into the cache at |cache_key|. If there is an existing
   // one, it is released. Every |expiration_check_count| calls, the cache is
@@ -64,6 +73,15 @@ class NET_EXPORT SSLClientSessionCache : public base::MemoryCoordinatorClient {
   void DumpMemoryStats(base::trace_event::ProcessMemoryDump* pmd);
 
  private:
+  struct Entry {
+    Entry();
+    Entry(Entry&&);
+    ~Entry();
+
+    size_t lookups;

[davidben, 2017/01/19 20:29:34]

size_t is a little odd here. This is a count of things, so it doesn't actually
measure the size of a buffer. In BoringSSL, I'd probably have used unsigned or
uint64_t, but I think Chromium may prescribe differently.

My read of these two:
https://google.github.io/styleguide/cppguide.html#Integer_Types
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
is that int is what to use?

(I don't know why I made lookups_since_flush a size_t. I think that was just a
mistake.)

[nharper, 2017/01/19 21:50:35]

I'm fine with int. I'm only logging up to 20, and I doubt this count will get
anywhere close to 2^31 (although maybe something in a tight loop trying to make
a lot of connections with no network connectivity might?)

+    bssl::UniquePtr<SSL_SESSION> session;
+  };

[davidben, 2017/01/19 20:29:34]

[Does it work to just do:

  struct Entry {
    size_t lookups = 0;
    bssl::UniquePtr<SSL_SESSION> session;
  };

I dunno if that trips up our inline vs out-of-line ctor/dtor check.]

[nharper, 2017/01/19 21:50:35]

That (with int instead of size_t) still trips up the chromium-style check.

+
   // base::MemoryCoordinatorClient implementation:
   void OnMemoryStateChange(base::MemoryState state) override;
 
@@ -79,7 +97,7 @@ class NET_EXPORT SSLClientSessionCache : public base::MemoryCoordinatorClient {
 
   std::unique_ptr<base::Clock> clock_;
   Config config_;
-  base::HashingMRUCache<std::string, bssl::UniquePtr<SSL_SESSION>> cache_;
+  base::HashingMRUCache<std::string, Entry> cache_;
   size_t lookups_since_flush_;
 
   // TODO(davidben): After https://crbug.com/458365 is fixed, replace this with