Add UMA metrics for ICE regathering reasons.

webrtc/p2p/base/p2ptransportchannel.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 452 matching lines @@
       // Process the pooled session's existing candidates/ports, if they exist.
       OnCandidatesReady(raw_pooled_session,
                         raw_pooled_session->ReadyCandidates());
       for (PortInterface* port : allocator_sessions_.back()->ReadyPorts()) {
         OnPortReady(raw_pooled_session, port);
       }
       if (allocator_sessions_.back()->CandidatesAllocationDone()) {
         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();
     }
   }
 }
 
 // A new port is available, attempt to make connections for it
 void P2PTransportChannel::OnPortReady(PortAllocatorSession *session,
                                       PortInterface* port) {
   ASSERT(network_thread_ == rtc::Thread::Current());
 
@@ skipping 1480 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket