Fix AllocationSequence to handle the case when TurnPort stops using shared socket.

Fix AllocationSequence to handle the case when TurnPort stops using shared socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
R=honghaiz@webrtc.org, jiayl@chromium.org, pthatcher@webrtc.org

Committed: https://crrev.com/17fa67214caf066887bf429cf95ecaf895b0688c
Cr-Commit-Position: refs/heads/master@{#12675}

[Sergey Ulanov, 2016-04-09 00:15:03]

sergeyu@chromium.org changed reviewers:
+ jiayl@webrtc.org

[Sergey Ulanov, 2016-04-12 22:58:55]

ping

[jiayl, 2016-04-18 18:04:52]

Description was changed from

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=5757
==========

to

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=5757
==========

[jiayl, 2016-04-18 18:04:52]

jiayl@chromium.org changed reviewers:
+ honghaiz@chromium.org, pthatcher@webrtc.org
- jiayl@webrtc.org

[Sergey Ulanov, 2016-04-18 18:06:38]

Honghai, Peter,
Can you please take a look?

[Irfan, 2016-04-18 21:53:12]

isheriff@chromium.org changed reviewers:
+ isheriff@chromium.org

[Irfan, 2016-04-18 21:53:12]

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
webrtc/p2p/client/basicportallocator.cc:1100:
RTP_DCHECK(udp_port_->SharedSocket());
RTC_DCHECK ?

[Sergey Ulanov, 2016-04-18 22:35:14]

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
webrtc/p2p/client/basicportallocator.cc:1100:
RTP_DCHECK(udp_port_->SharedSocket());
On 2016/04/18 21:53:12, Irfan wrote:
> RTC_DCHECK ?

It's webrtc's version of DCHECK(). See webrtc/base/checks.h

[Irfan, 2016-04-18 22:42:44]

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
webrtc/p2p/client/basicportallocator.cc:1100:
RTP_DCHECK(udp_port_->SharedSocket());
On 2016/04/18 22:35:14, Sergey Ulanov wrote:
> On 2016/04/18 21:53:12, Irfan wrote:
> > RTC_DCHECK ?
> 
> It's webrtc's version of DCHECK(). See webrtc/base/checks.h

yes, I meant s/RTP_DCHECK/RTC_DCHECK

[Sergey Ulanov, 2016-04-18 23:02:41]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[Sergey Ulanov, 2016-04-18 23:02:44]

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.chromium.org/1871693004/diff/1/webrtc/p2p/client/basicport...
webrtc/p2p/client/basicportallocator.cc:1100:
RTP_DCHECK(udp_port_->SharedSocket());
On 2016/04/18 22:42:43, Irfan wrote:
> On 2016/04/18 22:35:14, Sergey Ulanov wrote:
> > On 2016/04/18 21:53:12, Irfan wrote:
> > > RTC_DCHECK ?
> > 
> > It's webrtc's version of DCHECK(). See webrtc/base/checks.h
> 
> yes, I meant s/RTP_DCHECK/RTC_DCHECK

Done.

[commit-bot: I haz the power, 2016-04-18 23:02:51]

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

[honghaiz3, 2016-04-19 00:43:22]

On 2016/04/18 23:02:51, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1871693004/20001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1871693004/20001

The change looks good. 
Can you add a test?

In the CL description, change the bug line to "BUG=webrtc:5757".

[honghaiz3, 2016-04-19 00:44:39]

On 2016/04/19 00:43:22, honghaiz3 wrote:
> On 2016/04/18 23:02:51, commit-bot: I haz the power wrote:
> > Dry run: CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patch-status/1871693004/20001
> > View timeline at
> >  https://chromium-cq-status.appspot.com/patch-timeline/1871693004/20001
> 
> The change looks good. 
> Can you add a test?
> 
> In the CL description, change the bug line to "BUG=webrtc:5757".
Sorry I was on vacation (and I think Peter is too).

[commit-bot: I haz the power, 2016-04-19 01:03:22]

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

[commit-bot: I haz the power, 2016-04-19 01:03:23]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz, 2016-04-19 05:32:20]

