Cert - protobufs to serialize and deserialize CertVerifierCache.

net/extras/cert/cert_verifier_cache_persister.h

Index: net/extras/cert/cert_verifier_cache_persister.h
diff --git a/net/extras/cert/cert_verifier_cache_persister.h b/net/extras/cert/cert_verifier_cache_persister.h
new file mode 100644
index 0000000000000000000000000000000000000000..eadd3535353283afa4f6ce1b74054a80d8a65344
--- /dev/null
+++ b/net/extras/cert/cert_verifier_cache_persister.h
@@ -0,0 +1,37 @@
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[Alexei Svitkine (slow), 2016/06/09 21:46:31]

Nit: No (c) for new files.

[ramant (doing other things), 2016/06/10 06:13:52]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// CertVerifierCachePersister provides API calls to serialize and deserialize
+// CachingCertVerifier's cache.

[Ryan Sleevi, 2016/06/09 22:38:42]

This comment is out of date

[ramant (doing other things), 2016/06/10 06:13:52]

thanks. Deleted it.

+//
+// WARNING: This is experimental code, please don't use it.

[Alexei Svitkine (slow), 2016/06/09 21:46:31]

This is a strange comment.

What does it mean not to use it? Does it mean it shouldn't be used from outside
the network stack or at all? Is there a plan to transition away from this state?
If so, suggest adding a crbug here with more details.

[Ryan Sleevi, 2016/06/09 22:38:42]

+1 to crbug with more details. The context is this is experimental code which
we're cautious about supporting long term, and it's only intended for specific
(non-Chromium, but using Cronet) products to use.

To that end: Raman, is there any reason this code can't live in Cronet for now,
and then, if we decide to make it a part of Chrome for Android, move it down
here with appropriate disclaimers?

[ramant (doing other things), 2016/06/10 06:13:52]

Added a TODO to move it. Will do in the next CL.

Updated the bug.

[ramant (doing other things), 2016/06/10 06:13:52]

Added a crbug.

Done.

+
+#ifndef NET_EXTRAS_CERT_CERT_VERIFIER_CACHE_PERSISTER_H_
+#define NET_EXTRAS_CERT_CERT_VERIFIER_CACHE_PERSISTER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "net/base/net_export.h"
+#include "net/extras/cert/proto/cert_verification.pb.h"
+
+namespace net {
+
+class CachingCertVerifier;
+
+// Iterates through |verifier|'s cache and returns serialized data. This can be
+// used to populate a new CachingCertVerifier with
+// |DeserializeCertVerifierCache()|.
+NET_EXPORT_PRIVATE CertVerificationCache

[Ryan Sleevi, 2016/06/09 22:38:42]

Is NET_EXPORT_PRIVATE the right API, if you're planning on using this from
Cronet?

[Ryan Sleevi, 2016/06/09 22:38:42]

Naming wise, this seems like a non-ideal name for a public type in net.

Can you namespace this to indicate it's scope? For example, sync uses
sync_pb::[Type of Protobuf] to indicate when types are protobuf-specific.

[ramant (doing other things), 2016/06/10 06:13:52]

Done.

[ramant (doing other things), 2016/06/10 06:13:52]

Will change it when we move this code to cronet.

[Ryan Sleevi, 2016/06/10 07:57:57]

I'll follow-up in person with you tomorrow to understand the motivations for not
moving it now, before it's committed, which seems the correct step for this CL.

+SerializeCertVerifierCache(const CachingCertVerifier& verifier);
+
+// Populates |verifier|'s cache. Returns true if the |proto_cert_cache| is

[Ryan Sleevi, 2016/06/09 22:38:42]

Naming: Here, naming the variable after the type is a style anti-pattern
("proto_cert_cache"). "cert_cache" seems to be a better name, with the type
itself being indicative it's a protobuf.

[ramant (doing other things), 2016/06/10 06:13:52]

Done.

+// deserialized correctly.
+NET_EXPORT_PRIVATE bool DeserializeCertVerifierCache(
+    const CertVerificationCache& proto_cert_cache,
+    CachingCertVerifier* verifier);
+
+}  // namespace net
+
+#endif  // NET_EXTRAS_CERT_CERT_VERIFIER_CACHE_PERSISTER_H_