TransportController refactoring

talk/app/webrtc/statscollector.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 674 matching lines @@
   // the proxy map directly from the session stats.
   // As is, if GetStats() failed, we could be using old (incorrect?) proxy
   // data.
   proxy_to_transport_ = stats.proxy_to_transport;
 
   for (const auto& transport_iter : stats.transport_stats) {
     // Attempt to get a copy of the certificates from the transport and
     // expose them in stats reports.  All channels in a transport share the
     // same local and remote certificates.
     //
-    // Note that Transport::GetIdentity and Transport::GetRemoteCertificate
-    // invoke method calls on the worker thread and block this thread, but
-    // messages are still processed on this thread, which may blow way the
-    // existing transports. So we cannot reuse |transport| after these calls.
     StatsReport::Id local_cert_report_id, remote_cert_report_id;
-
-    cricket::Transport* transport =
-        session_->GetTransport(transport_iter.second.content_name);
     rtc::scoped_ptr<rtc::SSLIdentity> identity;
-    if (transport && transport->GetIdentity(identity.accept())) {
+    if (session_->GetIdentity(transport_iter.second.content_name,
+                              identity.accept())) {
       StatsReport* r = AddCertificateReports(&(identity->certificate()));
       if (r)
         local_cert_report_id = r->id();
     }
 
-    transport = session_->GetTransport(transport_iter.second.content_name);
     rtc::scoped_ptr<rtc::SSLCertificate> cert;
-    if (transport && transport->GetRemoteCertificate(cert.accept())) {
+    if (session_->GetRemoteCertificate(transport_iter.second.content_name,
+                                       cert.accept())) {

[pthatcher1, 2015/08/10 20:40:16]

Would it make more sense for GetIdentity and GetRemoteCertificate to be on the
TransportController?

[Taylor Brandstetter, 2015/08/11 01:20:06]

They ARE methods on the TransportController. But the thing is, WebRtcSession
needs to map a content name to a transport name. Also, WebRtcSession doesn't
directly expose the TransportController interface to StatsCollector.

[pthatcher1, 2015/08/18 22:32:46]

Ah, so the transport_iter.second.content_name is the content name, not the
transport name, and the proxy_to_transport_ (now misnamed) converts from
content_name to the transport_name.  

That all seems like a big mess, but it looks like you're retaining the existing
behavior in a clean way.  We'll have to shave the stats yak another time.

       StatsReport* r = AddCertificateReports(cert.get());
       if (r)
         remote_cert_report_id = r->id();
     }
 
     for (const auto& channel_iter : transport_iter.second.channel_stats) {
       StatsReport::Id id(StatsReport::NewComponentId(
           transport_iter.second.content_name, channel_iter.component));
       StatsReport* channel_report = reports_.ReplaceOrAddNew(id);
       channel_report->set_timestamp(stats_gathering_started_);
@@ skipping 204 matching lines @@
 
 void StatsCollector::UpdateTrackReports() {
   DCHECK(session_->signaling_thread()->IsCurrent());
 
   rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
 
   for (const auto& entry : track_ids_) {
     StatsReport* report = entry.second;
     report->set_timestamp(stats_gathering_started_);
   }
-
 }
 
 void StatsCollector::ClearUpdateStatsCacheForTest() {
   stats_gathering_started_ = 0;
 }
 
 }  // namespace webrtc