Correctly update the networkState on media elements playing MediaStreams

With this patch, the network state of elements playing inactive streams
is NETWORK_IDLE and the state of elements playing active streams is
NETWORK_LOADING.

Before this CL, the network state was always NETWORK_LOADING, which
prevented media elements from being garbage collected, which was a
source of JavaScript memory leaks and made it easy to reach limits in
the number of active low-level audio streams.

Drive-by: Fix the case of addition of stopped tracks to a stream.

Bug: 771550
Change-Id: Id0d8e27fc30c9850e5ad43d7a3887c03a9415a18
Reviewed-on: https://chromium-review.googlesource.com/1057808
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558655}

content/renderer/media/stream/webmediaplayer_ms.cc

@@ -42,12 +42,19 @@
 #include "third_party/blink/public/web/web_local_frame.h"
 
 namespace {
+
 enum class RendererReloadAction {
   KEEP_RENDERER,
   REMOVE_RENDERER,
   NEW_RENDERER
 };
 
+bool IsPlayableTrack(const blink::WebMediaStreamTrack& track) {
+  return !track.IsNull() && !track.Source().IsNull() &&
+         track.Source().GetReadyState() !=
+             blink::WebMediaStreamSource::kReadyStateEnded;
+}
+
 }  // namespace
 
 namespace content {
@@ -417,6 +424,16 @@ void WebMediaPlayerMS::TrackRemoved(const blink::WebMediaStreamTrack& track) {
   Reload();
 }
 
+void WebMediaPlayerMS::ActiveStateChanged(bool is_active) {
+  if (!is_active) {
+    // This makes the media element elegible to be garbage collected. Otherwise,
+    // the element will be considered active and will never be garbage
+    // collected.
+    SetNetworkState(kNetworkStateIdle);
+  }
+  // The case when the stream becomes active is handled by TrackAdded().
+}
+
 void WebMediaPlayerMS::Reload() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (web_stream_.IsNull())
@@ -438,7 +455,8 @@ void WebMediaPlayerMS::ReloadVideo() {
     if (video_frame_provider_)
       renderer_action = RendererReloadAction::REMOVE_RENDERER;
     current_video_track_id_ = blink::WebString();
-  } else if (video_tracks[0].Id() != current_video_track_id_) {
+  } else if (video_tracks[0].Id() != current_video_track_id_ &&
+             IsPlayableTrack(video_tracks[0])) {
     renderer_action = RendererReloadAction::NEW_RENDERER;
     current_video_track_id_ = video_tracks[0].Id();
   }
@@ -448,6 +466,7 @@ void WebMediaPlayerMS::ReloadVideo() {
       if (video_frame_provider_)
         video_frame_provider_->Stop();
 
+      SetNetworkState(kNetworkStateLoading);
       video_frame_provider_ = renderer_factory_->GetVideoRenderer(
           web_stream_,
           media::BindToCurrentLoop(
@@ -487,7 +506,8 @@ void WebMediaPlayerMS::ReloadAudio() {
     if (audio_renderer_)
       renderer_action = RendererReloadAction::REMOVE_RENDERER;
     current_audio_track_id_ = blink::WebString();
-  } else if (audio_tracks[0].Id() != current_video_track_id_) {
+  } else if (audio_tracks[0].Id() != current_audio_track_id_ &&
+             IsPlayableTrack(audio_tracks[0])) {
     renderer_action = RendererReloadAction::NEW_RENDERER;
     current_audio_track_id_ = audio_tracks[0].Id();
   }
@@ -497,8 +517,14 @@ void WebMediaPlayerMS::ReloadAudio() {
       if (audio_renderer_)
         audio_renderer_->Stop();
 
+      SetNetworkState(WebMediaPlayer::kNetworkStateLoading);
       audio_renderer_ = renderer_factory_->GetAudioRenderer(
           web_stream_, frame->GetRoutingID(), initial_audio_output_device_id_);
+
+      // |audio_renderer_| can be null in tests.
+      if (!audio_renderer_)
+        break;
+
       audio_renderer_->SetVolume(volume_);
       audio_renderer_->Start();
       audio_renderer_->Play();

content/renderer/media/stream/webmediaplayer_ms.h

@@ -195,6 +195,7 @@ class CONTENT_EXPORT WebMediaPlayerMS
   // blink::WebMediaStreamObserver implementation
   void TrackAdded(const blink::WebMediaStreamTrack& track) override;
   void TrackRemoved(const blink::WebMediaStreamTrack& track) override;
+  void ActiveStateChanged(bool is_active) override;
 
  private:
   friend class WebMediaPlayerMSTest;

third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-MediaElement-networkState.html

+<!doctype html>
+<html>
+<head>
+<title>The networkState of a media element that has assigned a MediaStream
+  should be NETWORK_LOADING if the stream is active and NETWORK_IDLE if the
+  stream is inactive.
+</title>
+</head>
+<body>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script>
+var t = async_test("Tests that the networkState of a video element playing a " +
+                   "MediaStream is updated correctly.");
+t.step(function() {
+  navigator.mediaDevices.getUserMedia({audio:true, video: true})
+    .then(t.step_func(stream => {
+      var video = document.createElement('video');
+      assert_equals(video.networkState, HTMLMediaElement.NETWORK_EMPTY);
+      video.srcObject = stream;
+      setTimeout(()=>{
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_LOADING);
+        stream.getTracks().forEach(track => track.stop());
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+        // Removing and re-adding the stopped audio track does not change the
+        // network state.
+        var track = stream.getAudioTracks()[0];
+        stream.removeTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+        stream.addTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+        // Removing and re-adding the stopped tracks does not change the
+        // network state.
+        var track = stream.getVideoTracks()[0];
+        stream.removeTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+        stream.addTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+        var track = stream.getAudioTracks()[0];
+        stream.removeTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+        stream.addTrack(track);
+        assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+        // Replacing the existing tracks with new active tracks does change the
+        // network state.
+        navigator.mediaDevices.getUserMedia({audio:true, video:true}).then(
+          t.step_func(stream2 => {
+            stream.removeTrack(stream.getAudioTracks()[0]);
+            stream.addTrack(stream2.getAudioTracks()[0]);
+            assert_equals(video.networkState, HTMLMediaElement.NETWORK_LOADING);
+
+            stream.removeTrack(stream.getAudioTracks()[0]);
+            assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+            stream.removeTrack(stream.getVideoTracks()[0]);
+            stream.addTrack(stream2.getVideoTracks()[0]);
+            assert_equals(video.networkState, HTMLMediaElement.NETWORK_LOADING);
+
+            stream.removeTrack(stream.getVideoTracks()[0]);
+            assert_equals(video.networkState, HTMLMediaElement.NETWORK_IDLE);
+
+            t.done();
+          }), t.unreached_func);
+      });
+    }), t.unreached_func);
+});
+</script>
+</body>
+</html>

third_party/blink/public/platform/web_media_stream.h

@@ -38,9 +38,12 @@ class WebString;
 class BLINK_PLATFORM_EXPORT WebMediaStreamObserver {
  public:
   // TrackAdded is called when |track| is added to the observed MediaStream.
-  virtual void TrackAdded(const blink::WebMediaStreamTrack&) = 0;
+  virtual void TrackAdded(const blink::WebMediaStreamTrack&) {}
   // TrackRemoved is called when |track| is added to the observed MediaStream.
-  virtual void TrackRemoved(const blink::WebMediaStreamTrack&) = 0;
+  virtual void TrackRemoved(const blink::WebMediaStreamTrack&) {}
+  // ActiveStateChanged is called when the observed MediaStream becomes either
+  // active or inactive.
+  virtual void ActiveStateChanged(bool is_active) {}
 
  protected:
   virtual ~WebMediaStreamObserver() = default;

third_party/blink/renderer/platform/mediastream/media_stream_descriptor.cc

@@ -80,9 +80,10 @@ void MediaStreamDescriptor::AddComponent(MediaStreamComponent* component) {
       break;
   }
 
-  for (auto*& observer : observers_) {
+  // Iterate over a copy of |observers_| to avoid re-entrancy issues.
+  Vector<WebMediaStreamObserver*> observers = observers_;
+  for (auto*& observer : observers)
     observer->TrackAdded(component);
-  }
 }
 
 void MediaStreamDescriptor::RemoveComponent(MediaStreamComponent* component) {
@@ -100,9 +101,10 @@ void MediaStreamDescriptor::RemoveComponent(MediaStreamComponent* component) {
       break;
   }
 
-  for (auto*& observer : observers_) {
+  // Iterate over a copy of |observers_| to avoid re-entrancy issues.
+  Vector<WebMediaStreamObserver*> observers = observers_;
+  for (auto*& observer : observers)
     observer->TrackRemoved(component);
-  }
 }
 
 void MediaStreamDescriptor::AddRemoteTrack(MediaStreamComponent* component) {
@@ -119,6 +121,17 @@ void MediaStreamDescriptor::RemoveRemoteTrack(MediaStreamComponent* component) {
     RemoveComponent(component);
 }
 
+void MediaStreamDescriptor::SetActive(bool active) {
+  if (active == active_)
+    return;
+
+  active_ = active;
+  // Iterate over a copy of |observers_| to avoid re-entrancy issues.
+  Vector<WebMediaStreamObserver*> observers = observers_;
+  for (auto*& observer : observers)
+    observer->ActiveStateChanged(active_);
+}
+
 void MediaStreamDescriptor::AddObserver(WebMediaStreamObserver* observer) {
   DCHECK_EQ(observers_.Find(observer), kNotFound);
   observers_.push_back(observer);

third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h

@@ -102,7 +102,7 @@ class PLATFORM_EXPORT MediaStreamDescriptor final
   void RemoveRemoteTrack(MediaStreamComponent*);
 
   bool Active() const { return active_; }
-  void SetActive(bool active) { active_ = active; }
+  void SetActive(bool active);
 
   void AddObserver(WebMediaStreamObserver*);
   void RemoveObserver(WebMediaStreamObserver*);