Move SSLIdentity Generate() implementations from .h to .cc file.

Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
R=hbos@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/a3dc79e0721f1e3d115ba9b52bb91cb8375d582c
Cr-Commit-Position: refs/heads/master@{#11632}

[torbjorng (webrtc), 2016-02-16 11:20:35]

Description was changed from

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

BUG=
==========

to

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
==========

[torbjorng (webrtc), 2016-02-16 11:20:35]

torbjorng@webrtc.org changed reviewers:
+ hbos@webrtc.org, tommi@webrtc.org

[torbjorng (webrtc), 2016-02-16 11:54:25]

The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-16 11:54:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1701953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1701953002/1

[tommi, 2016-02-16 12:00:17]

lgtm - I left a couple of comments, feel free to ignore or do something about
them.

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc
File webrtc/base/sslidentity.cc (right):

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:168: SSLIdentity* SSLIdentity::Generate(const
std::string& common_name,
fyi: typically we tag static methods in the .cc file like so:

// static
void MyClass::Foo() {
}

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:170: return OpenSSLIdentity::Generate(common_name,
key_params,
why call into  OpenSSLIdentity here and not the above Generate() function like
before?

[hbos, 2016-02-16 12:02:18]

Know if this solves the bot problem or a best guess?
lgtm with a suggestion, still lgtm if you discard the suggestion.

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc
File webrtc/base/sslidentity.cc (right):

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:178: }
Suggestion: These should call the three-paramed SSLIdentity::Generate (which is
what they did before this patch) over OpenSSLIdentity::Generate directly, which
would mean that if we had other implementations than SSL_USE_OPENSSL you would
only have to override the one with three params considering the other ones are
just meant to default the other params. If you do this, move it out of the #if
SSL_USE_OPENSSL clause.

I know you want to get rid of #if SSL_USE_OPENSSL and the extra layer since we
only have one implementation though, but right now this layer exists and the
above suggestion makes sense.

[torbjorng (webrtc), 2016-02-16 12:22:46]

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc
File webrtc/base/sslidentity.cc (right):

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:168: SSLIdentity* SSLIdentity::Generate(const
std::string& common_name,
On 2016/02/16 12:00:17, tommi-webrtc wrote:
> fyi: typically we tag static methods in the .cc file like so:
> 
> // static
> void MyClass::Foo() {
> }

Done.

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:170: return OpenSSLIdentity::Generate(common_name,
key_params,
On 2016/02/16 12:00:17, tommi-webrtc wrote:
> why call into  OpenSSLIdentity here and not the above Generate() function like
> before?

Mainly to shorten the call chains, but I think the 3 functions' current
similarity make the code a bit more readable.

https://codereview.webrtc.org/1701953002/diff/1/webrtc/base/sslidentity.cc#ne...
webrtc/base/sslidentity.cc:178: }
On 2016/02/16 12:02:18, hbos wrote:
> Suggestion: These should call the three-paramed SSLIdentity::Generate (which
is
> what they did before this patch) over OpenSSLIdentity::Generate directly,
which
> would mean that if we had other implementations than SSL_USE_OPENSSL you would
> only have to override the one with three params considering the other ones are
> just meant to default the other params. If you do this, move it out of the #if
> SSL_USE_OPENSSL clause.
> 
> I know you want to get rid of #if SSL_USE_OPENSSL and the extra layer since we
> only have one implementation though, but right now this layer exists and the
> above suggestion makes sense.

Please see above reply.

[tommi, 2016-02-16 12:24:55]

lgtm - ship it!

[torbjorng (webrtc), 2016-02-16 12:34:08]

Description was changed from

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
==========

to

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
R=hbos@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/a3dc79e0721f1e3d115ba9b52...
==========

[torbjorng (webrtc), 2016-02-16 12:34:10]

Committed patchset #2 (id:20001) manually as
a3dc79e0721f1e3d115ba9b52bb91cb8375d582c (presubmit successful).

[commit-bot: I haz the power, 2016-02-16 12:34:13]

Description was changed from

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
R=hbos@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/a3dc79e0721f1e3d115ba9b52...
==========

to

==========
Move SSLIdentity Generate() implementations from .h to .cc file.

This amends https://codereview.webrtc.org/1683193003/

BUG=
R=hbos@webrtc.org, tommi@webrtc.org

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

[commit-bot: I haz the power, 2016-02-16 12:34:14]

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