Add the last_sent_packet_id to the candidate pair change signal

Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not set.

Also moved the tests for candidate pair changes to TestSelectConnectionBeforeNomination.

BUG=
R=deadbeef@webrtc.org, pthatcher@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/52dce73fac798e6b5b6b65366882806a6c710301

[honghaiz3, 2016-03-30 19:08:59]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2016-03-30 19:14:36]

Patchset #1 (id:20001) has been deleted

[honghaiz3, 2016-03-30 19:15:59]

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

[honghaiz3, 2016-03-30 19:16:00]

PTAL.

[honghaiz3, 2016-03-30 19:18:18]

Description was changed from

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.

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

to

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination

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

[honghaiz3, 2016-03-30 19:26:47]

Description was changed from

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination

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

to

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not
set.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination.

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

[pthatcher1, 2016-03-30 19:34:43]

I'd prefer Optional<uint16_t>, but it's not a blocker for the CL.

https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute.h
File webrtc/base/networkroute.h (right):

https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute....
webrtc/base/networkroute.h:24: int last_sent_packet_id;  // Last packet id sent
on the PREVIOUS route.
Can we use an Optional<uint16_t>?

[pthatcher1, 2016-03-30 19:34:54]

On 2016/03/30 19:34:43, pthatcher1 wrote:
> I'd prefer Optional<uint16_t>, but it's not a blocker for the CL.
> 
> https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute.h
> File webrtc/base/networkroute.h (right):
> 
>
https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute....
> webrtc/base/networkroute.h:24: int last_sent_packet_id;  // Last packet id
sent
> on the PREVIOUS route.
> Can we use an Optional<uint16_t>?

lgtm

[honghaiz3, 2016-03-30 19:48:23]

https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute.h
File webrtc/base/networkroute.h (right):

https://codereview.webrtc.org/1842093002/diff/40001/webrtc/base/networkroute....
webrtc/base/networkroute.h:24: int last_sent_packet_id;  // Last packet id sent
on the PREVIOUS route.
On 2016/03/30 19:34:43, pthatcher1 wrote:
> Can we use an Optional<uint16_t>?

Since packet_id in the PacketOption has already used -1 (32bit) to indicate the
unset value and this value is coped from there, it is better for us to use the
same convention. Otherwise, we will have to convert that passed in 32 bits to 16
bit optional and perhaps need to check whether the passed in value is unset or
not.

[honghaiz3, 2016-03-30 20:25:48]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2016-03-30 20:25:49]

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/1842093002/#ps60001 (title:
"Merge with head")

[commit-bot: I haz the power, 2016-03-30 20:25:55]

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

[Taylor Brandstetter, 2016-03-30 21:17:14]

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.h (right):

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:199: }
Why is this necessary? Can't SendPacket be used by the unit tests?

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (left):

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:1650: EXPECT_EQ(2,
num_selected_candidate_pair_changes());
Why is this being removed?

[honghaiz3, 2016-03-30 21:25:22]

The CQ bit was unchecked by honghaiz@webrtc.org

[honghaiz3, 2016-03-30 21:48:09]

PTAL. Thanks!

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.h (right):

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:199: }
On 2016/03/30 21:17:14, Taylor Brandstetter wrote:
> Why is this necessary? Can't SendPacket be used by the unit tests?

Good point. I removed this and revised the tests with SendPacket.

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (left):

https://codereview.webrtc.org/1842093002/diff/60001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:1650: EXPECT_EQ(2,
num_selected_candidate_pair_changes());
On 2016/03/30 21:17:14, Taylor Brandstetter wrote:
> Why is this being removed?

I moved the tests  for selected candidate pair to
TestSelectConnectionBeforeNomination.

[Taylor Brandstetter, 2016-03-31 17:20:02]

lgtm

[honghaiz3, 2016-03-31 17:45:11]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2016-03-31 17:45:11]

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/1842093002/#ps80001 (title:
"Removed the set_last_sent_packet_id method")

[commit-bot: I haz the power, 2016-03-31 17:45:18]

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

[honghaiz3, 2016-03-31 19:05:16]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2016-03-31 19:05:17]

The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org,
pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1842093002/#ps100001 (title:
"Merge with head")

[commit-bot: I haz the power, 2016-03-31 19:05:25]

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

[commit-bot: I haz the power, 2016-03-31 19:37:44]

Description was changed from

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not
set.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination.

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

to

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not
set.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination.

BUG=
R=deadbeef@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/52dce73fac798e6b5b6b65366882806a6c710301
Cr-Commit-Position: refs/heads/master@{#12184}
==========

[honghaiz3, 2016-03-31 19:37:44]

Description was changed from

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not
set.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination.

BUG=
R=deadbeef@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/52dce73fac798e6b5b6b65366882806a6c710301
Cr-Commit-Position: refs/heads/master@{#12184}
==========

to

==========
Add the last_sent_packet_id to the candidate pair change signal
so that the call knows which packet ids were sent on the previous candidate
pair.
Note that packet_id is actually 16bits, so we can use -1 for values that are not
set.

Also moved the tests for candidate pair changes to
TestSelectConnectionBeforeNomination.

BUG=
R=deadbeef@webrtc.org, pthatcher@webrtc.org

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

[commit-bot: I haz the power, 2016-03-31 19:37:45]

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

[honghaiz3, 2016-03-31 19:37:46]

Committed patchset #4 (id:100001) manually as
52dce73fac798e6b5b6b65366882806a6c710301 (presubmit successful).