Only try to pair protocol matching candidates for creating connections.

Only try to pair protocol matching candidates for creating connections.
If the local port and the remote candidate's protocols do not match,
do not even try to pair them.
This avoids printing out confusing logs like
"Attempt to change a remote candidate..." in p2ptransportchannel
when two remote candidates have the same port number but different
protocols.

BUG=
R=pthatcher@webrtc.org

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

[honghaiz3, 2015-12-09 18:15:19]

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

[pthatcher1, 2015-12-11 02:05:36]

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
webrtc/p2p/base/p2ptransportchannel.cc:746: Connection* connection =
port->GetConnection(remote_candidate.address());
So, if we get two TURN candidates, one for TCP and one for UDP, both with the
same IP and port, what happens?  It's the same TurnPort, right?  So wouldn't
that case fail?  Would that mean we'd be back to making the map be based on (ip,
port, protocol)?

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/port_unittest.cc
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/port_unittest...
webrtc/p2p/base/port_unittest.cc:144: virtual bool ProtocolMatch(const
std::string& protocol) const { return true; }
Shouldn't we use the enum instead of the string?

And a better name might be "SupportsProtocol"

[honghaiz3, 2015-12-11 18:36:31]

PTAL.

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
webrtc/p2p/base/p2ptransportchannel.cc:746: Connection* connection =
port->GetConnection(remote_candidate.address());
On 2015/12/11 02:05:36, pthatcher1 wrote:
> So, if we get two TURN candidates, one for TCP and one for UDP, both with the
> same IP and port, what happens?  It's the same TurnPort, right?  So wouldn't
> that case fail?  Would that mean we'd be back to making the map be based on
(ip,
> port, protocol)?  

TurnPort should only generate UDP candidate. GTurnPort may handle both UDP and
TCP connections but that is deprecated, right?

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/port_unittest.cc
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/port_unittest...
webrtc/p2p/base/port_unittest.cc:144: virtual bool ProtocolMatch(const
std::string& protocol) const { return true; }
On 2015/12/11 02:05:36, pthatcher1 wrote:
> Shouldn't we use the enum instead of the string?
> 
> And a better name might be "SupportsProtocol"
I am using string because candidate.protocol() returns string. 
This just make the comparison slightly easier. 
Changed the name.

[pthatcher1, 2015-12-12 03:05:30]

lgtm

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1516613002/diff/1/webrtc/p2p/base/p2ptransportc...
webrtc/p2p/base/p2ptransportchannel.cc:746: Connection* connection =
port->GetConnection(remote_candidate.address());
On 2015/12/11 18:36:31, honghaiz3 wrote:
> On 2015/12/11 02:05:36, pthatcher1 wrote:
> > So, if we get two TURN candidates, one for TCP and one for UDP, both with
the
> > same IP and port, what happens?  It's the same TurnPort, right?  So wouldn't
> > that case fail?  Would that mean we'd be back to making the map be based on
> (ip,
> > port, protocol)?  
> 
> TurnPort should only generate UDP candidate. GTurnPort may handle both UDP and
> TCP connections but that is deprecated, right?

Oh, duh.  Good point.

[honghaiz3, 2015-12-14 16:19:21]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-12-14 16:19:24]

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

[commit-bot: I haz the power, 2015-12-14 18:20:02]

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

[commit-bot: I haz the power, 2015-12-14 18:20:03]

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

[honghaiz3, 2015-12-14 18:46:17]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2015-12-14 18:46:17]

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/1516613002/#ps40001 (title:
"merge")

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

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

[commit-bot: I haz the power, 2015-12-14 19:32:04]

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

[commit-bot: I haz the power, 2015-12-14 19:32:04]

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

[honghaiz3, 2015-12-15 19:33:46]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-12-15 19:33:54]

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

[honghaiz3, 2015-12-15 20:20:27]

Description was changed from

==========
Only try to pair protocol matching candidates for creating connections.
If the local port and the remote candidate's protocols do not match,
do not even try to pair them.
This avoids printing out confusing logs like
"Attempt to change a remote candidate..." in p2ptransportchannel
when two remote candidates have the same port number but different
protocols.

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

to

==========
Only try to pair protocol matching candidates for creating connections.
If the local port and the remote candidate's protocols do not match,
do not even try to pair them.
This avoids printing out confusing logs like
"Attempt to change a remote candidate..." in p2ptransportchannel
when two remote candidates have the same port number but different
protocols.

BUG=
R=pthatcher@webrtc.org

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

[honghaiz3, 2015-12-15 20:20:28]

Committed patchset #3 (id:40001) manually as
f9945b2d1aa2d78b19987219ea872605167d7b5f (presubmit successful).

[commit-bot: I haz the power, 2015-12-15 20:20:32]

Description was changed from

==========
Only try to pair protocol matching candidates for creating connections.
If the local port and the remote candidate's protocols do not match,
do not even try to pair them.
This avoids printing out confusing logs like
"Attempt to change a remote candidate..." in p2ptransportchannel
when two remote candidates have the same port number but different
protocols.

BUG=
R=pthatcher@webrtc.org

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

to

==========
Only try to pair protocol matching candidates for creating connections.
If the local port and the remote candidate's protocols do not match,
do not even try to pair them.
This avoids printing out confusing logs like
"Attempt to change a remote candidate..." in p2ptransportchannel
when two remote candidates have the same port number but different
protocols.

BUG=
R=pthatcher@webrtc.org

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

[commit-bot: I haz the power, 2015-12-15 20:20:33]

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