Instrument SSL sockets using MemoryDumpProvider

net/ssl/ssl_client_session_cache.cc

Index: net/ssl/ssl_client_session_cache.cc
diff --git a/net/ssl/ssl_client_session_cache.cc b/net/ssl/ssl_client_session_cache.cc
index fc817fdba0b410f37294f964c63be70d6e7753c4..6dec456675275c46083ec2af22ab5de55d4af392 100644
--- a/net/ssl/ssl_client_session_cache.cc
+++ b/net/ssl/ssl_client_session_cache.cc
@@ -4,11 +4,16 @@
 
 #include "net/ssl/ssl_client_session_cache.h"
 
+#include <openssl/ssl.h>

[davidben, 2016/12/06 01:02:07]

Already included below.

[xunjieli, 2016/12/06 18:36:08]

Done.

+#include <openssl/x509.h>

[davidben, 2016/12/06 01:02:07]

#include "third_party/boringssl/src/include/openssl/x509.h"

[xunjieli, 2016/12/06 18:36:08]

Done.

 #include <utility>
 
 #include "base/memory/memory_coordinator_client_registry.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "net/cert/x509_util_openssl.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
 
 namespace net {
@@ -82,6 +87,44 @@ bool SSLClientSessionCache::IsExpired(SSL_SESSION* session, time_t now) {
              SSL_SESSION_get_time(session) + SSL_SESSION_get_timeout(session);
 }
 
+void SSLClientSessionCache::DumpMemoryStats(
+    base::trace_event::ProcessMemoryDump* pmd) {
+  std::string absolute_name = "net/ssl_session_cache";
+  base::trace_event::MemoryAllocatorDump* cache_dump =
+      pmd->GetAllocatorDump(absolute_name);
+  // This method can be reached from different URLRequestContexts. Since this is
+  // a singleton, only log memory stats once.

[davidben, 2016/12/06 01:02:07]

Could you link to https://crbug.com/458365? That's the bug to fix this
ridiculous singleton.

[xunjieli, 2016/12/06 18:36:08]

Done.

+  if (cache_dump)
+    return;

[davidben, 2016/12/06 01:02:07]

How does this interact with multiple threads? (I'm not familiar with this API.)
The SSLClientSessionCache is shared between all URLRequestContexts, regardless
of which thread they're on.

[xunjieli, 2016/12/06 18:36:08]

Right, this came up in the design doc discussion as well. I think we are going
to depend on |lock_| for now and change this once 458365 is fixed. I tested this
locally. I don't see any weird issue.

+  cache_dump = pmd->CreateAllocatorDump(absolute_name);
+  base::AutoLock lock(lock_);
+  auto iter = cache_.begin();
+  while (iter != cache_.end()) {

[davidben, 2016/12/06 01:02:07]

Does this work?

  for (const auto& pair : cache_) {

[xunjieli, 2016/12/06 18:36:08]

Done. Yep it works. Copied from SSLClientSessionCache::FlushExpiredSessions().
I will change this to const auto& pair.

+    auto entry = iter->second.get();
+    auto cert_chain = entry->x509_chain;
+    size_t cert_count = sk_X509_num(cert_chain);
+    base::trace_event::MemoryAllocatorDump* entry_dump =
+        pmd->CreateAllocatorDump(
+            base::StringPrintf("%s/entry_%p", absolute_name.c_str(), entry));
+    int cert_size = 0;
+    for (size_t i = 0; i < cert_count; ++i) {
+      X509* cert = sk_X509_value(cert_chain, i);
+      cert_size += i2d_X509(cert, nullptr);
+    }
+    entry_dump->AddScalar("cert_size",

[davidben, 2016/12/06 01:02:07]

Same comment as on ssl_client_socket_impl.cc

[xunjieli, 2016/12/06 18:36:08]

Done.

+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                          cert_size);
+    entry_dump->AddScalar("cert_count",
+                          base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+                          cert_count);
+    entry_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                          cert_size);
+
+    ++iter;
+  }
+}
+
 void SSLClientSessionCache::FlushExpiredSessions() {
   time_t now = clock_->Now().ToTimeT();
   auto iter = cache_.begin();