https://codereview.webrtc.org/1871693004/diff/20001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1086: if (port->SharedSocket() &&
port->server_address().address == remote_addr) {
I think it is better to still let the TurnPort eat the packet if the server
address matches and the port is not using Shared socket. With the current
change, if the server address matches and the port is not using shared socket,
the packet will be passed to the stun port, which is unexpected. 

So I suggest instead of changing here, you can change the
TurnPort::HandleIncomingPacket method, 
in which if the port is using shared socket, pass it to OnReadPacket, otherwise,
drop it.

[Sergey Ulanov, 2016-04-19 19:09:02]

Description was changed from

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=5757
==========

to

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
==========

[Sergey Ulanov, 2016-04-20 20:50:56]

Added a test now. PTAL

https://codereview.chromium.org/1871693004/diff/20001/webrtc/p2p/client/basic...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.chromium.org/1871693004/diff/20001/webrtc/p2p/client/basic...
webrtc/p2p/client/basicportallocator.cc:1086: if (port->SharedSocket() &&
port->server_address().address == remote_addr) {
On 2016/04/19 05:32:20, honghaiz wrote:
> I think it is better to still let the TurnPort eat the packet if the server
> address matches and the port is not using Shared socket. With the current
> change, if the server address matches and the port is not using shared socket,
> the packet will be passed to the stun port, which is unexpected.

The packet will be passed to a stun port only when the remote address is in the
list of stun servers. Also, according the comment above there is a case when
STUN and TURN servers share the same address. In that case I think we actually
want the packet to be passed to the stun port.

BTW the comment doesn't make it clear what's the right behavior when we receive
a STUN binding response here. If I understand correctly we want that packet be
passed to StunPort, but currently this code gives it to the TurnPort and assumes
that it was handled there.

> 
> So I suggest instead of changing here, you can change the
> TurnPort::HandleIncomingPacket method, 
> in which if the port is using shared socket, pass it to OnReadPacket,
otherwise,
> drop it. 

Move all code from OnReadPacket() to HandleIncomingPacket(). This allows to
return correct result from HandleIncomingPacket() and so STUN binding responses
are now passed to STUN port as they should.

[Sergey Ulanov, 2016-04-21 23:12:20]

honghaiz: ping

[honghaiz3, 2016-04-22 01:49:02]

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

[honghaiz3, 2016-04-22 01:49:03]

https://codereview.webrtc.org/1871693004/diff/20001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1086: if (port->SharedSocket() &&
port->server_address().address == remote_addr) {
On 2016/04/20 20:50:56, Sergey Ulanov wrote:
> The packet will be passed to a stun port only when the remote address is in
the
> list of stun servers. Also, according the comment above there is a case when
> STUN and TURN servers share the same address. In that case I think we actually
> want the packet to be passed to the stun port.
It will be passed to the stun port because the condition was 
if (not found in the turn server list OR it is found in the stun server list).  
> 
> BTW the comment doesn't make it clear what's the right behavior when we
receive
> a STUN binding response here. If I understand correctly we want that packet be
> passed to StunPort, but currently this code gives it to the TurnPort and
assumes
> that it was handled there.
Yes, we want that packet to be passed to StunPort. 
I think the existing code is doing that because of the OR condition. 

> 
> > 

> Move all code from OnReadPacket() to HandleIncomingPacket(). This allows to
> return correct result from HandleIncomingPacket() and so STUN binding
responses
> are now passed to STUN port as they should.
>

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1096: if (stun_servers.find(remote_addr)
!= stun_servers.end()) {
I think this is incorrect. 
In the current code, a packet coming from the other peer directly will be routed
through the UDP port (because of the condition (!turn_port_found). 
With the new change, that packet will be dropped.

[honghaiz3, 2016-04-22 03:52:34]

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1096: if (stun_servers.find(remote_addr)
!= stun_servers.end()) {
On 2016/04/22 01:49:03, honghaiz3 wrote:
> I think this is incorrect. 
> In the current code, a packet coming from the other peer directly will be
routed
> through the UDP port (because of the condition (!turn_port_found). 
> With the new change, that packet will be dropped.
One possible approach is to use the existing logic; if the source addr is not
found in turn servers or it is found in stun servers, pass it to the udp port.
But then every turn channel packet is sent to udp port (and will be dropped
there). 
A better approach is, if the source address is not found in turn servers or (it
is found there but not consumed and it is also found in stun servers), pass it
to the udp port.

[Sergey Ulanov, 2016-04-22 18:05:52]

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1096: if (stun_servers.find(remote_addr)
!= stun_servers.end()) {
On 2016/04/22 03:52:33, honghaiz3 wrote:
> On 2016/04/22 01:49:03, honghaiz3 wrote:
> > I think this is incorrect. 
> > In the current code, a packet coming from the other peer directly will be
> routed
> > through the UDP port (because of the condition (!turn_port_found). 
> > With the new change, that packet will be dropped.
> One possible approach is to use the existing logic; if the source addr is not
> found in turn servers or it is found in stun servers, pass it to the udp port.
> But then every turn channel packet is sent to udp port (and will be dropped
> there). 
> A better approach is, if the source address is not found in turn servers or
(it
> is found there but not consumed and it is also found in stun servers), pass it
> to the udp port. 
> 

Implemented this suggestion in Patch Set 4.
I'm not sure it it's actually necessary to check if the address in the list of
STUN servers. I think the logic could be simplified:
 1. Find a turn port with the given remote_addr and try passing the new packet
there.
 2. If the packet is not handled by any of the turn ports then pass it to the
udp_port_.

This would simplify the code and I don't think it would make any difference
otherwise. UDPPort::OnReadPacket() should be able to handle the packet
appropriately. I've implemented this approach in Patch Set 5. WDYT?

[honghaiz3, 2016-04-22 19:17:28]

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:1096: if (stun_servers.find(remote_addr)
!= stun_servers.end()) {
On 2016/04/22 18:05:52, Sergey Ulanov wrote:
> On 2016/04/22 03:52:33, honghaiz3 wrote:
> > On 2016/04/22 01:49:03, honghaiz3 wrote:
> > > I think this is incorrect. 
> > > In the current code, a packet coming from the other peer directly will be
> > routed
> > > through the UDP port (because of the condition (!turn_port_found). 
> > > With the new change, that packet will be dropped.
> > One possible approach is to use the existing logic; if the source addr is
not
> > found in turn servers or it is found in stun servers, pass it to the udp
port.
> > But then every turn channel packet is sent to udp port (and will be dropped
> > there). 
> > A better approach is, if the source address is not found in turn servers or
> (it
> > is found there but not consumed and it is also found in stun servers), pass
it
> > to the udp port. 
> > 
> 
> Implemented this suggestion in Patch Set 4.
> I'm not sure it it's actually necessary to check if the address in the list of
> STUN servers. I think the logic could be simplified:
>  1. Find a turn port with the given remote_addr and try passing the new packet
> there.
>  2. If the packet is not handled by any of the turn ports then pass it to the
> udp_port_.
> 
> This would simplify the code and I don't think it would make any difference
> otherwise. UDPPort::OnReadPacket() should be able to handle the packet
> appropriately. I've implemented this approach in Patch Set 5. WDYT?
I think patch set 4 is slightly safer. 
Although patch set 5 looks nicer, it leaves a potential hole: 
For those packets whose source address is found in the turn port addresses but
not consumed by turn port, they will always be passed to the stun port. It has
no problem if it is also found in the stun_server addresses (just like patch set
4). But if it is not found in the stun server addresses, the packets will be
treated as app data in the stun port, which may cause problems later (although
in our implementation, they will be dropped).

[Sergey Ulanov, 2016-04-22 21:23:50]

On 2016/04/22 19:17:28, honghaiz3 wrote:
>
https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
> File webrtc/p2p/client/basicportallocator.cc (right):
> 
>
https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
> webrtc/p2p/client/basicportallocator.cc:1096: if
(stun_servers.find(remote_addr)
> != stun_servers.end()) {
> On 2016/04/22 18:05:52, Sergey Ulanov wrote:
> > On 2016/04/22 03:52:33, honghaiz3 wrote:
> > > On 2016/04/22 01:49:03, honghaiz3 wrote:
> > > > I think this is incorrect. 
> > > > In the current code, a packet coming from the other peer directly will
be
> > > routed
> > > > through the UDP port (because of the condition (!turn_port_found). 
> > > > With the new change, that packet will be dropped.
> > > One possible approach is to use the existing logic; if the source addr is
> not
> > > found in turn servers or it is found in stun servers, pass it to the udp
> port.
> > > But then every turn channel packet is sent to udp port (and will be
dropped
> > > there). 
> > > A better approach is, if the source address is not found in turn servers
or
> > (it
> > > is found there but not consumed and it is also found in stun servers),
pass
> it
> > > to the udp port. 
> > > 
> > 
> > Implemented this suggestion in Patch Set 4.
> > I'm not sure it it's actually necessary to check if the address in the list
of
> > STUN servers. I think the logic could be simplified:
> >  1. Find a turn port with the given remote_addr and try passing the new
packet
> > there.
> >  2. If the packet is not handled by any of the turn ports then pass it to
the
> > udp_port_.
> > 
> > This would simplify the code and I don't think it would make any difference
> > otherwise. UDPPort::OnReadPacket() should be able to handle the packet
> > appropriately. I've implemented this approach in Patch Set 5. WDYT?
> I think patch set 4 is slightly safer. 
> Although patch set 5 looks nicer, it leaves a potential hole: 
> For those packets whose source address is found in the turn port addresses but
> not consumed by turn port, they will always be passed to the stun port. It has
> no problem if it is also found in the stun_server addresses (just like patch
set
> 4). But if it is not found in the stun server addresses, the packets will be
> treated as app data in the stun port, which may cause problems later (although
> in our implementation, they will be dropped).


Ok, reverted to patchset 4, but I still think that check is not necessary.
StunPort must be able to handle cases like this where it receives packet from an
address it doesn't recognize and so it will only accept data packets after
complete binding request/response handshake.

[honghaiz3, 2016-04-22 22:05:42]

On 2016/04/22 21:23:50, Sergey Ulanov wrote:
> On 2016/04/22 19:17:28, honghaiz3 wrote:
> >
>
https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
> > File webrtc/p2p/client/basicportallocator.cc (right):
> > 
> >
>
https://codereview.webrtc.org/1871693004/diff/40001/webrtc/p2p/client/basicpo...
> > webrtc/p2p/client/basicportallocator.cc:1096: if
> (stun_servers.find(remote_addr)
> > != stun_servers.end()) {
> > On 2016/04/22 18:05:52, Sergey Ulanov wrote:
> > > On 2016/04/22 03:52:33, honghaiz3 wrote:
> > > > On 2016/04/22 01:49:03, honghaiz3 wrote:
> > > > > I think this is incorrect. 
> > > > > In the current code, a packet coming from the other peer directly will
> be
> > > > routed
> > > > > through the UDP port (because of the condition (!turn_port_found). 
> > > > > With the new change, that packet will be dropped.
> > > > One possible approach is to use the existing logic; if the source addr
is
> > not
> > > > found in turn servers or it is found in stun servers, pass it to the udp
> > port.
> > > > But then every turn channel packet is sent to udp port (and will be
> dropped
> > > > there). 
> > > > A better approach is, if the source address is not found in turn servers
> or
> > > (it
> > > > is found there but not consumed and it is also found in stun servers),
> pass
> > it
> > > > to the udp port. 
> > > > 
> > > 
> > > Implemented this suggestion in Patch Set 4.
> > > I'm not sure it it's actually necessary to check if the address in the
list
> of
> > > STUN servers. I think the logic could be simplified:
> > >  1. Find a turn port with the given remote_addr and try passing the new
> packet
> > > there.
> > >  2. If the packet is not handled by any of the turn ports then pass it to
> the
> > > udp_port_.
> > > 
> > > This would simplify the code and I don't think it would make any
difference
> > > otherwise. UDPPort::OnReadPacket() should be able to handle the packet
> > > appropriately. I've implemented this approach in Patch Set 5. WDYT?
> > I think patch set 4 is slightly safer. 
> > Although patch set 5 looks nicer, it leaves a potential hole: 
> > For those packets whose source address is found in the turn port addresses
but
> > not consumed by turn port, they will always be passed to the stun port. It
has
> > no problem if it is also found in the stun_server addresses (just like patch
> set
> > 4). But if it is not found in the stun server addresses, the packets will be
> > treated as app data in the stun port, which may cause problems later
(although
> > in our implementation, they will be dropped).
> 
> 
> Ok, reverted to patchset 4, but I still think that check is not necessary.
> StunPort must be able to handle cases like this where it receives packet from
an
> address it doesn't recognize and so it will only accept data packets after
> complete binding request/response handshake.
Thanks for addressing the comments.

[honghaiz3, 2016-04-22 22:05:57]

lgtm

[jiayl, 2016-04-22 22:19:14]

jiayl@chromium.org changed reviewers:
+ jiayl@chromium.org

[jiayl, 2016-04-22 22:19:15]

lgtm

[Sergey Ulanov, 2016-04-22 22:48:11]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-04-22 22:48:16]

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

[commit-bot: I haz the power, 2016-04-23 00:48:47]

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

[commit-bot: I haz the power, 2016-04-23 00:48:49]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[pthatcher1, 2016-05-05 23:11:00]

lgtm

[Sergey Ulanov, 2016-05-10 00:13:59]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-05-10 00:14:04]

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

[commit-bot: I haz the power, 2016-05-10 00:26:34]

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

[commit-bot: I haz the power, 2016-05-10 00:26:35]

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

[commit-bot: I haz the power, 2016-05-10 17:20:59]

Description was changed from

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
==========

to

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
R=honghaiz@webrtc.org, jiayl@chromium.org, pthatcher@webrtc.org

Committed: https://crrev.com/17fa67214caf066887bf429cf95ecaf895b0688c
Cr-Commit-Position: refs/heads/master@{#12675}
==========

[Sergey Ulanov, 2016-05-10 17:21:00]

Description was changed from

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
R=honghaiz@webrtc.org, jiayl@chromium.org, pthatcher@webrtc.org

Committed: https://crrev.com/17fa67214caf066887bf429cf95ecaf895b0688c
Cr-Commit-Position: refs/heads/master@{#12675}
==========

to

==========
Fix AllocationSequence to handle the case when TurnPort stops using shared
socket.

AllocationSequence is responsible for receiving incoming packets on
a shared UDP socket and passing them to the Port objects. TurnPort
may stop sharing UDP socket in which case it allocates a new socket.
AllocationSequence::OnReadPacket() wasn't handling that case properly
which was causing an assert in TurnPort::OnReadPacket().

BUG=webrtc:5757
R=honghaiz@webrtc.org, jiayl@chromium.org, pthatcher@webrtc.org

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

[commit-bot: I haz the power, 2016-05-10 17:21:00]

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

[Sergey Ulanov, 2016-05-10 17:21:01]

Committed patchset #6 (id:100001) manually as
17fa67214caf066887bf429cf95ecaf895b0688c (presubmit successful).