Add UMA metrics for ICE regathering reasons.

webrtc/p2p/base/p2ptransportchannel.cc

Index: webrtc/p2p/base/p2ptransportchannel.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 3f68d6d6e078fd635e8f696b3b46ae12bae8ce67..0e891777795846a2ffd72c9d1c6346b9c4ee798f 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -470,10 +470,21 @@ void P2PTransportChannel::MaybeStartGathering() {
         OnCandidatesAllocationDone(raw_pooled_session);
       }
     } else {
+      if (!allocator_sessions_.empty()) {
+        IceRegatheringReason reason;
+        if (writable()) {
+          reason = IceRegatheringReason::ICE_RESTART_WHEN_CONNECTED;
+        } else if (IsGettingPorts()) {
+          reason = IceRegatheringReason::ICE_RESTART_WHEN_CONNECTING;
+        } else {
+          reason = IceRegatheringReason::ICE_RESTART_WHEN_DISCONNECTED;
+        }
+        PortAllocatorSession* session = allocator_session();
+        session->SignalIceRegatheringReason(session, reason);

[Taylor Brandstetter, 2016/10/03 20:33:34]

Calling another class's signal breaks encapsulation; only the class that owns
the signal should emit it. Also, it's not clear that this is expected to
increment a UMA stat.

It seems like it would be better if P2PTransportChannel had a pointer to the
metrics interface directly. Is that possible?

[honghaiz3, 2016/10/04 01:03:54]

We can, but it would require some wiring of the pointer from peer connection all
the way to the p2ptransportchannel. 
An alternative, simpler approach is to report the event to the allocator and let
the allocator log the metric. This is more in line with other UMA reporting
where we passed the event dtls error all the way up to the webrtcsession and
webrtcsession wrote the metric. 

[Taylor Brandstetter, 2016/10/04 02:42:32]

I think that's the right thing to do. We'll need to do a similar thing to wire
up the RtcEventLog as well (I assume).
The difference is that in the WebRtcSession case, a parent listened to an event
from its child, and forwarded it upwards; a common pattern. Here, a parent is
creating an event for its child, which the child itself listens to. It's not
very intuitive.

[honghaiz3, 2016/10/05 05:01:53]

Done.

+      }

[Taylor Brandstetter, 2016/10/03 20:33:34]

I think this should be outside the if/else block, so that once candidate pooling
becomes available and applications start to use it, ICE restart stats are
included for it as well.

[honghaiz3, 2016/10/04 01:03:54]

Done.

       AddAllocatorSession(allocator_->CreateSession(
           transport_name(), component(), ice_parameters_.ufrag,
           ice_parameters_.pwd));
-      LOG(LS_INFO) << "Start getting ports";

[Taylor Brandstetter, 2016/10/03 20:33:34]

Why remove this log statement? Is there another one that provides the same
information?

[honghaiz3, 2016/10/04 01:03:54]

I moved it to StartGettingPorts(). It is better to be there than here. 

       allocator_sessions_.back()->StartGettingPorts();
     }
   }