DTLS-SRTP set up is bypassed when the channel has been writable.

DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint update. What happened is that during PrAnswer, we incorrectly do not apply bundle. However, the channel has become writable at that time. When Answer comes, we still reset the srtp_filter but since the channel has been writable, the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has been writable. We'll address the issue that bundle should apply even in PrAnswer in a different CL.

BUG=568734
Committed: https://crrev.com/4638331fd8857b263bb65f12dbf5e1f7005e1a9a
Cr-Commit-Position: refs/heads/master@{#11075}

[guoweis_webrtc, 2015-12-16 20:36:14]

Description was changed from

==========
crbug.com/568734


fix

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

to

==========
DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint
update. What happened is that during PrAnswer, we incorrectly do not apply
bundle. However, the channel has become writable at that time. When Answer
comes, we still reset the srtp_filter but since the channel has been writable,
the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has
been writable. We'll address the issue that bundle should apply even in PrAnswer
in a different CL.

BUG=568734
==========

[guoweis_webrtc, 2015-12-16 20:36:14]

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

[guoweis_webrtc, 2015-12-16 20:36:57]

On 2015/12/16 20:36:14, guoweis wrote:
> mailto:guoweis@webrtc.org changed reviewers:
> + mailto:pthatcher@webrtc.org

PTAL.

[pthatcher1, 2015-12-17 23:38:54]

lgtm, with comment nits

https://codereview.webrtc.org/1532543003/diff/2/talk/app/webrtc/peerconnectio...
File talk/app/webrtc/peerconnection_unittest.cc (right):

https://codereview.webrtc.org/1532543003/diff/2/talk/app/webrtc/peerconnectio...
talk/app/webrtc/peerconnection_unittest.cc:1235: // This test sets up a
non-bundle call and apply bundle during ICE restart. When
apply => applies
during ICE restart => during an ICE restart

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
File talk/session/media/channel.cc (right):

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
talk/session/media/channel.cc:269: // DTLS-SRTP again.
could => can
again => when the writable_ becomes true again

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
talk/session/media/channel.cc:297: // setting new channel
This comment isn't accurate any more.  Perhaps something like "We can only
update the RTCP ready to send after set_transport_channel has handled channel
writability."

[guoweis_webrtc, 2015-12-17 23:43:06]

The CQ bit was checked by guoweis@webrtc.org

[guoweis_webrtc, 2015-12-17 23:43:06]

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/1532543003/#ps50001 (title:
"improve comments.")

[commit-bot: I haz the power, 2015-12-17 23:43:11]

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

[guoweis_webrtc, 2015-12-17 23:43:17]

Landing now.

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
File talk/session/media/channel.cc (right):

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
talk/session/media/channel.cc:269: // DTLS-SRTP again.
On 2015/12/17 23:38:54, pthatcher1 wrote:
> could => can
> again => when the writable_ becomes true again

Done.

https://codereview.webrtc.org/1532543003/diff/30001/talk/session/media/channe...
talk/session/media/channel.cc:297: // setting new channel
On 2015/12/17 23:38:54, pthatcher1 wrote:
> This comment isn't accurate any more.  Perhaps something like "We can only
> update the RTCP ready to send after set_transport_channel has handled channel
> writability."

Done.

[commit-bot: I haz the power, 2015-12-18 00:46:03]

Description was changed from

==========
DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint
update. What happened is that during PrAnswer, we incorrectly do not apply
bundle. However, the channel has become writable at that time. When Answer
comes, we still reset the srtp_filter but since the channel has been writable,
the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has
been writable. We'll address the issue that bundle should apply even in PrAnswer
in a different CL.

BUG=568734
==========

to

==========
DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint
update. What happened is that during PrAnswer, we incorrectly do not apply
bundle. However, the channel has become writable at that time. When Answer
comes, we still reset the srtp_filter but since the channel has been writable,
the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has
been writable. We'll address the issue that bundle should apply even in PrAnswer
in a different CL.

BUG=568734
==========

[commit-bot: I haz the power, 2015-12-18 00:46:04]

Committed patchset #4 (id:50001)

[commit-bot: I haz the power, 2015-12-18 00:46:11]

Description was changed from

==========
DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint
update. What happened is that during PrAnswer, we incorrectly do not apply
bundle. However, the channel has become writable at that time. When Answer
comes, we still reset the srtp_filter but since the channel has been writable,
the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has
been writable. We'll address the issue that bundle should apply even in PrAnswer
in a different CL.

BUG=568734
==========

to

==========
DTLS-SRTP set up is bypassed when the channel has been writable.

This regression was introduced by CL 1505573002 to support remote fingerprint
update. What happened is that during PrAnswer, we incorrectly do not apply
bundle. However, the channel has become writable at that time. When Answer
comes, we still reset the srtp_filter but since the channel has been writable,
the new SRTP context has never been applied.

We're making sure that we could always apply SRTP context even when channel has
been writable. We'll address the issue that bundle should apply even in PrAnswer
in a different CL.

BUG=568734

Committed: https://crrev.com/4638331fd8857b263bb65f12dbf5e1f7005e1a9a
Cr-Commit-Position: refs/heads/master@{#11075}
==========

[commit-bot: I haz the power, 2015-12-18 00:46:12]

Patchset 4 (id:??) landed as
https://crrev.com/4638331fd8857b263bb65f12dbf5e1f7005e1a9a
Cr-Commit-Position: refs/heads/master@{#11075}