Combine SSLCertErrorHandler and SSLErrorHandler

Combine SSLCertErrorHandler and SSLErrorHandler

SSLCertErrorHandler is the only subclass of SSLErrorHandler, so
splitting them up doesn't serve any purpose.

This is a clean-up on the way to making SSLErrorHandler UI-thread-only
and not-refcounted.

This CL is based on top of https://codereview.chromium.org/2220603003/
(Remove unnecessary |result| argument from AllowCertificateError()) and
followed up by https://codereview.chromium.org/2213193005/ (which makes
SSLErrorHandler UI-thread-only).

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

[estark, 2016-08-08 17:32:03]

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

[estark, 2016-08-08 17:43:33]

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

[estark, 2016-08-08 17:43:34]

Nasko: this is CL 2/3 on the way to making SSLErrorHandler UI-thread-only and
not-refcounted. PTAL -- thank you!

[nasko, 2016-08-08 18:47:12]

LGTM with a few nits.

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
File content/browser/ssl/ssl_error_handler.cc (right):

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.cc:139: delegate_->CancelSSLRequest(error,
&ssl_info());
ssl_info_?

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:104: protected:
Why not remove "protected:"?

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:146: 
nit: No need for empty lines between the members, especially when there are no
comments.

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:149: const int cert_error_;  // The
error we represent.
I know this is a code move, but let's clean things up a bit. Usually comments on
member variables go right above them with an empty line before the comment.

[estark, 2016-08-08 20:02:32]

Patchset #2 (id:20001) has been deleted

[estark, 2016-08-08 20:13:01]

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
File content/browser/ssl/ssl_error_handler.cc (right):

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.cc:139: delegate_->CancelSSLRequest(error,
&ssl_info());
On 2016/08/08 18:47:12, nasko wrote:
> ssl_info_?

Done.

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:104: protected:
On 2016/08/08 18:47:12, nasko wrote:
> Why not remove "protected:"?

Done.

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:146: 
On 2016/08/08 18:47:12, nasko wrote:
> nit: No need for empty lines between the members, especially when there are no
> comments.

I ended up adding comments to each of them for consistency, so I left the empty
lines before each comment to break it up a bit (and consistent with your comment
below).

https://codereview.chromium.org/2222003002/diff/1/content/browser/ssl/ssl_err...
content/browser/ssl/ssl_error_handler.h:149: const int cert_error_;  // The
error we represent.
On 2016/08/08 18:47:12, nasko wrote:
> I know this is a code move, but let's clean things up a bit. Usually comments
on
> member variables go right above them with an empty line before the comment.

Done.

[estark, 2016-08-09 03:18:50]

The CQ bit was checked by estark@chromium.org

[estark, 2016-08-09 03:18:51]

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

[commit-bot: I haz the power, 2016-08-09 03:19:10]

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-08-09 03:21:00]

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

[commit-bot: I haz the power, 2016-08-09 03:21:01]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[estark, 2016-08-09 03:26:08]

The CQ bit was checked by estark@chromium.org

[estark, 2016-08-09 03:26:09]

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

[commit-bot: I haz the power, 2016-08-09 03:26:34]

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-08-09 05:18:30]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-08-09 05:20:13]

Description was changed from

==========
Combine SSLCertErrorHandler and SSLErrorHandler

SSLCertErrorHandler is the only subclass of SSLErrorHandler, so
splitting them up doesn't serve any purpose.

This is a clean-up on the way to making SSLErrorHandler UI-thread-only
and not-refcounted.

This CL is based on top of https://codereview.chromium.org/2220603003/
(Remove unnecessary |result| argument from AllowCertificateError()) and
followed up by https://codereview.chromium.org/2213193005/ (which makes
SSLErrorHandler UI-thread-only).

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

to

==========
Combine SSLCertErrorHandler and SSLErrorHandler

SSLCertErrorHandler is the only subclass of SSLErrorHandler, so
splitting them up doesn't serve any purpose.

This is a clean-up on the way to making SSLErrorHandler UI-thread-only
and not-refcounted.

This CL is based on top of https://codereview.chromium.org/2220603003/
(Remove unnecessary |result| argument from AllowCertificateError()) and
followed up by https://codereview.chromium.org/2213193005/ (which makes
SSLErrorHandler UI-thread-only).

BUG=558491

Committed: https://crrev.com/01e64446f0418ea248649197fa1dbb6d6cdb536c
Cr-Commit-Position: refs/heads/master@{#410582}
==========

[commit-bot: I haz the power, 2016-08-09 05:20:14]

Patchset 4 (id:??) landed as
https://crrev.com/01e64446f0418ea248649197fa1dbb6d6cdb536c
Cr-Commit-Position: refs/heads/master@{#410582}