Refactoring some tests in peerconnectioninterface_unittest.cc.

Refactoring some tests in peerconnectioninterface_unittest.cc.

Some tests were passing in a local description created from hard-coded
SDP strings, which won't work in the future (since some attributes such
as the fingerprint and ICE ufrag/pwd are non-modifiable). These tests
now do the typical approach of calling CreateOffer and modifying the
result if necessary.

Also added some non-const versions of the SessionDescription accessor
helper functions, since that makes it much easier to modify a
SessionDescription. Previous alternatives were re-implementing the
helper methods from scratch, or converting the description to SDP,
modifying it, and converting it back.

R=tommi@webrtc.org

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

[Taylor Brandstetter, 2016-05-11 19:05:24]

deadbeef@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[Taylor Brandstetter, 2016-05-11 19:05:25]

PTAL Peter. This fixes an issue hbos@ encountered in this CL:
https://codereview.webrtc.org/1965723002/

[hbos, 2016-05-12 09:41:43]

Good job, I have confirmed that this patch fixes my issues and would allow me to
do KT_DEFAULT = KT_ECDSA (https://codereview.webrtc.org/1972873002/).

[tommi, 2016-05-12 11:41:43]

tommi@webrtc.org changed reviewers:
+ tommi@webrtc.org

[tommi, 2016-05-12 11:41:44]

nice. lgtm.  Pure speculation below, feel free to ignore.

https://codereview.webrtc.org/1966333002/diff/1/webrtc/api/peerconnectioninte...
File webrtc/api/peerconnectioninterface_unittest.cc (right):

https://codereview.webrtc.org/1966333002/diff/1/webrtc/api/peerconnectioninte...
webrtc/api/peerconnectioninterface_unittest.cc:2391: for (unsigned int& ssrc :
stream.ssrcs) {
nit: since we rarely use unsigned int, and should generally by using int
instead, suggest you just use "auto&" here and below.

https://codereview.webrtc.org/1966333002/diff/1/webrtc/pc/mediasession.cc
File webrtc/pc/mediasession.cc (right):

https://codereview.webrtc.org/1966333002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:1922: for (const ContentInfo& content : contents) {
nit: I wonder if a template like this could cover both cases?

template<typename Infos, typename Info=Infos::value_type>
Info* GetFirstMediaContent(Infos& contents, MediaType media_type) {
  auto found = std::find_if(contents.begin(), contents.end(),
      [media_type](const Info& info) {
          return IsMediaContentOfType(&info, media_type); });
  return found == contents.end() ? nullptr : &*found;
}

[hbos, 2016-05-12 11:49:57]

The CQ bit was checked by hbos@webrtc.org

[commit-bot: I haz the power, 2016-05-12 11:50:02]

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

[hbos, 2016-05-12 11:50:24]

On 2016/05/12 11:50:02, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1966333002/1
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1966333002/1

I pressed the CQ button :3

[tommi, 2016-05-12 11:51:00]

On 2016/05/12 11:50:02, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1966333002/1
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1966333002/1

hbos and I decided to just land this now instead of waiting. Hope that's alright
:)

[commit-bot: I haz the power, 2016-05-12 11:58:54]

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

[commit-bot: I haz the power, 2016-05-12 11:58:55]

Try jobs failed on following builders:
  android_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/13250)

[Taylor Brandstetter, 2016-05-12 15:14:58]

Description was changed from

==========
Refactoring some tests in peerconnectioninterface_unittest.cc.

Some tests were passing in a local description created from hard-coded
SDP strings, which won't work in the future (since some attributes such
as the fingerprint and ICE ufrag/pwd are non-modifiable). These tests
now do the typical approach of calling CreateOffer and modifying the
result if necessary.

Also added some non-const versions of the SessionDescription accessor
helper functions, since that makes it much easier to modify a
SessionDescription. Previous alternatives were re-implementing the
helper methods from scratch, or converting the description to SDP,
modifying it, and converting it back.
==========

to

==========
Refactoring some tests in peerconnectioninterface_unittest.cc.

Some tests were passing in a local description created from hard-coded
SDP strings, which won't work in the future (since some attributes such
as the fingerprint and ICE ufrag/pwd are non-modifiable). These tests
now do the typical approach of calling CreateOffer and modifying the
result if necessary.

Also added some non-const versions of the SessionDescription accessor
helper functions, since that makes it much easier to modify a
SessionDescription. Previous alternatives were re-implementing the
helper methods from scratch, or converting the description to SDP,
modifying it, and converting it back.

R=tommi@webrtc.org

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

[commit-bot: I haz the power, 2016-05-12 15:14:58]

Description was changed from

==========
Refactoring some tests in peerconnectioninterface_unittest.cc.

Some tests were passing in a local description created from hard-coded
SDP strings, which won't work in the future (since some attributes such
as the fingerprint and ICE ufrag/pwd are non-modifiable). These tests
now do the typical approach of calling CreateOffer and modifying the
result if necessary.

Also added some non-const versions of the SessionDescription accessor
helper functions, since that makes it much easier to modify a
SessionDescription. Previous alternatives were re-implementing the
helper methods from scratch, or converting the description to SDP,
modifying it, and converting it back.
==========

to

==========
Refactoring some tests in peerconnectioninterface_unittest.cc.

Some tests were passing in a local description created from hard-coded
SDP strings, which won't work in the future (since some attributes such
as the fingerprint and ICE ufrag/pwd are non-modifiable). These tests
now do the typical approach of calling CreateOffer and modifying the
result if necessary.

Also added some non-const versions of the SessionDescription accessor
helper functions, since that makes it much easier to modify a
SessionDescription. Previous alternatives were re-implementing the
helper methods from scratch, or converting the description to SDP,
modifying it, and converting it back.

R=tommi@webrtc.org

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

[commit-bot: I haz the power, 2016-05-12 15:14:59]

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

[Taylor Brandstetter, 2016-05-12 15:15:00]

Committed patchset #1 (id:1) manually as
dc4eb8c5b3c996389046694d3b4fdb2ab315376f (presubmit successful).

[Taylor Brandstetter, 2016-05-12 15:16:44]

https://codereview.webrtc.org/1966333002/diff/1/webrtc/pc/mediasession.cc
File webrtc/pc/mediasession.cc (right):

https://codereview.webrtc.org/1966333002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:1922: for (const ContentInfo& content : contents) {
On 2016/05/12 11:41:44, tommi-webrtc wrote:
> nit: I wonder if a template like this could cover both cases?
> 
> template<typename Infos, typename Info=Infos::value_type>
> Info* GetFirstMediaContent(Infos& contents, MediaType media_type) {
>   auto found = std::find_if(contents.begin(), contents.end(),
>       [media_type](const Info& info) {
>           return IsMediaContentOfType(&info, media_type); });
>   return found == contents.end() ? nullptr : &*found;
> }

The problem is that if Infos is const, Info still won't be. I haven't found a
way around this yet (I'm not really a template guru). For now I'll just land the
CL and maybe revisit this later.