Collapse SSLPolicy/SSLPolicyBackend into SSLManager

Collapse SSLPolicy/SSLPolicyBackend into SSLManager

SSLManager, SSLPolicy, and SSLPolicyBackend all pretty much ferry events
from one to the other, without any clear division of responsibility
among them. This CL collapses all the functionality into SSLManager for
clearer code.

BUG=558491
Committed: https://crrev.com/f83ac1214add3b33a716f090415e5a87910c7273
Cr-Commit-Position: refs/heads/master@{#423240}

[estark, 2016-10-04 23:20:02]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 23:20:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 23:52:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 23:52:02]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[estark, 2016-10-05 00:26:24]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 00:26:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estark, 2016-10-05 00:27:56]

estark@chromium.org changed reviewers:
+ jam@chromium.org

[estark, 2016-10-05 00:27:57]

jam, PTAL? I think we've discussed before that the
SSLManager/SSLPolicy/SSLPolicyBackend separation doesn't seem to do much good,
so I'm collapsing them down into one class (SSLManager).

https://codereview.chromium.org/2395663002/diff/40001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.cc (left):

https://codereview.chromium.org/2395663002/diff/40001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.cc:176: // TODO(abarth): Make sure our redirect
behavior is correct.  If we ever see a
I added a browser test in ssl_browser_tests.cc to check that we do the correct
thing here. (Blink catches redirects and passes them through
MixedContentChecker.)

[commit-bot: I haz the power, 2016-10-05 01:28:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 01:28:23]

Dry run: Exceeded global retry quota

[estark, 2016-10-05 02:56:40]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 02:57:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 03:58:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 03:58:47]

Dry run: This issue passed the CQ dry run.

[jam, 2016-10-05 16:36:03]

lgtm

agree this much easier to understand than before, thanks.

https://codereview.chromium.org/2395663002/diff/60001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_browser_tests.cc (right):

https://codereview.chromium.org/2395663002/diff/60001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_browser_tests.cc:261: std::string encodeQueryStr(const
std::string& query) {
nit: per style guide, begin with capital. also, str shouldn't be shortened per
style guide. so either EncodeQueryString, or maybe just EncodeQuery alone

https://codereview.chromium.org/2395663002/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.h (right):

https://codereview.chromium.org/2395663002/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.h:31: // The SSLManager  controls the SSL UI
elements in a WebContents.  It
nit: extra space after SSLManager

[estark, 2016-10-05 16:52:31]

The CQ bit was checked by estark@chromium.org

[estark, 2016-10-05 16:52:31]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2395663002/#ps80001
(title: "jam comments")

[commit-bot: I haz the power, 2016-10-05 16:52:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estark, 2016-10-05 16:53:06]

Thanks jam!

https://codereview.chromium.org/2395663002/diff/60001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_browser_tests.cc (right):

https://codereview.chromium.org/2395663002/diff/60001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_browser_tests.cc:261: std::string encodeQueryStr(const
std::string& query) {
On 2016/10/05 16:36:02, jam wrote:
> nit: per style guide, begin with capital. also, str shouldn't be shortened per
> style guide. so either EncodeQueryString, or maybe just EncodeQuery alone

Done.

https://codereview.chromium.org/2395663002/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.h (right):

https://codereview.chromium.org/2395663002/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.h:31: // The SSLManager  controls the SSL UI
elements in a WebContents.  It
On 2016/10/05 16:36:02, jam wrote:
> nit: extra space after SSLManager

Done.

[commit-bot: I haz the power, 2016-10-05 17:58:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 17:58:40]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[estark, 2016-10-05 18:01:13]

The CQ bit was checked by estark@chromium.org

[estark, 2016-10-05 18:01:14]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2395663002/#ps100001
(title: "remove accidentally added temp file... oops...")

[commit-bot: I haz the power, 2016-10-05 18:02:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 19:06:27]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-05 19:08:45]

Description was changed from

==========
Collapse SSLPolicy/SSLPolicyBackend into SSLManager

SSLManager, SSLPolicy, and SSLPolicyBackend all pretty much ferry events
from one to the other, without any clear division of responsibility
among them. This CL collapses all the functionality into SSLManager for
clearer code.

BUG=558491
==========

to

==========
Collapse SSLPolicy/SSLPolicyBackend into SSLManager

SSLManager, SSLPolicy, and SSLPolicyBackend all pretty much ferry events
from one to the other, without any clear division of responsibility
among them. This CL collapses all the functionality into SSLManager for
clearer code.

BUG=558491

Committed: https://crrev.com/f83ac1214add3b33a716f090415e5a87910c7273
Cr-Commit-Position: refs/heads/master@{#423240}
==========

[commit-bot: I haz the power, 2016-10-05 19:08:50]

Patchset 6 (id:??) landed as
https://crrev.com/f83ac1214add3b33a716f090415e5a87910c7273
Cr-Commit-Position: refs/heads/master@{#423240}