RTCIceCandidatePairStats.nominated collected.

webrtc/p2p/base/port.h

Index: webrtc/p2p/base/port.h
diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index f6900210f4417e35da6384eaeba7b54433d43b46..edfb5c934759e017a4b82b82b9df6aa22f08eee5 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -531,7 +531,7 @@ class Connection : public CandidatePairInterface,
   void set_nomination(uint32_t value) { nomination_ = value; }
 
   uint32_t remote_nomination() const { return remote_nomination_; }
-  bool nominated() const { return remote_nomination_ > 0; }
+  bool nominated() const { return acked_nomination_ || remote_nomination_; }

[hbos, 2017/02/23 13:38:56]

According to https://tools.ietf.org/html/rfc5245#section-8 one or several pairs
may me nominated depending on if Regular or Aggressive Nomination is used.
Lacking background I'm having trouble understanding the RFC, or if we want a
controlling agent to be "nominated" if |acked_nomination_ > 0| or if
|nomination_ > 0| (before acked).

In my discussion with deadbeef we concluded to use |acked_nomination_|. I think
it makes sense to unify the stats' idea of "nominated" with
Connection::nominated(), which is currently only used for controlled agents, so
changing its meaning for controlling agents should be safe.

[Taylor Brandstetter, 2017/02/24 05:38:31]

webrtc-stats refers to the "nominated" flag from ICE, and ICE definitely says
the nominated flag is only updated on a binding response. So this seems correct.

Could you add a comment above "nominated()" in the code, summarizing this?

[hbos, 2017/02/24 11:09:13]

Done.

   // Public for unit tests.
   void set_remote_nomination(uint32_t remote_nomination) {
     remote_nomination_ = remote_nomination;