Fix UMA stat for expiration of certificate memory 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 <set>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/guid.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 17 matching lines @@
     "RememberCertificateErrorDecisions";
 const char kRememberCertificateErrorDecisionsFieldTrialDefaultGroup[] =
     "Default";
 const char kRememberCertificateErrorDecisionsFieldTrialLengthParam[] = "length";
 
 // Keys for the per-site error + certificate finger to judgment content
 // settings map.
 const char kSSLCertDecisionCertErrorMapKey[] = "cert_exceptions_map";
 const char kSSLCertDecisionExpirationTimeKey[] = "decision_expiration_time";
 const char kSSLCertDecisionVersionKey[] = "version";
+const char kSSLCertDecisionGUIDKey[] = "guid";
 
 const int kDefaultSSLCertDecisionVersion = 1;
 
 void CloseIdleConnections(
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
   url_request_context_getter->
       GetURLRequestContext()->
       http_transaction_factory()->
       GetSession()->
       CloseIdleConnections();
@@ skipping 151 matching lines @@
       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()));
+  } else if (should_remember_ssl_decisions_ ==
+             FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END) {
+    if (dict->HasKey(kSSLCertDecisionGUIDKey)) {
+      std::string old_expiration_guid;
+      success = dict->GetString(kSSLCertDecisionGUIDKey, &old_expiration_guid);
+      if (old_expiration_guid.compare(current_expiration_guid_) != 0) {
+        *expired_previous_decision = true;
+        expired = true;
+      }
+    }
   }
 
+  dict->SetString(kSSLCertDecisionGUIDKey, current_expiration_guid_);
+
   // Extract the map of certificate fingerprints to errors from the setting.
   base::DictionaryValue* cert_error_dict = NULL;  // Will be owned by dict
   if (expired ||
       !dict->GetDictionary(kSSLCertDecisionCertErrorMapKey, &cert_error_dict)) {
     if (create_entries == DO_NOT_CREATE_DICTIONARY_ENTRIES)
       return NULL;
 
     cert_error_dict = new base::DictionaryValue();
     // dict takes ownership of cert_error_dict
     dict->Set(kSSLCertDecisionCertErrorMapKey, cert_error_dict);
   }
 
   return cert_error_dict;
 }
 
 // If |should_remember_ssl_decisions_| is
 // FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END, that means that all invalid
-// certificate proceed decisions should be forgotten when the session ends. At
-// attempt is made in the destructor to remove the entries, but in the case that
-// things didn't shut down cleanly, on start, Clear is called to guarantee a
-// clean state.
+// certificate proceed decisions should be forgotten when the session ends. To
+// simulate that, Chrome keeps track of a guid to represent the current browser
+// session and stores it in decision entries. See the comment for
+// |current_expiration_guid_| for more information.
 ChromeSSLHostStateDelegate::ChromeSSLHostStateDelegate(Profile* profile)
-    : clock_(new base::DefaultClock()), profile_(profile) {
+    : clock_(new base::DefaultClock()),
+      profile_(profile),
+      current_expiration_guid_(base::GenerateGUID()) {

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

This comment is not actionable, it's just a note for posterity:

ChromeSSLHostStateDelegateFactory is a singleton that matches one
ChromeSSLHostStateDelegate to each profile. That means it's fine to generate
base::GenerateGUID() here; it will only be done once per session, since
ChromeSSLHostStateDelegate will only be instantiated once per session per
profile.

   int64 expiration_delta = GetExpirationDelta();
   if (expiration_delta == kForgetAtSessionEndSwitchValue) {
     should_remember_ssl_decisions_ =
         FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END;
     expiration_delta = 0;
-    Clear();
   } else {
     should_remember_ssl_decisions_ = REMEMBER_SSL_EXCEPTION_DECISIONS_FOR_DELTA;
   }
   default_ssl_cert_decision_expiration_delta_ =
       base::TimeDelta::FromSeconds(expiration_delta);
 }
 
 ChromeSSLHostStateDelegate::~ChromeSSLHostStateDelegate() {
-  if (should_remember_ssl_decisions_ ==
-      FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END)
-    Clear();
 }
 
 void ChromeSSLHostStateDelegate::AllowCert(const std::string& host,
                                            const net::X509Certificate& cert,
                                            net::CertStatus error) {
   GURL url = GetSecureGURLForHost(host);
   const ContentSettingsPattern pattern =
       ContentSettingsPattern::FromURLNoWildcard(url);
   HostContentSettingsMap* map = profile_->GetHostContentSettingsMap();
   scoped_ptr<base::Value> value(map->GetWebsiteSetting(
@@ skipping 147 matching lines @@
 }
 
 bool ChromeSSLHostStateDelegate::DidHostRunInsecureContent(
     const std::string& host,
     int pid) const {
   return !!ran_insecure_content_hosts_.count(BrokenHostEntry(host, pid));
 }
 void ChromeSSLHostStateDelegate::SetClock(scoped_ptr<base::Clock> clock) {
   clock_.reset(clock.release());
 }