RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated.

RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated.

State and priority added to ConnectionInfo. The Connection::State enum
is replaced by IceCandidatePairState enum class.

At P2PTransportChannel::GetStats, Connection::stats is called, producing
ConnectionInfo for the connection that is then filled in with additional
values from the Connection. This is refactored so that all values are
set by Connection::stats.

RTCStatsCollector is updated to surface the ConnectionInfo stats.

BUG=webrtc:6755, chromium:633550, chromium:627816
Review-Url: https://codereview.webrtc.org/2597423003
Cr-Commit-Position: refs/heads/master@{#15870}
Committed: https://chromium.googlesource.com/external/webrtc/+/06495bcbb7477b8df1e9f2e34eae0efd11e0b9fe

[hbos, 2016-12-23 13:36:28]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-23 13:36:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[hbos, 2016-12-23 13:38:28]

Patchset #1 (id:1) has been deleted

[hbos, 2016-12-23 13:45:33]

hbos@webrtc.org changed reviewers:
+ deadbeef@webrtc.org, hta@webrtc.org

[hbos, 2016-12-23 13:45:34]

Please take a look, deadbeef & hta.

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector_unittest.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:1109: expected_pair.state =
"in-progress";
nit: RTCStatsIceCandidatePairState::kInProgress instead of cstr

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/stats/rtcstats...
File webrtc/api/stats/rtcstats_objects.h (left):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/stats/rtcstats...
webrtc/api/stats/rtcstats_objects.h:35: static const char* kCancelled;
Our implementation does not support this value and it will be removed from the
spec anyway https://github.com/w3c/webrtc-stats/issues/66

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
File webrtc/p2p/base/jseptransport.h (left):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
webrtc/p2p/base/jseptransport.h:104: key(NULL) {}
Moved to .cc because constructor needs to know the definition of
IceCandidatePairState which is only forward declared because of circular
inclusion. (The alternative would be to move IceCandidatePairState to here or
another header.)

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.cc
File webrtc/p2p/base/port.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1484: stats_.priority = priority();
state + priority are the new ones, the rest was moved from
P2PTransportChannel::GetStats

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/stats/rtcstats_obj...
File webrtc/stats/rtcstats_objects.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/stats/rtcstats_obj...
webrtc/stats/rtcstats_objects.cc:22: const char*
RTCStatsIceCandidatePairState::kInProgress = "in-progress";
"inprogress" vs "in-progress" being discussed here:
https://github.com/w3c/webrtc-stats/issues/66#issuecomment-268825747

[commit-bot: I haz the power, 2016-12-23 14:16:46]

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

[commit-bot: I haz the power, 2016-12-23 14:16:47]

Dry run: This issue passed the CQ dry run.

[Taylor Brandstetter, 2016-12-23 18:59:45]

lgtm, though I'd prefer if my nit regarding "best_connection" was addressed

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
File webrtc/p2p/base/jseptransport.h (left):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
webrtc/p2p/base/jseptransport.h:104: key(NULL) {}
On 2016/12/23 13:45:34, hbos wrote:
> Moved to .cc because constructor needs to know the definition of
> IceCandidatePairState which is only forward declared because of circular
> inclusion. (The alternative would be to move IceCandidatePairState to here or
> another header.)

Actually, I think IceCandidatePairState is in a sensible place. It's really
ConnectionInfo that doesn't belong here (but we'll fix that eventually, you
don't have to worry about it now).

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:1013:
infos->push_back(connection->stats(selected_connection_ == connection));
I'd prefer if "best_connection" was set here, rather than in Connection::stats.
Can leave a comment above the Connection::stats declaration saying "doesn't
populate the best_connection" field.

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:97: // TODO(hbos): Add and support the |FROZEN| state.
For now, nothing is ever frozen, FYI. The Earth may freeze over before we end up
implementing ICE freezing logic. :)

[hbos, 2016-12-27 10:24:34]

