Make sure a track lifetime message is sent even if ice has connected before a MediaStream is added to a PeerConnection.

BUG=361024

Review URL: https://codereview.chromium.org/226743003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262367 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 94 insertions, 36 deletions

diff --git a/content/renderer/media/webrtc/media_stream_track_metrics.cc b/content/renderer/media/webrtc/media_stream_track_metrics.cc
index 29b56cb..59902ff 100644
--- a/content/renderer/media/webrtc/media_stream_track_metrics.cc
+++ b/content/renderer/media/webrtc/media_stream_track_metrics.cc
@@ -204,7 +204,8 @@ void MediaStreamTrackMetricsObserver::ReportTracks(
   }
 }
 
-MediaStreamTrackMetrics::MediaStreamTrackMetrics() {}
+MediaStreamTrackMetrics::MediaStreamTrackMetrics()
+    : ice_state_(webrtc::PeerConnectionInterface::kIceConnectionNew) {}
 
 MediaStreamTrackMetrics::~MediaStreamTrackMetrics() {
   for (ObserverVector::iterator it = observers_.begin(); it != observers_.end();
@@ -216,8 +217,10 @@ MediaStreamTrackMetrics::~MediaStreamTrackMetrics() {
 void MediaStreamTrackMetrics::AddStream(StreamType type,
                                         MediaStreamInterface* stream) {
   DCHECK(CalledOnValidThread());
-  observers_.insert(observers_.end(),
-                    new MediaStreamTrackMetricsObserver(type, stream, this));
+  MediaStreamTrackMetricsObserver* observer =
+      new MediaStreamTrackMetricsObserver(type, stream, this);
+  observers_.insert(observers_.end(), observer);
+  SendLifeTimeMessageDependingOnIceState(observer);
 }
 
 void MediaStreamTrackMetrics::RemoveStream(StreamType type,
@@ -237,41 +240,46 @@ void MediaStreamTrackMetrics::RemoveStream(StreamType type,
 void MediaStreamTrackMetrics::IceConnectionChange(
     PeerConnectionInterface::IceConnectionState new_state) {
   DCHECK(CalledOnValidThread());
+  ice_state_ = new_state;
   for (ObserverVector::iterator it = observers_.begin(); it != observers_.end();
        ++it) {
-    // There is a state transition diagram for these states at
-    // http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCIceConnectionState
-    switch (new_state) {
-      case PeerConnectionInterface::kIceConnectionConnected:
-      case PeerConnectionInterface::kIceConnectionCompleted:
-        (*it)->SendLifetimeMessages(CONNECTED);
-        break;
-
-      case PeerConnectionInterface::kIceConnectionFailed:
-        // We don't really need to handle FAILED (it is only supposed
-        // to be preceded by CHECKING so we wouldn't yet have sent a
-        // lifetime message) but we might as well use belt and
-        // suspenders and handle it the same as the other "end call"
-        // states. It will be ignored anyway if the call is not
-        // already connected.
-      case PeerConnectionInterface::kIceConnectionNew:
-        // It's a bit weird to count NEW as an end-lifetime event, but
-        // it's possible to transition directly from a connected state
-        // (CONNECTED or COMPLETED) to NEW, which can then be followed
-        // by a new connection. The observer will ignore the end
-        // lifetime event if it was not preceded by a begin-lifetime
-        // event.
-      case PeerConnectionInterface::kIceConnectionDisconnected:
-      case PeerConnectionInterface::kIceConnectionClosed:
-        (*it)->SendLifetimeMessages(DISCONNECTED);
-        break;
-
-      default:
-        // We ignore the remaining state (CHECKING) as it is never
-        // involved in a transition from connected to disconnected or
-        // vice versa.
-        break;
-    }
+    SendLifeTimeMessageDependingOnIceState(*it);
+  }
+}
+void MediaStreamTrackMetrics::SendLifeTimeMessageDependingOnIceState(
+    MediaStreamTrackMetricsObserver* observer) {
+  // There is a state transition diagram for these states at
+  // http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCIceConnectionState
+  switch (ice_state_) {
+    case PeerConnectionInterface::kIceConnectionConnected:
+    case PeerConnectionInterface::kIceConnectionCompleted:
+      observer->SendLifetimeMessages(CONNECTED);
+      break;
+
+    case PeerConnectionInterface::kIceConnectionFailed:
+      // We don't really need to handle FAILED (it is only supposed
+      // to be preceded by CHECKING so we wouldn't yet have sent a
+      // lifetime message) but we might as well use belt and
+      // suspenders and handle it the same as the other "end call"
+      // states. It will be ignored anyway if the call is not
+      // already connected.
+    case PeerConnectionInterface::kIceConnectionNew:
+      // It's a bit weird to count NEW as an end-lifetime event, but
+      // it's possible to transition directly from a connected state
+      // (CONNECTED or COMPLETED) to NEW, which can then be followed
+      // by a new connection. The observer will ignore the end
+      // lifetime event if it was not preceded by a begin-lifetime
+      // event.
+    case PeerConnectionInterface::kIceConnectionDisconnected:
+    case PeerConnectionInterface::kIceConnectionClosed:
+      observer->SendLifetimeMessages(DISCONNECTED);
+      break;
+
+    default:
+      // We ignore the remaining state (CHECKING) as it is never
+      // involved in a transition from connected to disconnected or
+      // vice versa.
+      break;
   }
 }
 
diff --git a/content/renderer/media/webrtc/media_stream_track_metrics.h b/content/renderer/media/webrtc/media_stream_track_metrics.h
index 79709b0..aaf669d 100644
--- a/content/renderer/media/webrtc/media_stream_track_metrics.h
+++ b/content/renderer/media/webrtc/media_stream_track_metrics.h
@@ -73,6 +73,10 @@ class CONTENT_EXPORT MediaStreamTrackMetrics : public base::NonThreadSafe {
                                    StreamType stream_type);
 
  protected:
+  // Calls SendLifetimeMessage for |observer| depending on |ice_state_|.
+  void SendLifeTimeMessageDependingOnIceState(
+      MediaStreamTrackMetricsObserver* observer);
+
   // Implements MakeUniqueId. |pc_id| is a cast of this object's
   // |this| pointer to a 64-bit integer, which is usable as a unique
   // ID for the PeerConnection this object is attached to (since there
@@ -88,6 +92,8 @@ class CONTENT_EXPORT MediaStreamTrackMetrics : public base::NonThreadSafe {
 
   typedef ScopedVector<MediaStreamTrackMetricsObserver> ObserverVector;
   ObserverVector observers_;
+
+  webrtc::PeerConnectionInterface::IceConnectionState ice_state_;
 };
 
 }  // namespace
diff --git a/content/renderer/media/webrtc/media_stream_track_metrics_unittest.cc b/content/renderer/media/webrtc/media_stream_track_metrics_unittest.cc
index 9aa76f60..b976c71 100644
--- a/content/renderer/media/webrtc/media_stream_track_metrics_unittest.cc
+++ b/content/renderer/media/webrtc/media_stream_track_metrics_unittest.cc
@@ -199,6 +199,50 @@ TEST_F(MediaStreamTrackMetricsTest, BasicLocalStreams) {
   metrics_->IceConnectionChange(PeerConnectionInterface::kIceConnectionFailed);
 }
 
+TEST_F(MediaStreamTrackMetricsTest, LocalStreamAddedAferIceConnect) {
+  metrics_->IceConnectionChange(
+        PeerConnectionInterface::kIceConnectionConnected);
+
+  EXPECT_CALL(*metrics_,
+              SendLifetimeMessage("audio",
+                                  MediaStreamTrackMetrics::AUDIO_TRACK,
+                                  MediaStreamTrackMetrics::CONNECTED,
+                                  MediaStreamTrackMetrics::SENT_STREAM));
+  EXPECT_CALL(*metrics_,
+              SendLifetimeMessage("video",
+                                  MediaStreamTrackMetrics::VIDEO_TRACK,
+                                  MediaStreamTrackMetrics::CONNECTED,
+                                  MediaStreamTrackMetrics::SENT_STREAM));
+
+  scoped_refptr<MockAudioTrackInterface> audio(MakeAudioTrack("audio"));
+  scoped_refptr<MockVideoTrackInterface> video(MakeVideoTrack("video"));
+  stream_->AddTrack(audio);
+  stream_->AddTrack(video);
+  metrics_->AddStream(MediaStreamTrackMetrics::SENT_STREAM, stream_);
+}
+
+TEST_F(MediaStreamTrackMetricsTest, RemoteStreamAddedAferIceConnect) {
+  metrics_->IceConnectionChange(
+        PeerConnectionInterface::kIceConnectionConnected);
+
+  EXPECT_CALL(*metrics_,
+              SendLifetimeMessage("audio",
+                                  MediaStreamTrackMetrics::AUDIO_TRACK,
+                                  MediaStreamTrackMetrics::CONNECTED,
+                                  MediaStreamTrackMetrics::RECEIVED_STREAM));
+  EXPECT_CALL(*metrics_,
+              SendLifetimeMessage("video",
+                                  MediaStreamTrackMetrics::VIDEO_TRACK,
+                                  MediaStreamTrackMetrics::CONNECTED,
+                                  MediaStreamTrackMetrics::RECEIVED_STREAM));
+
+  scoped_refptr<MockAudioTrackInterface> audio(MakeAudioTrack("audio"));
+  scoped_refptr<MockVideoTrackInterface> video(MakeVideoTrack("video"));
+  stream_->AddTrack(audio);
+  stream_->AddTrack(video);
+  metrics_->AddStream(MediaStreamTrackMetrics::RECEIVED_STREAM, stream_);
+}
+
 TEST_F(MediaStreamTrackMetricsTest, RemoteStreamTrackAdded) {
   scoped_refptr<MockAudioTrackInterface> initial(MakeAudioTrack("initial"));
   scoped_refptr<MockAudioTrackInterface> added(MakeAudioTrack("added"));