Fix active tcp port to 9

Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and turn it on.

Existing test case should be suffice to cover this.

R=juberti@google.com
TBR=jubert@webrtc.org
BUG=webrtc:3849
Committed: https://crrev.com/310b093aa41d00be744b6f4bce77fbb657a80096
Cr-Commit-Position: refs/heads/master@{#10683}

[guoweis_webrtc, 2015-11-06 21:06:05]

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

[guoweis_webrtc, 2015-11-06 21:06:06]

PTAL.

[guoweis_webrtc, 2015-11-06 21:06:21]

Description was changed from

==========
Fix active tcp port to 9

BUG=webrtc:3849
==========

to

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout
Tested with firefox.


BUG=webrtc:3849
==========

[guoweis_webrtc, 2015-11-06 21:07:42]

Description was changed from

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout
Tested with firefox.


BUG=webrtc:3849
==========

to

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and
turn it on.


BUG=webrtc:3849
==========

[guoweis_webrtc, 2015-11-06 21:11:06]

Description was changed from

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and
turn it on.


BUG=webrtc:3849
==========

to

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and
turn it on.

Existing test case should be suffice to cover this.


BUG=webrtc:3849
==========

[juberti, 2015-11-07 07:38:22]

On 2015/11/06 21:06:06, guoweis wrote:
> PTAL.

code LG. Test?

[pthatcher1, 2015-11-11 20:06:24]

https://codereview.webrtc.org/1217463004/diff/40001/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1217463004/diff/40001/webrtc/p2p/base/tcpport.c...
webrtc/p2p/base/tcpport.cc:188: AddAddress(rtc::SocketAddress(ip(),
DISCARD_PORT),
Can you please leave a comment explaining why DISCARD_PORT is used here?  It's
not very obvious for just looking at the code.

[guoweis_webrtc, 2015-11-13 19:55:04]

P2PTransportChannelTest.TestTcpConnectionsFromActiveToPassive exercises the code
path.

[juberti, 2015-11-14 01:09:28]

juberti@google.com changed reviewers:
+ juberti@google.com

[juberti, 2015-11-14 01:09:28]

Suggest adding the code to that specific test instead, to keep the main test
body simple.

https://codereview.webrtc.org/1217463004/diff/80001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1217463004/diff/80001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:642: if (c.protocol() ==
cricket::TCP_PROTOCOL_NAME) {
I think this would be best in the TestTcp* tests below.

[guoweis_webrtc, 2015-11-16 23:33:17]

Patchset #6 (id:100001) has been deleted

[guoweis_webrtc, 2015-11-16 23:43:21]

On 2015/11/14 01:09:28, juberti wrote:
> Suggest adding the code to that specific test instead, to keep the main test
> body simple.
> 
>
https://codereview.webrtc.org/1217463004/diff/80001/webrtc/p2p/base/p2ptransp...
> File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):
> 
>
https://codereview.webrtc.org/1217463004/diff/80001/webrtc/p2p/base/p2ptransp...
> webrtc/p2p/base/p2ptransportchannel_unittest.cc:642: if (c.protocol() ==
> cricket::TCP_PROTOCOL_NAME) {
> I think this would be best in the TestTcp* tests below.

PTAL. Have to verify them when they are generated (on signaling path). On the
media side, the active one will become prflx and will have a random port.

[juberti, 2015-11-17 02:01:05]

https://codereview.webrtc.org/1217463004/diff/160001/webrtc/p2p/base/p2ptrans...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1217463004/diff/160001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:656:
std::vector<CandidateData*>::iterator it = ed->saved_candidates_.begin();
can this be done as a c++ foreach? (e.g. for (candidate : ed->saved_candidates)?

https://codereview.webrtc.org/1217463004/diff/160001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:658:
EXPECT_EQ((*it)->candidate.protocol(),cricket::TCP_PROTOCOL_NAME);
space after comma

https://codereview.webrtc.org/1217463004/diff/160001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:662: continue;
this is kind of weird structure; I think 
if (a) {
} else if (b) {
} else {
  FAIL << "unknown tcptype";
}

would be better

[guoweis_webrtc, 2015-11-17 18:48:33]

Addressed these. PTAL.

[juberti, 2015-11-17 21:48:03]

https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:655: for (auto& it :
GetEndpoint(endpoint)->saved_candidates_) {
|it| is probably a bad name here, since it's not an iterator, but a
CandidateData object. Suggest calling it |data|

https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:663: RTC_DCHECK(false) <<
"Unknown tcptype: " << it->candidate.tcptype();
Use the gtest FAIL() macro

[guoweis_webrtc, 2015-11-17 21:59:38]

On 2015/11/17 21:48:03, juberti wrote:
>
https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
> File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):
> 
>
https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
> webrtc/p2p/base/p2ptransportchannel_unittest.cc:655: for (auto& it :
> GetEndpoint(endpoint)->saved_candidates_) {
> |it| is probably a bad name here, since it's not an iterator, but a
> CandidateData object. Suggest calling it |data|
> 
>
https://codereview.webrtc.org/1217463004/diff/180001/webrtc/p2p/base/p2ptrans...
> webrtc/p2p/base/p2ptransportchannel_unittest.cc:663: RTC_DCHECK(false) <<
> "Unknown tcptype: " << it->candidate.tcptype();
> Use the gtest FAIL() macro

Done.

[juberti, 2015-11-18 01:30:43]

lgtm

[guoweis_webrtc, 2015-11-18 01:31:34]

The CQ bit was checked by guoweis@webrtc.org

[commit-bot: I haz the power, 2015-11-18 01:31:44]

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

[commit-bot: I haz the power, 2015-11-18 01:35:37]

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

[commit-bot: I haz the power, 2015-11-18 01:35:38]

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

[guoweis_webrtc, 2015-11-18 03:15:45]

Description was changed from

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and
turn it on.

Existing test case should be suffice to cover this.


BUG=webrtc:3849
==========

to

==========
Fix active tcp port to 9

In tcp only call:
Tested with hangout.
Tested with firefox.

To test firefox, goto about:config, search for media.peerconnection.ice.tcp and
turn it on.

Existing test case should be suffice to cover this.

TBR=jubert@webrtc.org
BUG=webrtc:3849
==========

[guoweis_webrtc, 2015-11-18 03:16:02]

Committed patchset #10 (id:200001) manually as
310b093aa41d00be744b6f4bce77fbb657a80096 (presubmit successful).

[commit-bot: I haz the power, 2015-11-18 03:16:04]

Patchset 10 (id:??) landed as
https://crrev.com/310b093aa41d00be744b6f4bce77fbb657a80096
Cr-Commit-Position: refs/heads/master@{#10683}