Cleanup: More accurate output parameter type for CTVerifier

Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.

BUG=653825
Committed: https://crrev.com/dcec963661499b7e6cb04cb2ecbec343b8ffc2a4
Cr-Commit-Position: refs/heads/master@{#424146}

[Eran Messeri, 2016-10-07 13:20:10]

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

[commit-bot: I haz the power, 2016-10-07 13:20:24]

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

[Eran Messeri, 2016-10-07 13:20:34]

Description was changed from

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.


BUG=653825
==========

to

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.


BUG=653825
==========

[Eran Messeri, 2016-10-07 13:20:34]

eranm@chromium.org changed reviewers:
+ eroman@chromium.org

[Eran Messeri, 2016-10-07 13:21:39]

Eric, please review this small API clean-up change that does not introduce any
functional changes.

[commit-bot: I haz the power, 2016-10-07 13:31:36]

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

[commit-bot: I haz the power, 2016-10-07 13:31:36]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Sleevi, 2016-10-07 14:05:28]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2016-10-07 14:05:29]

LGTM % IWYU fixes

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
File net/quic/chromium/crypto/proof_verifier_chromium.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
net/quic/chromium/crypto/proof_verifier_chromium.cc:235:
&(verify_details_->ct_verify_result.scts), net_log_);
Are the parens necessary here?

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:1769: &(ct_verify_result_.scts), net_log_);
I don't believe these are necessary?

[Sergey Ulanov, 2016-10-07 15:39:22]

sergeyu@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Sergey Ulanov, 2016-10-07 15:39:22]

remoting lgtm

[Eran Messeri, 2016-10-10 13:13:52]

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

[commit-bot: I haz the power, 2016-10-10 13:14:03]

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

[Eran Messeri, 2016-10-10 13:14:08]

Thanks for the quick review all, done.

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
File net/quic/chromium/crypto/proof_verifier_chromium.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
net/quic/chromium/crypto/proof_verifier_chromium.cc:235:
&(verify_details_->ct_verify_result.scts), net_log_);
On 2016/10/07 14:05:28, Ryan Sleevi (slow) wrote:
> Are the parens necessary here?

Done, removed.

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:1769: &(ct_verify_result_.scts), net_log_);
On 2016/10/07 14:05:29, Ryan Sleevi (slow) wrote:
> I don't believe these are necessary?

Done.

[Eran Messeri, 2016-10-10 13:14:09]

Thanks for the quick review all, done.

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
File net/quic/chromium/crypto/proof_verifier_chromium.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/quic/chromium/crypto/pr...
net/quic/chromium/crypto/proof_verifier_chromium.cc:235:
&(verify_details_->ct_verify_result.scts), net_log_);
On 2016/10/07 14:05:28, Ryan Sleevi (slow) wrote:
> Are the parens necessary here?

Done, removed.

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2400183002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:1769: &(ct_verify_result_.scts), net_log_);
On 2016/10/07 14:05:29, Ryan Sleevi (slow) wrote:
> I don't believe these are necessary?

Done.

[Eran Messeri, 2016-10-10 14:11:45]

The CQ bit was unchecked by eranm@chromium.org

[Eran Messeri, 2016-10-10 14:11:50]

The CQ bit was checked by eranm@chromium.org

[Eran Messeri, 2016-10-10 14:11:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyu@chromium.org, rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2400183002/#ps20001
(title: "IWYU, review comments")

[commit-bot: I haz the power, 2016-10-10 14:11:58]

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-10 14:16:28]

Description was changed from

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.


BUG=653825
==========

to

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.


BUG=653825
==========

[commit-bot: I haz the power, 2016-10-10 14:16:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-10 14:18:39]

Description was changed from

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.


BUG=653825
==========

to

==========
Cleanup: More accurate output parameter type for CTVerifier

The CTVerifier interface took a CTVerifyResult* as an output parameter.
This change replaces the output parameter with
SignedCertificateTimestampAndStatusList*, which is the only output all
implementations of the CTVerifier interface could provide.

Other fields in the CTVerifyResult struct are filled in by other entities
and CTVerifier implementations have no ability to fill them in (or any
need to access them at all).
So it is cleaner, and more conservative, to only provide the SCT List
to be filled to the CTVerifier.

BUG=653825

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

[commit-bot: I haz the power, 2016-10-10 14:18:40]

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