Fix UMA stat for expiration of certificate memory decisions

chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc

 // Copyright 2014 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 "chrome/browser/ssl/chrome_ssl_host_state_delegate.h"
 
 #include <stdint.h>
 
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 214 matching lines @@
   EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 191));
   EXPECT_FALSE(state->DidHostRunInsecureContent("example.com", 42));
 
   state->HostRanInsecureContent("example.com", 42);
 
   EXPECT_TRUE(state->DidHostRunInsecureContent("www.google.com", 42));
   EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 191));
   EXPECT_TRUE(state->DidHostRunInsecureContent("example.com", 42));
 }
 
+// QueryPolicyExpired unit tests to make sure that if a certificate decision has
+// expired, the return value from QueryPolicy returns the correct vaule.
+IN_PROC_BROWSER_TEST_F(ChromeSSLHostStateDelegateTest, PRE_QueryPolicyExpired) {
+  scoped_refptr<net::X509Certificate> google_cert = GetGoogleCert();
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
+  content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
+  bool expired_previous_decision;
+
+  // The certificate has never been seen before, so it should be UNKONWN and

[felt, 2014/10/01 02:42:36]

nit: UNKNOWN misspelled

[jww, 2014/10/01 16:33:20]

Done.

+  // should also indicate that it hasn't expired.
+  EXPECT_EQ(content::SSLHostStateDelegate::DENIED,
+            state->QueryPolicy(kWWWGoogleHost,
+                               *google_cert.get(),
+                               net::CERT_STATUS_DATE_INVALID,
+                               &expired_previous_decision));
+  EXPECT_FALSE(expired_previous_decision);
+
+  // After allowing the certificate, a query should say that it is allowed and
+  // also specify that it hasn't expired.
+  state->AllowCert(
+      kWWWGoogleHost, *google_cert.get(), net::CERT_STATUS_DATE_INVALID);
+  EXPECT_EQ(content::SSLHostStateDelegate::ALLOWED,
+            state->QueryPolicy(kWWWGoogleHost,
+                               *google_cert.get(),
+                               net::CERT_STATUS_DATE_INVALID,
+                               &expired_previous_decision));
+  EXPECT_FALSE(expired_previous_decision);
+}
+
+// Since this is being checked on a browser instance that forgets security
+// decisions after restart, we wait until after a restart to see if the decision

[felt, 2014/10/01 02:42:36]

nit: Sleevi has instructed me to never put "we" in comments. He has a point:
"we" is ambiguous. Can you rephrase?

[jww, 2014/10/01 16:33:20]

Done.

+// has expired.
+IN_PROC_BROWSER_TEST_F(ChromeSSLHostStateDelegateTest, QueryPolicyExpired) {
+  scoped_refptr<net::X509Certificate> google_cert = GetGoogleCert();
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
+  content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
+  bool expired_previous_decision;
+
+  // The browser content has restart thus expiring the user decision made above,
+  // so it should indicate that the certificate and error are DENIED but also
+  // that they expired since the last query.
+  EXPECT_EQ(content::SSLHostStateDelegate::DENIED,
+            state->QueryPolicy(kWWWGoogleHost,
+                               *google_cert.get(),
+                               net::CERT_STATUS_DATE_INVALID,
+                               &expired_previous_decision));
+  EXPECT_TRUE(expired_previous_decision);
+
+  // However, with a new query, it should indicate that no new expiration has
+  // occurred.
+  EXPECT_EQ(content::SSLHostStateDelegate::DENIED,
+            state->QueryPolicy(kWWWGoogleHost,
+                               *google_cert.get(),
+                               net::CERT_STATUS_DATE_INVALID,
+                               &expired_previous_decision));
+  EXPECT_FALSE(expired_previous_decision);
+}
+
 // Tests the basic behavior of cert memory in incognito.
 class IncognitoSSLHostStateDelegateTest
     : public ChromeSSLHostStateDelegateTest {
  protected:
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     ChromeSSLHostStateDelegateTest::SetUpCommandLine(command_line);
     command_line->AppendSwitchASCII(switches::kRememberCertErrorDecisions,
                                     kDeltaSecondsString);
   }
 };
@@ skipping 211 matching lines @@
   clock->Advance(base::TimeDelta::FromSeconds(kDeltaOneDayInSeconds + 1));
 
   // The cert should now be |DENIED| because the specified delta has passed.
   EXPECT_EQ(content::SSLHostStateDelegate::DENIED,
             state->QueryPolicy(kWWWGoogleHost,
                                *google_cert.get(),
                                net::CERT_STATUS_DATE_INVALID,
                                &unused_value));
 }
 
-// QueryPolicyExpired unit tests to make sure that if a certificate decision has
-// expired, the return value from QueryPolicy returns the correct vaule.
+// The same test as ChromeSSLHostStateDelegateTest.QueryPolicyExpired but now
+// applied to a browser context that expires based on time, not restart. This
+// unit tests to make sure that if a certificate decision has expired, the
+// return value from QueryPolicy returns the correct vaule.
 IN_PROC_BROWSER_TEST_F(RememberSSLHostStateDelegateTest, QueryPolicyExpired) {
   scoped_refptr<net::X509Certificate> google_cert = GetGoogleCert();
   content::WebContents* tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
   content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
   bool expired_previous_decision;
 
   // chrome_state takes ownership of this clock
   base::SimpleTestClock* clock = new base::SimpleTestClock();
@@ skipping 76 matching lines @@
   // worth of browsing history and verify that the exception has been deleted.
   state->AllowCert(
       kGoogleHost, *google_cert.get(), net::CERT_STATUS_DATE_INVALID);
   RemoveAndWait(profile);
   EXPECT_EQ(content::SSLHostStateDelegate::DENIED,
             state->QueryPolicy(kGoogleHost,
                                *google_cert.get(),
                                net::CERT_STATUS_DATE_INVALID,
                                &unused_value));
 }