MockTransportChannel::GetRemoteSSLCertificate: Prepare for base class change

MockTransportChannel::GetRemoteSSLCertificate: Prepare for base class change

See https://codereview.webrtc.org/1802013002/#msg3.
TransportChannel::GetRemoteSSLCertificate is about to change
signature; with this change, we can handle the new one as well as the
old.

BUG=webrtc:5520
Committed: https://crrev.com/2ee995f81fd62b01cc05f4d364343d18106d453d
Cr-Commit-Position: refs/heads/master@{#385405}

[kwiberg-chromium, 2016-04-02 00:52:22]

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

[kwiberg-chromium, 2016-04-02 00:52:22]

This needs to land before https://codereview.webrtc.org/1802013002/.

[Sergey Ulanov, 2016-04-05 18:05:24]

lgtm

https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
File remoting/protocol/channel_socket_adapter_unittest.cc (right):

https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
remoting/protocol/channel_socket_adapter_unittest.cc:61: virtual
rtc::scoped_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate() const {
I think this should be marked as override instead of virtual, and it won't
compile otherwise.

[kwiberg-chromium, 2016-04-06 08:14:51]

https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
File remoting/protocol/channel_socket_adapter_unittest.cc (right):

https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
remoting/protocol/channel_socket_adapter_unittest.cc:61: virtual
rtc::scoped_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate() const {
On 2016/04/05 18:05:23, Sergey Ulanov wrote:
> I think this should be marked as override instead of virtual, and it won't
> compile otherwise.

This code must compile both before and after the parent class has a method to
override, and "override" causes a compilation error in the former case, whereas
"virtual" (as you say) causes a compilation error in the latter case.

However, the cause of the error in the latter case isn't something built-in, but
a Chromium-specific warning. And that warning appears to be written specifically
to not apply to classes that use the gmock MOCK_* macros (for a good reason:
those macros predate C++11, and internally use virtual instead of override).

Relying on this is of course horribly ugly. But the second CL---to be landed
after the WebRTC change---replaces "virtual" with "override".

[kwiberg-chromium, 2016-04-06 08:15:00]

The CQ bit was checked by kwiberg@chromium.org

[commit-bot: I haz the power, 2016-04-06 08:15:17]

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

[commit-bot: I haz the power, 2016-04-06 08:53:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-06 08:55:35]

Description was changed from

==========
MockTransportChannel::GetRemoteSSLCertificate: Prepare for base class change

See https://codereview.webrtc.org/1802013002/#msg3.
TransportChannel::GetRemoteSSLCertificate is about to change
signature; with this change, we can handle the new one as well as the
old.

BUG=webrtc:5520
==========

to

==========
MockTransportChannel::GetRemoteSSLCertificate: Prepare for base class change

See https://codereview.webrtc.org/1802013002/#msg3.
TransportChannel::GetRemoteSSLCertificate is about to change
signature; with this change, we can handle the new one as well as the
old.

BUG=webrtc:5520

Committed: https://crrev.com/2ee995f81fd62b01cc05f4d364343d18106d453d
Cr-Commit-Position: refs/heads/master@{#385405}
==========

[commit-bot: I haz the power, 2016-04-06 08:55:36]

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

[Sergey Ulanov, 2016-04-06 22:36:58]

On 2016/04/06 08:14:51, kwiberg-chromium wrote:
>
https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
> File remoting/protocol/channel_socket_adapter_unittest.cc (right):
> 
>
https://codereview.chromium.org/1847303006/diff/1/remoting/protocol/channel_s...
> remoting/protocol/channel_socket_adapter_unittest.cc:61: virtual
> rtc::scoped_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate() const {
> On 2016/04/05 18:05:23, Sergey Ulanov wrote:
> > I think this should be marked as override instead of virtual, and it won't
> > compile otherwise.
> 
> This code must compile both before and after the parent class has a method to
> override, and "override" causes a compilation error in the former case,
whereas
> "virtual" (as you say) causes a compilation error in the latter case.
> 
> However, the cause of the error in the latter case isn't something built-in,
but
> a Chromium-specific warning. And that warning appears to be written
specifically
> to not apply to classes that use the gmock MOCK_* macros (for a good reason:
> those macros predate C++11, and internally use virtual instead of override).
> 
> Relying on this is of course horribly ugly. But the second CL---to be landed
> after the WebRTC change---replaces "virtual" with "override".

I see. Thanks for the explanation