Restoring behavior where PeerConnection tracks changes to MediaStreams.

Restoring behavior where PeerConnection tracks changes to MediaStreams.

If a MediaStream is added to a PeerConnection, and later a track
is added to the MediaStream, a new RtpSender will now be created for
that track, and it will appear in subsequent offers.
Similarly, removed tracks will remove RtpSenders.

BUG=webrtc:5265
Committed: https://crrev.com/eb45981165f982dd51425fad5ecb7ea9619063d3
Cr-Commit-Position: refs/heads/master@{#11040}

[Taylor Brandstetter, 2015-12-08 02:19:42]

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

[pthatcher1, 2015-12-08 21:31:05]

lgtm, with some ideas for readability

Well done, by the way.  I like how cleanly you implemented this.

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:773: const
rtc::scoped_ptr<MediaStreamObserver>& observer) -> bool {
I think you can leave off the "-> bool" (it can be inferred).

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:778: }
I think you can use std::remove_if to save a few lines and a few cycles.

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:1384: if (sender == senders_.end()) {
Might as well use an early return, here and in OnVideoTrackEnded.

if (sender != senders_.end()) {
  // We already have a sender for this track, so just change the stream_id
  // so that it's correct in the next call to CreateOffer.
  (*sender)->set_stream_id(stream->label());
  return;
}

// Normal case; we've never seen this track before.
...

[Taylor Brandstetter, 2015-12-09 00:36:02]

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:773: const
rtc::scoped_ptr<MediaStreamObserver>& observer) -> bool {
On 2015/12/08 21:31:05, pthatcher1 wrote:
> I think you can leave off the "-> bool" (it can be inferred).

Done.

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:778: }
On 2015/12/08 21:31:05, pthatcher1 wrote:
> I think you can use std::remove_if to save a few lines and a few cycles.

Done.

https://codereview.webrtc.org/1507973003/diff/20001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:1384: if (sender == senders_.end()) {
On 2015/12/08 21:31:05, pthatcher1 wrote:
> Might as well use an early return, here and in OnVideoTrackEnded.
> 
> if (sender != senders_.end()) {
>   // We already have a sender for this track, so just change the stream_id
>   // so that it's correct in the next call to CreateOffer.
>   (*sender)->set_stream_id(stream->label());
>   return;
> }
> 
> // Normal case; we've never seen this track before.
> ...

Done.

[pthatcher1, 2015-12-09 00:59:23]

lgtm

[Taylor Brandstetter, 2015-12-10 18:39:32]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2015-12-10 18:39:33]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1507973003/#ps60001 (title:
"Forgot to call erase with remove_if.")

[commit-bot: I haz the power, 2015-12-10 18:39:40]

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

[commit-bot: I haz the power, 2015-12-10 18:47:34]

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

[commit-bot: I haz the power, 2015-12-10 18:47:34]

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

[Taylor Brandstetter, 2015-12-10 22:00:45]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2015-12-10 22:00:45]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1507973003/#ps80001 (title:
"Fixing formatting.")

[commit-bot: I haz the power, 2015-12-10 22:00:49]

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

[commit-bot: I haz the power, 2015-12-10 22:13:06]

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

[commit-bot: I haz the power, 2015-12-10 22:13:06]

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

[Taylor Brandstetter, 2015-12-16 02:02:25]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2015-12-16 02:02:26]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1507973003/#ps100001 (title:
"Fixing merge conflicts.")

[commit-bot: I haz the power, 2015-12-16 02:02:38]

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

[commit-bot: I haz the power, 2015-12-16 03:24:48]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-16 03:24:58]

Description was changed from

==========
Restoring behavior where PeerConnection tracks changes to MediaStreams.

If a MediaStream is added to a PeerConnection, and later a track
is added to the MediaStream, a new RtpSender will now be created for
that track, and it will appear in subsequent offers.
Similarly, removed tracks will remove RtpSenders.

BUG=webrtc:5265
==========

to

==========
Restoring behavior where PeerConnection tracks changes to MediaStreams.

If a MediaStream is added to a PeerConnection, and later a track
is added to the MediaStream, a new RtpSender will now be created for
that track, and it will appear in subsequent offers.
Similarly, removed tracks will remove RtpSenders.

BUG=webrtc:5265

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

[commit-bot: I haz the power, 2015-12-16 03:24:59]

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