Please take a look, hta

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector_unittest.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector_unittest.cc:1109: expected_pair.state =
"in-progress";
On 2016/12/23 13:45:34, hbos wrote:
> nit: RTCStatsIceCandidatePairState::kInProgress instead of cstr

Done.

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
File webrtc/p2p/base/jseptransport.h (left):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/jseptrans...
webrtc/p2p/base/jseptransport.h:104: key(NULL) {}
On 2016/12/23 18:59:44, Taylor Brandstetter (OOO) wrote:
> On 2016/12/23 13:45:34, hbos wrote:
> > Moved to .cc because constructor needs to know the definition of
> > IceCandidatePairState which is only forward declared because of circular
> > inclusion. (The alternative would be to move IceCandidatePairState to here
or
> > another header.)
> 
> Actually, I think IceCandidatePairState is in a sensible place. It's really
> ConnectionInfo that doesn't belong here (but we'll fix that eventually, you
> don't have to worry about it now).

Acknowledged.

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:1013:
infos->push_back(connection->stats(selected_connection_ == connection));
On 2016/12/23 18:59:45, Taylor Brandstetter (OOO) wrote:
> I'd prefer if "best_connection" was set here, rather than in
Connection::stats.
> Can leave a comment above the Connection::stats declaration saying "doesn't
> populate the best_connection" field.

Done.

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.h
File webrtc/p2p/base/port.h (right):

https://codereview.webrtc.org/2597423003/diff/20001/webrtc/p2p/base/port.h#ne...
webrtc/p2p/base/port.h:97: // TODO(hbos): Add and support the |FROZEN| state.
On 2016/12/23 18:59:45, Taylor Brandstetter (OOO) wrote:
> For now, nothing is ever frozen, FYI. The Earth may freeze over before we end
up
> implementing ICE freezing logic. :)

Ack. Updated the comment and removed the "TODO" bit.

[hbos, 2016-12-27 10:24:45]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 10:24:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-12-27 11:16:42]

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

[commit-bot: I haz the power, 2016-12-27 11:16:43]

Dry run: This issue passed the CQ dry run.

[hta-webrtc, 2017-01-02 14:20:17]

lgtm

[hbos, 2017-01-02 15:14:10]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2017-01-02 15:14:11]

The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org,
hta@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2597423003/#ps60001 (title:
"Rebase with master")

[commit-bot: I haz the power, 2017-01-02 15:14:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-02 16:08:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483370050951180,
"parent_rev": "7eb0e23bcf675635ef339a519a10563ebc9d93dc", "commit_rev":
"06495bcbb7477b8df1e9f2e34eae0efd11e0b9fe"}

[commit-bot: I haz the power, 2017-01-02 16:08:21]

Description was changed from

==========
RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated.

State and priority added to ConnectionInfo. The Connection::State enum
is replaced by IceCandidatePairState enum class.

At P2PTransportChannel::GetStats, Connection::stats is called, producing
ConnectionInfo for the connection that is then filled in with additional
values from the Connection. This is refactored so that all values are
set by Connection::stats.

RTCStatsCollector is updated to surface the ConnectionInfo stats.

BUG=webrtc:6755, chromium:633550, chromium:627816
==========

to

==========
RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated.

State and priority added to ConnectionInfo. The Connection::State enum
is replaced by IceCandidatePairState enum class.

At P2PTransportChannel::GetStats, Connection::stats is called, producing
ConnectionInfo for the connection that is then filled in with additional
values from the Connection. This is refactored so that all values are
set by Connection::stats.

RTCStatsCollector is updated to surface the ConnectionInfo stats.

BUG=webrtc:6755, chromium:633550, chromium:627816

Review-Url: https://codereview.webrtc.org/2597423003
Cr-Commit-Position: refs/heads/master@{#15870}
Committed:
https://chromium.googlesource.com/external/webrtc/+/06495bcbb7477b8df1e9f2e34...
==========

[commit-bot: I haz the power, 2017-01-02 16:08:23]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/06495bcbb7477b8df1e9f2e34...