SSL Host info: Make sure that we can update certificate chains in

net/http/disk_cache_based_ssl_host_info_unittest.cc

Index: net/http/disk_cache_based_ssl_host_info_unittest.cc
===================================================================
--- net/http/disk_cache_based_ssl_host_info_unittest.cc	(revision 111217)
+++ net/http/disk_cache_based_ssl_host_info_unittest.cc	(working copy)
@@ -8,6 +8,8 @@
 #include "net/http/mock_http_cache.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
 class DeleteSSLHostInfoOldCompletionCallback : public TestOldCompletionCallback {
  public:
   explicit DeleteSSLHostInfoOldCompletionCallback(net::SSLHostInfo* ssl_host_info)
@@ -22,6 +24,24 @@
   net::SSLHostInfo* ssl_host_info_;
 };
 
+// This is an empty transaction, needed to register the URL and the test mode.
+const MockTransaction kHostInfoTransaction = {
+  "sslhostinfo:https://www.google.com",
+  "",
+  base::Time(),
+  "",
+  net::LOAD_NORMAL,
+  "",
+  "",
+  base::Time(),
+  "",
+  TEST_MODE_NORMAL,
+  NULL,
+  0
+};
+
+}  // namespace
+
 // Tests that we can delete a DiskCacheBasedSSLHostInfo object in a
 // completion callback for DiskCacheBasedSSLHostInfo::WaitForDataReady.
 TEST(DiskCacheBasedSSLHostInfo, DeleteInCallback) {
@@ -42,3 +62,59 @@
   factory->FinishCreation();
   EXPECT_EQ(net::OK, callback.GetResult(rv));
 }
+
+// Tests the basic logic of storing, retrieving and updating data.
+TEST(DiskCacheBasedSSLHostInfo, Update) {
+  MockHttpCache cache;
+  AddMockTransaction(&kHostInfoTransaction);
+  TestOldCompletionCallback callback;
+
+  // Store a certificate chain.
+  net::CertVerifier cert_verifier;
+  net::SSLConfig ssl_config;
+  scoped_ptr<net::SSLHostInfo> ssl_host_info(
+      new net::DiskCacheBasedSSLHostInfo("https://www.google.com", ssl_config,
+                                         &cert_verifier, cache.http_cache()));
+  ssl_host_info->Start();
+  int rv = ssl_host_info->WaitForDataReady(&callback);
+  EXPECT_EQ(net::OK, callback.GetResult(rv));

[wtc, 2011/11/30 00:22:09]

We should check that state->certs.empty() is true.

+
+  net::SSLHostInfo::State* state = ssl_host_info->mutable_state();
+  state->certs.push_back(std::string("foo"));
+  ssl_host_info->Persist();
+
+  // Wait until Persist() does the work.
+  MessageLoop::current()->RunAllPending();
+
+  // Open the stored certificate chain.
+  ssl_host_info.reset(
+      new net::DiskCacheBasedSSLHostInfo("https://www.google.com", ssl_config,
+                                         &cert_verifier, cache.http_cache()));
+  ssl_host_info->Start();
+  rv = ssl_host_info->WaitForDataReady(&callback);
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  // And now update the data.
+  state = ssl_host_info->mutable_state();
+  EXPECT_EQ(1U, state->certs.size());
+  EXPECT_EQ("foo", state->certs.front());
+  state->certs.push_back(std::string("bar"));
+
+  // Fail instead of DCHECKing double creates.
+  cache.disk_cache()->set_double_create_check(false);

[wtc, 2011/11/30 00:22:09]

Is this necessary?  With your found_entry_ change, I
believe we won't be calling CreateEntry twice.  We should
be calling OpenEntry if found_entry_ is true.

[rvargas (doing something else), 2011/11/30 01:05:24]

I just want to make sure that if the test fails (without this fix), we don't
crash net_unittests.

+  ssl_host_info->Persist();
+  MessageLoop::current()->RunAllPending();
+
+  // Verify that the state was updated.
+  ssl_host_info.reset(
+      new net::DiskCacheBasedSSLHostInfo("https://www.google.com", ssl_config,
+                                         &cert_verifier, cache.http_cache()));
+  ssl_host_info->Start();
+  rv = ssl_host_info->WaitForDataReady(&callback);
+  EXPECT_EQ(net::OK, callback.GetResult(rv));
+
+  state = ssl_host_info->mutable_state();
+  EXPECT_EQ(2U, state->certs.size());

[wtc, 2011/11/30 00:22:09]

Should we check that state->certs[0] is "foo" and
state->certs[1] is "bar"?

[rvargas (doing something else), 2011/11/30 01:05:24]

It felt like testing the mock cache... but I guess we can test that the order is
not reversed...

+
+  RemoveMockTransaction(&kHostInfoTransaction);
+}