Add back prefs::kSSL3Enabled and prefs::kTLS1Enabled, but control

Add back prefs::kSSL3Enabled and prefs::kTLS1Enabled, but control
the preferences with the command-line options via the
CommandLinePrefStore. This allows us to control the preferences
via the PolicyPrefStores or any other pref store in the future.

Patch originally by wtc@chromium.org at http://codereview.chromium.org/7776002

R=wtc@chromium.org
BUG=102019
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108073

[Ryan Sleevi, 2011-10-27 03:04:15]

wtc: With test.

[wtc, 2011-10-27 23:59:47]

rsleevi: thanks for the patch!

Please open a new bug, set Mstone-17, assign it to yourself,
and reference it in the CL's commit message.  Bug 93554 has
been closed.

I will review this tomorrow or this weekend.

[wtc, 2011-10-31 19:48:23]

LGTM.  Excellent work!

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
File chrome/browser/net/ssl_config_service_manager_pref_unittest.cc (left):

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:32:
SSLConfigServiceManager::RegisterPrefs(pref_service_.get());

Good cleanup!

Why doesn't this class need the pref_service_ member?

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
File chrome/browser/net/ssl_config_service_manager_pref_unittest.cc (right):

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:111:
TestingPrefStore* user_prefs = new TestingPrefStore;

Is user_prefs heap-allocated because the builder.WithUserPrefs()
call below will take ownership of it?

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:124: bool
is_tls1_enabled = false;

Nit: perhaps initialize is_ssl3_enabled and is_tls1_enabled
to true to strengthen these tests?

[Ryan Sleevi, 2011-11-01 03:01:19]

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
File chrome/browser/net/ssl_config_service_manager_pref_unittest.cc (left):

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:32:
SSLConfigServiceManager::RegisterPrefs(pref_service_.get());
On 2011/10/31 19:48:24, wtc wrote:
> 
> Good cleanup!
> 
> Why doesn't this class need the pref_service_ member?

It's only used by two of the tests as a member (GoodDisabled*/BadDisabled*), so
I moved it to be a stack variable for those.

The two remaining tests (IgnoreLegacy, CommandLineOverrides) use the builder,
and thus don't use a TestingPrefService

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
File chrome/browser/net/ssl_config_service_manager_pref_unittest.cc (right):

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:111:
TestingPrefStore* user_prefs = new TestingPrefStore;
On 2011/10/31 19:48:24, wtc wrote:
> 
> Is user_prefs heap-allocated because the builder.WithUserPrefs()
> call below will take ownership of it?

Right. |user_prefs| is RefCounted, and WithUserPrefs will AddRef (which in this
case, assumes ownership). The only other tests using the Builder used this
pattern as well.

However, to be explicit, I've made them both scoped_refptr<>'s, so it behaves
fine and won't leak on any test failures (though they're all EXPECT()s before
the ownership transfer)

http://codereview.chromium.org/8402019/diff/1/chrome/browser/net/ssl_config_s...
chrome/browser/net/ssl_config_service_manager_pref_unittest.cc:124: bool
is_tls1_enabled = false;
On 2011/10/31 19:48:24, wtc wrote:
> 
> Nit: perhaps initialize is_ssl3_enabled and is_tls1_enabled
> to true to strengthen these tests?

Doh, I even had that originally. Done.

[commit-bot: I haz the power, 2011-11-01 03:02:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/8402019/8001

[commit-bot: I haz the power, 2011-11-01 04:02:29]

Change committed as 108073