Move SecurityLevel into a class of its own

chrome/browser/ssl/security_level_policy.h

Index: chrome/browser/ssl/security_level_policy.h
diff --git a/chrome/browser/ssl/security_level_policy.h b/chrome/browser/ssl/security_level_policy.h
new file mode 100644
index 0000000000000000000000000000000000000000..9d823bf7c68f1e9645dcde1ba8042d04459b0ea3
--- /dev/null
+++ b/chrome/browser/ssl/security_level_policy.h
@@ -0,0 +1,54 @@
+// Copyright 2015 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_SECURITY_LEVEL_POLICY_H_
+#define CHROME_BROWSER_SSL_SECURITY_LEVEL_POLICY_H_
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+// This class is responsible for computing the security level of a page.
+class SecurityLevelPolicy {

[felt, 2015/05/07 20:10:18]

While this is being moved anyway:

What do you think about renaming this ConnectionSecurityLevelPolicy, or
something else that makes it clear that this refers specifically to the HTTP[S]
status? "SecurityLevelPolicy" doesn't reflect whether a page has been flagged as
phishing or malware, which is also "Security".

[Peter Kasting, 2015/05/07 21:35:37]

Another possibility would be to eliminate the wrapping class here and then
rename the enum ConnectionSecurityLevel, leaving it in the global scope.  (I
don't love globals, but I also don't love using classes just to be
pseudo-namespaces.)

[estark, 2015/05/08 04:54:32]

I think I'd like to leave it as a class, if that's alright. My thinking is that
the next step in this project is to add a method here called
|ExplainSecurityLevel|. At that point it might make sense to make it a real
instantiable class. (i.e. you can construct a |SecurityLevelPolicy| object by
passing a |WebContents*| to the constructor, and then you can call
|GetSecurityLevel| and |ExplainSecurityLevel| on the object, and it will
probably make sense to factor out some logic into private methods). Or I could
go ahead and make it instantiable now in anticipation of the next step, if that
would be preferable.

I'm planning on renaming the class to ConnectionSecurityPolicy since
ConnectionSecurityLevelPolicy seems just a tad too long to me.

[Peter Kasting, 2015/05/08 06:38:00]

Leaving as a class is OK given your plans.  I wouldn't make it instantiable yet,
let's wait and do that when there are consumers that want that.
I'm not really sure where the "policy" part of the name comes from anyway.  This
class isn't about setting or enforcing security policies, but simply describing
the existing security level.  If anything maybe the class should be
ConnectionSecurityUtils or something.

[estark, 2015/05/08 14:16:00]

I was thinking of "SHA1 signature in chain => page should be treated as having a
degraded security status" as a policy of sorts, but I'm not too attached to it,
especially if "policy" is likely to be confusing to other people. I'm fine with
ConnectionSecurityUtils, or maybe ConnectionSecurityHelper?

[estark, 2015/05/09 02:29:08]

Renamed to ConnectionSecurityHelper.

+ public:
+  // TODO(wtc): unify ToolbarModel::SecurityLevel with SecurityStyle.  We

[Peter Kasting, 2015/05/07 21:35:37]

This CL would probably be a good time to either implement this TODO, or decide
that it makes no sense and remove it.

+  // don't need two sets of security UI levels.  SECURITY_STYLE_AUTHENTICATED
+  // needs to be refined into three levels: warning, standard, and EV.
+  //
+  // A Java counterpart will be generated for this enum.
+  // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.ssl
+  // GENERATED_JAVA_CLASS_NAME_OVERRIDE: SecurityLevelPolicySecurityLevel
+  enum SecurityLevel {
+    // HTTP/no URL/user is editing
+    NONE = 0,
+
+    // HTTPS with valid EV cert
+    EV_SECURE = 1,
+
+    // HTTPS (non-EV)
+    SECURE = 2,
+
+    // HTTPS, but unable to check certificate revocation status or with insecure
+    // content on the page
+    SECURITY_WARNING = 3,
+
+    // HTTPS, but the certificate verification chain is anchored on a
+    // certificate that was installed by the system administrator
+    SECURITY_POLICY_WARNING = 4,
+
+    // Attempted HTTPS and failed, page not authenticated
+    SECURITY_ERROR = 5,
+
+    NUM_SECURITY_LEVELS = 6,
+  };
+
+  static SecurityLevel GetSecurityLevelForWebContents(
+      content::WebContents* web_contents);
+
+ private:
+  SecurityLevelPolicy();

[Peter Kasting, 2015/05/07 21:35:37]

Use DISALLOW_IMPLICIT_CONSTRUCTORS here.

[estark, 2015/05/09 02:29:08]

Done.

+  ~SecurityLevelPolicy();
+};
+
+#endif  // CHROME_BROWSER_SSL_SECURITY_LEVEL_POLICY_H_