Add additional UMA stats for remembering certificate decisions.

chrome/browser/ssl/chrome_ssl_host_state_delegate.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 "base/base64.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
@@ skipping 110 matching lines @@
 // argument dict that is passed in.
 //
 // If create_entries is set to |DoNotCreateDictionaryEntries|,
 // GetValidCertDecisionsDict will return NULL if there is anything invalid about
 // the setting, such as an invalid version or invalid value types (in addition
 // to there not be any values in the dictionary). If create_entries is set to
 // |CreateDictionaryEntries|, if no dictionary is found or the decisions are
 // expired, a new dictionary will be created
 base::DictionaryValue* ChromeSSLHostStateDelegate::GetValidCertDecisionsDict(
     base::DictionaryValue* dict,
-    CreateDictionaryEntriesDisposition create_entries) {
+    CreateDictionaryEntriesDisposition create_entries,
+    bool* expired_previous_decision) {
+  // This needs to be done first in case the method is short circuited by an
+  // early failure.
+  *expired_previous_decision = false;
+
   // Extract the version of the certificate decision structure from the content
   // setting.
   int version;
   bool success = dict->GetInteger(kSSLCertDecisionVersionKey, &version);
   if (!success) {
     if (create_entries == DoNotCreateDictionaryEntries)
       return NULL;
 
     dict->SetInteger(kSSLCertDecisionVersionKey,
                      kDefaultSSLCertDecisionVersion);
@@ skipping 34 matching lines @@
   }
 
   // Check to see if the user's certificate decision has expired.
   // - Expired and |create_entries| is DoNotCreateDictionaryEntries, return
   // NULL.
   // - Expired and |create_entries| is CreateDictionaryEntries, update the
   // expiration time.
   if (should_remember_ssl_decisions_ !=
           ForgetSSLExceptionDecisionsAtSessionEnd &&
       decision_expiration.ToInternalValue() <= now.ToInternalValue()) {
+    expired = true;

[sky, 2014/08/12 16:47:31]

Why did you move this AFAICT if it's only needed after the early return.

[jww, 2014/08/12 19:23:42]

It seemed cleaner to set expired and expired_previous_decision at the same time.
I'll move it back, though, since it is indeed an unnecessary change.

+    *expired_previous_decision = true;
+
     if (create_entries == DoNotCreateDictionaryEntries)
       return NULL;
 
-    expired = true;
-
     base::Time expiration_time =
         now + default_ssl_cert_decision_expiration_delta_;
     // Unfortunately, JSON (and thus content settings) doesn't support int64
     // values, only doubles. Since this mildly depends on precision, it is
     // better to store the value as a string.
     dict->SetString(kSSLCertDecisionExpirationTimeKey,
                     base::Int64ToString(expiration_time.ToInternalValue()));
   }
 
   // Extract the map of certificate fingerprints to errors from the setting.
@@ skipping 49 matching lines @@
 }
 
 void ChromeSSLHostStateDelegate::Clear() {
   profile_->GetHostContentSettingsMap()->ClearSettingsForOneType(
       CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS);
 }
 
 net::CertPolicy::Judgment ChromeSSLHostStateDelegate::QueryPolicy(
     const std::string& host,
     net::X509Certificate* cert,
-    net::CertStatus error) {
+    net::CertStatus error,
+    bool* expired_previous_decision) {
   HostContentSettingsMap* map = profile_->GetHostContentSettingsMap();
   GURL url = GetSecureGURLForHost(host);
   scoped_ptr<base::Value> value(map->GetWebsiteSetting(
       url, url, CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS, std::string(), NULL));
 
+  // Set a default value in case this method is short circuited and doesn't do a
+  // full query.
+  *expired_previous_decision = false;
   if (!value.get() || !value->IsType(base::Value::TYPE_DICTIONARY))
     return net::CertPolicy::UNKNOWN;
 
   base::DictionaryValue* dict;  // Owned by value
   int policy_decision;
   bool success = value->GetAsDictionary(&dict);
   DCHECK(success);
 
   base::DictionaryValue* cert_error_dict;  // Owned by value
-  cert_error_dict =
-      GetValidCertDecisionsDict(dict, DoNotCreateDictionaryEntries);
-  if (!cert_error_dict)
+  cert_error_dict = GetValidCertDecisionsDict(
+      dict, DoNotCreateDictionaryEntries, expired_previous_decision);
+  if (!cert_error_dict) {
+    // This revoke is necessary to clear any old expired setting that may
+    // lingering in the case that an old decision expried.
+    RevokeAllowAndDenyPreferences(host);
     return net::CertPolicy::UNKNOWN;
+  }
 
   success = cert_error_dict->GetIntegerWithoutPathExpansion(GetKey(cert, error),
                                                             &policy_decision);
 
   // If a policy decision was successfully retrieved and it's a valid value of
   // ALLOWED or DENIED, return the valid value. Otherwise, return UNKNOWN.
   if (success && policy_decision == net::CertPolicy::Judgment::ALLOWED)
     return net::CertPolicy::Judgment::ALLOWED;
   else if (success && policy_decision == net::CertPolicy::Judgment::DENIED)
     return net::CertPolicy::Judgment::DENIED;
@@ skipping 59 matching lines @@
   scoped_ptr<base::Value> value(map->GetWebsiteSetting(
       url, url, CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS, std::string(), NULL));
 
   if (!value.get() || !value->IsType(base::Value::TYPE_DICTIONARY))
     value.reset(new base::DictionaryValue());
 
   base::DictionaryValue* dict;
   bool success = value->GetAsDictionary(&dict);
   DCHECK(success);
 
-  base::DictionaryValue* cert_dict =
-      GetValidCertDecisionsDict(dict, CreateDictionaryEntries);
+  bool expired_previous_decision;  // unused value in this function
+  base::DictionaryValue* cert_dict = GetValidCertDecisionsDict(
+      dict, CreateDictionaryEntries, &expired_previous_decision);
   // If a a valid certificate dictionary cannot be extracted from the content
   // setting, that means it's in an unknown format. Unfortunately, there's
   // nothing to be done in that case, so a silent fail is the only option.
   if (!cert_dict)
     return;
 
   dict->SetIntegerWithoutPathExpansion(kSSLCertDecisionVersionKey,
                                        kDefaultSSLCertDecisionVersion);
   cert_dict->SetIntegerWithoutPathExpansion(GetKey(cert, error), judgment);
 
   // The map takes ownership of the value, so it is released in the call to
   // SetWebsiteSetting.
   map->SetWebsiteSetting(pattern,
                          pattern,
                          CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS,
                          std::string(),
                          value.release());
 }