Cleanups for SSLConfigService and SSLConfigServiceManager.

Make SSLConfig.rev_checking_enable default to true (which also affects the defaults set by SSLConfigServicePref.)

Add static SSLConfigService::CreateSystemSSLConfigService which creates a standalone SSLConfigService (either SSLConfigServiceWin or SSLConfigServiceDefaults.)

Use CreateSystemSSLConfigService in fetch_client and test_shell_request_context.

Merge SSLConfigServiceManagerWin and SSLConfigServiceManagerDefaults into SSLConfigServiceManagerSystem, which uses CreateSystemSSLConfigService.

BUG=11507,19290
TEST=only visible change should be linux defaults to having rev checking option enabled.

Review URL: http://codereview.chromium.org/173097

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@23998 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 61 insertions, 80 deletions

diff --git a/chrome/browser/net/ssl_config_service_manager.h b/chrome/browser/net/ssl_config_service_manager.h
index d2a351f..3a7907c 100644
--- a/chrome/browser/net/ssl_config_service_manager.h
+++ b/chrome/browser/net/ssl_config_service_manager.h
@@ -16,12 +16,12 @@ class SSLConfigServiceManager {
  public:
   // Create an instance of the default SSLConfigServiceManager for the current
   // platform. The lifetime of the profile must be longer than that of the
-  // factory.
+  // manager.
   static SSLConfigServiceManager* CreateDefaultManager(Profile* profile);
 
   virtual ~SSLConfigServiceManager() {}
 
-  // Get an SSLConfigService instance.  It may be a new instance or the factory
+  // Get an SSLConfigService instance.  It may be a new instance or the manager
   // may return the same instance multiple times.
   // The caller should hold a reference as long as it needs the instance (eg,
   // using scoped_refptr.)
diff --git a/chrome/browser/net/ssl_config_service_manager_defaults.cc b/chrome/browser/net/ssl_config_service_manager_defaults.cc
deleted file mode 100644
index a01166e..0000000
--- a/chrome/browser/net/ssl_config_service_manager_defaults.cc
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (c) 2009 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/net/ssl_config_service_manager.h"
-#include "net/base/ssl_config_service_defaults.h"
-
-////////////////////////////////////////////////////////////////////////////////
-//  SSLConfigServiceManagerDefaults
-
-// The factory for creating an SSLConfigServiceDefaults instance.
-class SSLConfigServiceManagerDefaults
-    : public SSLConfigServiceManager {
- public:
-  SSLConfigServiceManagerDefaults()
-      : ssl_config_service_(new net::SSLConfigServiceDefaults()) {
-  }
-  virtual ~SSLConfigServiceManagerDefaults() {}
-
-  virtual net::SSLConfigService* Get() {
-    return ssl_config_service_;
-  }
-
- private:
-  scoped_refptr<net::SSLConfigServiceDefaults> ssl_config_service_;
-
-  DISALLOW_COPY_AND_ASSIGN(SSLConfigServiceManagerDefaults);
-};
-
-////////////////////////////////////////////////////////////////////////////////
-//  SSLConfigServiceManager
-
-// static
-SSLConfigServiceManager* SSLConfigServiceManager::CreateDefaultManager(
-    Profile* profile) {
-  return new SSLConfigServiceManagerDefaults();
-}
diff --git a/chrome/browser/net/ssl_config_service_manager_pref.cc b/chrome/browser/net/ssl_config_service_manager_pref.cc
index 6c1f5f1..0b89b25 100644
--- a/chrome/browser/net/ssl_config_service_manager_pref.cc
+++ b/chrome/browser/net/ssl_config_service_manager_pref.cc
@@ -52,7 +52,7 @@ void SSLConfigServicePref::SetNewSSLConfig(
 ////////////////////////////////////////////////////////////////////////////////
 //  SSLConfigServiceManagerPref
 
-// The factory for creating and updating an SSLConfigServicePref instance.
+// The manager for holding and updating an SSLConfigServicePref instance.
 class SSLConfigServiceManagerPref
     : public SSLConfigServiceManager,
       public NotificationObserver {
diff --git a/chrome/browser/net/ssl_config_service_manager_win.cc b/chrome/browser/net/ssl_config_service_manager_system.cc
index c39c472..15ac023 100644
--- a/chrome/browser/net/ssl_config_service_manager_win.cc
+++ b/chrome/browser/net/ssl_config_service_manager_system.cc
@@ -3,21 +3,23 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/net/ssl_config_service_manager.h"
-#include "net/base/ssl_config_service_win.h"
+#include "net/base/ssl_config_service.h"
 
 class Profile;
 
 ////////////////////////////////////////////////////////////////////////////////
-//  SSLConfigServiceManagerWin
+//  SSLConfigServiceManagerSystem
 
-// The factory for creating an SSLConfigServiceWin instance.
-class SSLConfigServiceManagerWin
+// The manager for holding a system SSLConfigService instance.  System
+// SSLConfigService objects do not depend on the profile.
+class SSLConfigServiceManagerSystem
     : public SSLConfigServiceManager {
  public:
-  SSLConfigServiceManagerWin()
-      : ssl_config_service_(new net::SSLConfigServiceWin) {
+  SSLConfigServiceManagerSystem()
+      : ssl_config_service_(
+          net::SSLConfigService::CreateSystemSSLConfigService()) {
   }
-  virtual ~SSLConfigServiceManagerWin() {}
+  virtual ~SSLConfigServiceManagerSystem() {}
 
   virtual net::SSLConfigService* Get() {
     return ssl_config_service_;
@@ -26,7 +28,7 @@ class SSLConfigServiceManagerWin
  private:
   scoped_refptr<net::SSLConfigService> ssl_config_service_;
 
-  DISALLOW_COPY_AND_ASSIGN(SSLConfigServiceManagerWin);
+  DISALLOW_COPY_AND_ASSIGN(SSLConfigServiceManagerSystem);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -35,5 +37,5 @@ class SSLConfigServiceManagerWin
 // static
 SSLConfigServiceManager* SSLConfigServiceManager::CreateDefaultManager(
     Profile* profile) {
-  return new SSLConfigServiceManagerWin();
+  return new SSLConfigServiceManagerSystem();
 }
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 52fe57b..0b44803 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -1442,7 +1442,6 @@
         'browser/net/sdch_dictionary_fetcher.cc',
         'browser/net/sdch_dictionary_fetcher.h',
         'browser/net/ssl_config_service_manager.h',
-        'browser/net/ssl_config_service_manager_win.cc',
         'browser/net/url_fetcher.cc',
         'browser/net/url_fetcher.h',
         'browser/net/url_fetcher_protect.cc',
@@ -2153,7 +2152,7 @@
             'browser/views/extensions/extension_view.h',
           ],
           'sources': [
-            'browser/net/ssl_config_service_manager_defaults.cc',
+            'browser/net/ssl_config_service_manager_system.cc',
             # Build the necessary GTM sources
             '../third_party/GTM/AppKit/GTMNSBezierPath+RoundRect.m',
             '../third_party/GTM/AppKit/GTMNSColor+Luminance.m',
@@ -2239,6 +2238,7 @@
             '../views/views.gyp:views',
           ],
           'sources': [
+            'browser/net/ssl_config_service_manager_system.cc',
             # Using built-in rule in vstudio for midl.
             'browser/history/history_indexer.idl',
           ],
diff --git a/net/base/ssl_config_service.cc b/net/base/ssl_config_service.cc
new file mode 100644
index 0000000..8fae14e
--- /dev/null
+++ b/net/base/ssl_config_service.cc
@@ -0,0 +1,24 @@
+// Copyright (c) 2009 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 "net/base/ssl_config_service.h"
+
+#if defined(OS_WIN)
+#include "net/base/ssl_config_service_win.h"
+#else
+#include "net/base/ssl_config_service_defaults.h"
+#endif
+
+namespace net {
+
+// static
+SSLConfigService* SSLConfigService::CreateSystemSSLConfigService() {
+#if defined(OS_WIN)
+  return new SSLConfigServiceWin;
+#else
+  return new SSLConfigServiceDefaults;
+#endif
+}
+
+}  // namespace net
diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index 7bfd08b..c8c0638 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -14,10 +14,10 @@ namespace net {
 
 // A collection of SSL-related configuration settings.
 struct SSLConfig {
-  // Default to no revocation checking.
+  // Default to revocation checking.
   // Default to SSL 2.0 off, SSL 3.0 on, and TLS 1.0 on.
   SSLConfig()
-      : rev_checking_enabled(false),  ssl2_enabled(false), ssl3_enabled(true),
+      : rev_checking_enabled(true),  ssl2_enabled(false), ssl3_enabled(true),
         tls1_enabled(true), send_client_cert(false), verify_ev_cert(false) {
   }
 
@@ -60,7 +60,7 @@ struct SSLConfig {
   scoped_refptr<X509Certificate> client_cert;
 };
 
-// The interface for retrieving the system SSL configuration.  This interface
+// The interface for retrieving the SSL configuration.  This interface
 // does not cover setting the SSL configuration, as on some systems, the
 // SSLConfigService objects may not have direct access to the configuration, or
 // live longer than the configuration preferences.
@@ -68,6 +68,13 @@ class SSLConfigService : public base::RefCountedThreadSafe<SSLConfigService> {
  public:
   virtual ~SSLConfigService() {}
 
+  // Create an instance of SSLConfigService which retrieves the configuration
+  // from the system SSL configuration, or an instance of
+  // SSLConfigServiceDefaults if the current system does not have a system SSL
+  // configuration.  Note: this does not handle SSLConfigService implementations
+  // that are not native to their platform, such as preference-backed ones.
+  static SSLConfigService* CreateSystemSSLConfigService();
+
   // May not be thread-safe, should only be called on the IO thread.
   virtual void GetSSLConfig(SSLConfig* config) = 0;
 };
diff --git a/net/base/ssl_config_service_win.cc b/net/base/ssl_config_service_win.cc
index dcb9b89..513681f 100644
--- a/net/base/ssl_config_service_win.cc
+++ b/net/base/ssl_config_service_win.cc
@@ -42,7 +42,7 @@ enum {
 
 SSLConfigServiceWin::SSLConfigServiceWin() : ever_updated_(false) {
   // We defer retrieving the settings until the first call to GetSSLConfig, to
-  // avoid a blocking call on the UI thread.
+  // avoid an expensive call on the UI thread, which could affect startup time.
 }
 
 SSLConfigServiceWin::SSLConfigServiceWin(TimeTicks now) : ever_updated_(false) {
diff --git a/net/base/ssl_config_service_win_unittest.cc b/net/base/ssl_config_service_win_unittest.cc
index 4cf508e..d9f68e2 100644
--- a/net/base/ssl_config_service_win_unittest.cc
+++ b/net/base/ssl_config_service_win_unittest.cc
@@ -13,12 +13,12 @@ namespace {
 class SSLConfigServiceWinTest : public testing::Test {
 };
 
-} // namespace
+}  // namespace
 
 TEST(SSLConfigServiceWinTest, GetNowTest) {
   // Verify that the constructor sets the correct default values.
   net::SSLConfig config;
-  EXPECT_EQ(false, config.rev_checking_enabled);
+  EXPECT_EQ(true, config.rev_checking_enabled);
   EXPECT_EQ(false, config.ssl2_enabled);
   EXPECT_EQ(true, config.ssl3_enabled);
   EXPECT_EQ(true, config.tls1_enabled);
diff --git a/net/http/http_network_layer.h b/net/http/http_network_layer.h
index c920ce1..7253998 100644
--- a/net/http/http_network_layer.h
+++ b/net/http/http_network_layer.h
@@ -55,7 +55,7 @@ class HttpNetworkLayer : public HttpTransactionFactory {
   // The factory we will use to create network sockets.
   ClientSocketFactory* socket_factory_;
 
-  // The host resolver and proxy service that will used when lazily
+  // The host resolver and proxy service that will be used when lazily
   // creating |session_|.
   scoped_refptr<HostResolver> host_resolver_;
   scoped_refptr<ProxyService> proxy_service_;
diff --git a/net/net.gyp b/net/net.gyp
index b373722..9be5194 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -121,6 +121,7 @@
         'base/ssl_cert_request_info.h',
         'base/ssl_client_auth_cache.cc',
         'base/ssl_client_auth_cache.h',
+        'base/ssl_config_service.cc',
         'base/ssl_config_service.h',
         'base/ssl_config_service_defaults.h',
         'base/ssl_config_service_win.cc',
diff --git a/net/tools/fetch/fetch_client.cc b/net/tools/fetch/fetch_client.cc
index f8b89df..ba78e22 100644
--- a/net/tools/fetch/fetch_client.cc
+++ b/net/tools/fetch/fetch_client.cc
@@ -14,11 +14,7 @@
 #include "net/base/host_resolver.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
-#if defined(OS_WIN)
-#include "net/base/ssl_config_service_win.h"
-#else
-#include "net/base/ssl_config_service_defaults.h"
-#endif
+#include "net/base/ssl_config_service.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_request_info.h"
@@ -138,14 +134,10 @@ int main(int argc, char**argv) {
   scoped_refptr<net::HostResolver> host_resolver(
       net::CreateSystemHostResolver());
 
-  scoped_refptr<net::ProxyService> proxy_service(net::ProxyService::CreateNull());
-#if defined(OS_WIN)
+  scoped_refptr<net::ProxyService> proxy_service(
+      net::ProxyService::CreateNull());
   scoped_refptr<net::SSLConfigService> ssl_config_service(
-      new net::SSLConfigServiceWin);
-#else
-  scoped_refptr<net::SSLConfigService> ssl_config_service(
-      new net::SSLConfigServiceDefaults);
-#endif
+      net::SSLConfigService::CreateSystemSSLConfigService());
   net::HttpTransactionFactory* factory = NULL;
   if (use_cache) {
     factory = new net::HttpCache(host_resolver, proxy_service,
diff --git a/webkit/tools/test_shell/test_shell_request_context.cc b/webkit/tools/test_shell/test_shell_request_context.cc
index 4b680d6..b5c86e5 100644
--- a/webkit/tools/test_shell/test_shell_request_context.cc
+++ b/webkit/tools/test_shell/test_shell_request_context.cc
@@ -8,11 +8,7 @@
 
 #include "net/base/cookie_monster.h"
 #include "net/base/host_resolver.h"
-#if defined(OS_WIN)
-#include "net/base/ssl_config_service_win.h"
-#else
-#include "net/base/ssl_config_service_defaults.h"
-#endif
+#include "net/base/ssl_config_service.h"
 #include "net/ftp/ftp_network_layer.h"
 #include "net/proxy/proxy_service.h"
 #include "webkit/glue/webkit_glue.h"
@@ -53,11 +49,7 @@ void TestShellRequestContext::Init(
   host_resolver_ = net::CreateSystemHostResolver();
   proxy_service_ = net::ProxyService::Create(no_proxy ? &proxy_config : NULL,
                                              false, NULL, NULL);
-#if defined(OS_WIN)
-  ssl_config_service_ = new net::SSLConfigServiceWin;
-#else
-  ssl_config_service_ = new net::SSLConfigServiceDefaults;
-#endif
+  ssl_config_service_ = net::SSLConfigService::CreateSystemSSLConfigService();
 
   net::HttpCache *cache;
   if (cache_path.empty()) {