Cleanup of SSLHostStateDelegate and related code

chrome/browser/ssl/chrome_ssl_host_state_delegate.h

 // 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.
 
 #ifndef CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DELEGATE_H_
 #define CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DELEGATE_H_
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
@@ skipping 11 matching lines @@
 // specified site, SSL fingerprint, and error. If the user makes a decision,
 // stores the decision until either the session ends or for a length of time
 // (across session restarts), based on command line flags.
 class ChromeSSLHostStateDelegate : public content::SSLHostStateDelegate {
  public:
   explicit ChromeSSLHostStateDelegate(Profile* profile);
   virtual ~ChromeSSLHostStateDelegate();
 
   // SSLHostStateDelegate:
   virtual void DenyCert(const std::string& host,
-                        net::X509Certificate* cert,
-                        net::CertStatus error) OVERRIDE;
+                        const net::X509Certificate* cert,

[willchan no longer on Chromium, 2014/09/04 00:02:38]

Can/should this be a const reference rather than a pointer to a const object?
Can it ever be NULL? In the implementation of this function, there's no NULL
check, so I assume it can't be NULL, or else there's a bug.

[jww, 2014/09/04 18:41:49]

I believe that historically this was a pointer because the origin of the cert is
from SSLInfo, which gives the cert as a pointer. But I think you are correct
that it would be a bug for it to be NULL, so I'll make this a const ref.

+                        const net::CertStatus error) OVERRIDE;
   virtual void AllowCert(const std::string& host,
-                         net::X509Certificate* cert,
-                         net::CertStatus error) OVERRIDE;
+                         const net::X509Certificate* cert,
+                         const net::CertStatus error) OVERRIDE;
   virtual void Clear() OVERRIDE;
   virtual net::CertPolicy::Judgment QueryPolicy(
       const std::string& host,
-      net::X509Certificate* cert,
-      net::CertStatus error,
+      const net::X509Certificate* cert,
+      const net::CertStatus error,
       bool* expired_previous_decision) OVERRIDE;
   virtual void HostRanInsecureContent(const std::string& host,
-                                      int pid) OVERRIDE;
+                                      const int pid) OVERRIDE;

[willchan no longer on Chromium, 2014/09/04 00:02:38]

Nit: I'd remove this const. The reason is it's copy by value anyway, so making
it const doesn't really protect anything.

I just noticed that net::CertStatus is the same. It's a POD type, so it's copy
by value too, so no need to make it const.

[jww, 2014/09/04 18:41:49]

Done.

   virtual bool DidHostRunInsecureContent(const std::string& host,
-                                         int pid) const OVERRIDE;
+                                         const int pid) const OVERRIDE;
 
   // ChromeSSLHostStateDelegate implementation:
   // Revoke all user decisions for |host| in the given Profile. The
   // RevokeUserDecisionsHard version may close idle connections in the process.
   // This version should be used *only* for rare events, such as a user
   // controlled button, as it may be very disruptive to the networking stack.
   virtual void RevokeUserDecisions(const std::string& host);
   virtual void RevokeUserDecisionsHard(const std::string& host);
 
   // Returns true if any decisions has been recorded for |host| for the given
   // Profile, otherwise false.
-  virtual bool HasUserDecision(const std::string& host);
-
-  // Called on the UI thread when the profile is about to be destroyed.
-  void ShutdownOnUIThread() {}
+  virtual bool HasUserDecision(const std::string& host) const;
 
  protected:
   // SetClock takes ownership of the passed in clock.
   void SetClock(scoped_ptr<base::Clock> clock);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ForgetInstantlySSLHostStateDelegateTest,
                            MakeAndForgetException);
   FRIEND_TEST_ALL_PREFIXES(RememberSSLHostStateDelegateTest, AfterRestart);
   FRIEND_TEST_ALL_PREFIXES(RememberSSLHostStateDelegateTest,
                            QueryPolicyExpired);
 
   // Used to specify whether new content setting entries should be created if
   // they don't already exist when querying the user's settings.
   enum CreateDictionaryEntriesDisposition {
-    CreateDictionaryEntries,
-    DoNotCreateDictionaryEntries
+    CREATE_DICTIONARY_ENTRIES,
+    DO_NOT_CREATE_DICTIONARY_ENTRIES
   };
 
   // Specifies whether user SSL error decisions should be forgetten at the end
   // of this current session (the old style of remembering decisions), or
   // whether they should be remembered across session restarts for a specified
   // length of time, deteremined by
   // |default_ssl_cert_decision_expiration_delta_|.
   enum RememberSSLExceptionDecisionsDisposition {
-    ForgetSSLExceptionDecisionsAtSessionEnd,
-    RememberSSLExceptionDecisionsForDelta
+    FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END,
+    REMEMBER_SSL_EXCEPTION_DECISIONS_FOR_DELTA
   };
 
   // Modify the user's content settings to specify a judgement made for a
   // specific site and certificate, where |url| is the site in question, |cert|
   // is the certificate with an error, |error| is the error in the certificate,
   // and |judgement| is the user decision to be recorded.
   void ChangeCertPolicy(const std::string& host,
-                        net::X509Certificate* cert,
+                        const net::X509Certificate* cert,
                         net::CertStatus error,
                         net::CertPolicy::Judgment judgment);
 
   // Query the content settings to retrieve a dictionary of certificate
   // fingerprints and errors of certificates to user decisions, as set by
   // ChangeCertPolicy. Returns NULL on a failure.
   //
   // |dict| specifies the user's full exceptions dictionary for a specific site
   // in their content settings. Must be retrieved directly from a website
   // setting in the the profile's HostContentSettingsMap.
@@ skipping 21 matching lines @@
 
   // Hosts which have been contaminated with insecure content in the
   // specified process.  Note that insecure content can travel between
   // same-origin frames in one processs but cannot jump between processes.
   std::set<BrokenHostEntry> ran_insecure_content_hosts_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeSSLHostStateDelegate);
 };
 
 #endif  // CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DELEGATE_H_