Add website settings backend v 0.1

chrome/browser/website_settings_model_unittest.cc

+// Copyright (c) 2012 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/website_settings_model.h"

[wtc, 2012/02/15 02:14:29]

Nit: I suggest adding a blank line to separate this header
(which declares the class under test) from the other headers.

[markusheintz_, 2012/02/15 19:04:18]

Done.

+#include "chrome/test/base/testing_profile.h"
+#include "content/public/common/ssl_status.h"
+#include "testing/gtest/include/gtest/gtest.h"

[wtc, 2012/02/15 02:14:29]

Nit: list in alphabetical order.

[markusheintz_, 2012/02/15 19:04:18]

Done.

+#include "net/base/cert_status_flags.h"
+#include "net/base/ssl_connection_status_flags.h"
+
+namespace {
+
+// SSL Cipher Suits like specified in RFC5246 Appendix A.5.

[wtc, 2012/02/15 02:14:29]

Typo: Suits => Suites

[markusheintz_, 2012/02/15 19:04:18]

Done.

+static int TLS_RSA_WITH_NULL_MD5 = 1;
+
+int SetSSLVersion(int connection_status, int version) {
+  // Clear ssl version bits (20, 21, 22);

[Finnur, 2012/02/15 08:59:49]

nit: period, not semicolon. :) Also: s/ssl/SSL/

[markusheintz_, 2012/02/15 19:04:18]

Done.

+  connection_status = connection_status & (3 << 20);

[wtc, 2012/02/15 02:14:29]

Nit: use the &= operator:
  connection_status &= (3 << 20);

BUG: this looks wrong.  I think we need to do
  connection_status &= ~(7 << 20);
because 3 has only two bits and the ~ is necessary.

You should replace 7 with
    net::SSL_CONNECTION_VERSION_MASK
and 20 with
    net::SSL_CONNECTION_VERSION_SHIFT

[markusheintz_, 2012/02/15 19:04:18]

Done. I am ashamed to death

+  int bitmask = version << net::SSL_CONNECTION_VERSION_SHIFT;
+  return bitmask | connection_status;
+}
+
+int SetSSLCipherSuite(int connection_status, int cipher_suite) {
+  // Clear cipher suite bits (the 16 lowest bits).
+  connection_status = connection_status & (~0 << 16);

[wtc, 2012/02/15 02:14:29]

Nit: I suggest you say
  connection_status &= ~net::SSL_CONNECTION_CIPHERSUITE_MASK;

[markusheintz_, 2012/02/15 19:04:18]

Done.

+  int bitmask = 0 | cipher_suite;

[wtc, 2012/02/15 02:14:29]

Remove "0 | ".  Actually you can just say
  return cipher_suit | connection_status;

[markusheintz_, 2012/02/15 19:04:18]

Done.

+  return bitmask | connection_status;
+}
+
+}  // namespace
+
+TEST(WebsiteSettingsModelTest, HTTPConnection) {
+  Profile* profile = new TestingProfile();
+  GURL url = GURL("http://www.example.com");
+  content::SSLStatus ssl;
+  ssl.security_style = content::SECURITY_STYLE_UNAUTHENTICATED;
+
+

[Finnur, 2012/02/15 08:59:49]

nit: Extra line, don't need it.

[markusheintz_, 2012/02/15 19:04:18]

Done.

+  scoped_ptr<WebsiteSettingsModel> model;
+  model.reset(new WebsiteSettingsModel(profile, url, ssl));
+
+  EXPECT_EQ(WebsiteSettingsModel::SITE_CONNECTION_STATUS_UNENCRYPTED,
+            model->site_connection_status());
+}
+
+TEST(WebsiteSettingsModelTest, HTTPSConnection) {
+  Profile* profile = new TestingProfile();
+  GURL url = GURL("http://www.example.com");
+
+  content::SSLStatus ssl;
+  ssl.security_style = content::SECURITY_STYLE_AUTHENTICATED;
+  ssl.cert_id = 1;      // Some id other than 0.
+  ssl.cert_status = 0;  // No errors.
+
+  ssl.security_bits = 81;
+  int status = 0;
+  status = SetSSLVersion(status, net::SSL_CONNECTION_VERSION_TLS1);
+  status = SetSSLCipherSuite(status, TLS_RSA_WITH_NULL_MD5);

[wtc, 2012/02/15 02:14:29]

Nit: TLS_RSA_WITH_NULL_MD5 is a cipher suite that does no
encryption (0 security bits).  So this is a bad example.

[markusheintz_, 2012/02/15 19:04:18]

Done. I'm using a different one now. Maybe you want to double check.

+  ssl.connection_status = status;
+
+  scoped_ptr<WebsiteSettingsModel> model;
+  model.reset(new WebsiteSettingsModel(profile, url, ssl));
+
+  EXPECT_EQ(WebsiteSettingsModel::SITE_CONNECTION_STATUS_ENCRYPTED,
+            model->site_connection_status());

[Finnur, 2012/02/15 08:59:49]

What about site_connection_details, etc?

[markusheintz_, 2012/02/15 19:04:18]

Actually I'd like to change the details from a string to a struct and create the
final string to display in the UI.

So I have no test for the details yet.

If it is fine with you I'll add a test in the second iteration.

+}
+
+TEST(WebsiteSettingsModelTest, HTTPSConnectionMixedContent) {
+// TODO(markusheintz): Implement

[Finnur, 2012/02/15 08:59:49]

Look forward to seeing those. Thanks for adding this!

[markusheintz_, 2012/02/15 19:04:18]

Done.

+}
+
+TEST(WebsiteSettingsModelTest, HTTPSConnectionBadCertificate) {
+// TODO(markusheintz): Implement
+}
+
+TEST(WebsiteSettingsModelTest, HTTPSConnectionEVCert) {
+// TODO(markusheintz): Implement

[Finnur, 2012/02/15 08:59:49]

nit: indentation (see also above)

[markusheintz_, 2012/02/15 19:04:18]

Done.

+}