Remove dangling PlayingState pointers in WebRtcAudioRenderer.

content/renderer/media/webrtc_audio_renderer.cc

Index: content/renderer/media/webrtc_audio_renderer.cc
diff --git a/content/renderer/media/webrtc_audio_renderer.cc b/content/renderer/media/webrtc_audio_renderer.cc
index 32c27bf8be65487b9e021bf5e6a3e95d8345ab66..586be2386cea51e8f62d95e634928b153124dcd7 100644
--- a/content/renderer/media/webrtc_audio_renderer.cc
+++ b/content/renderer/media/webrtc_audio_renderer.cc
@@ -4,6 +4,7 @@
 
 #include "content/renderer/media/webrtc_audio_renderer.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ -55,7 +56,8 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
   // Callback definition for a callback that is called when when Play(), Pause()
   // or SetVolume are called (whenever the internal |playing_state_| changes).
   typedef base::Callback<void(const blink::WebMediaStream&,
-                              WebRtcAudioRenderer::PlayingState*)>
+                              WebRtcAudioRenderer::PlayingState*,
+                              bool playing_state_deleted)>
       OnPlayStateChanged;
 
   SharedAudioRenderer(const scoped_refptr<MediaStreamAudioRenderer>& delegate,
@@ -74,6 +76,8 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
     DCHECK(thread_checker_.CalledOnValidThread());
     DVLOG(1) << __func__;
     Stop();
+    // Make extra sure no reference to the playing state is left.
+    on_play_state_changed_.Run(media_stream_, &playing_state_, true);

[tommi (sloooow) - chröme, 2017/03/17 12:44:08]

did you consider having a separate callback for when a shared renderer goes
away?

Alternatively, since a SharedAudioRenderer does have a pointer to the root
WebRtcAudioRenderer, what about calling a method such as
delegate_->OnSharedAudioRendererGone(this)?  (I suppose we can't call
delegate_->RemovePlayingState()?)

[Max Morin, 2017/03/17 14:08:04]

I went ahead and kept delegate_ as a pointer to interface and used a callback,
consistent with how OnPlayStateChanged is treated. Calling RemovePlayingState
directly isn't possible since we don't know which sources to call it with.

   }
 
   void Start() override {
@@ -90,7 +94,7 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
     if (playing_state_.playing())
       return;
     playing_state_.set_playing(true);
-    on_play_state_changed_.Run(media_stream_, &playing_state_);
+    on_play_state_changed_.Run(media_stream_, &playing_state_, false);
   }
 
   void Pause() override {
@@ -99,7 +103,7 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
     if (!playing_state_.playing())
       return;
     playing_state_.set_playing(false);
-    on_play_state_changed_.Run(media_stream_, &playing_state_);
+    on_play_state_changed_.Run(media_stream_, &playing_state_, false);
   }
 
   void Stop() override {
@@ -115,7 +119,7 @@ class SharedAudioRenderer : public MediaStreamAudioRenderer {
     DCHECK(thread_checker_.CalledOnValidThread());
     DCHECK(volume >= 0.0f && volume <= 1.0f);
     playing_state_.set_volume(volume);
-    on_play_state_changed_.Run(media_stream_, &playing_state_);
+    on_play_state_changed_.Run(media_stream_, &playing_state_, false);
   }
 
   media::OutputDeviceInfo GetOutputDeviceInfo() override {
@@ -249,7 +253,7 @@ void WebRtcAudioRenderer::Play() {
 
   playing_state_.set_playing(true);
 
-  OnPlayStateChanged(media_stream_, &playing_state_);
+  OnPlayStateChanged(media_stream_, &playing_state_, false);
 }
 
 void WebRtcAudioRenderer::EnterPlayState() {
@@ -281,7 +285,7 @@ void WebRtcAudioRenderer::Pause() {
 
   playing_state_.set_playing(false);
 
-  OnPlayStateChanged(media_stream_, &playing_state_);
+  OnPlayStateChanged(media_stream_, &playing_state_, false);
 }
 
 void WebRtcAudioRenderer::EnterPauseState() {
@@ -340,7 +344,7 @@ void WebRtcAudioRenderer::SetVolume(float volume) {
   DCHECK(volume >= 0.0f && volume <= 1.0f);
 
   playing_state_.set_volume(volume);
-  OnPlayStateChanged(media_stream_, &playing_state_);
+  OnPlayStateChanged(media_stream_, &playing_state_, false);
 }
 
 media::OutputDeviceInfo WebRtcAudioRenderer::GetOutputDeviceInfo() {
@@ -566,8 +570,27 @@ bool WebRtcAudioRenderer::RemovePlayingState(
 
 void WebRtcAudioRenderer::OnPlayStateChanged(
     const blink::WebMediaStream& media_stream,
-    PlayingState* state) {
+    PlayingState* state,
+    bool playing_state_deleted) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (playing_state_deleted) {
+    // It is possible we associated |state| to a source for a track that is no
+    // longer in |media_stream|. We iterate over |source_playing_states_| to
+    // ensure there are no dangling pointers to |state| there. See
+    // crbug.com/697256.
+    for (auto it = source_playing_states_.begin();
+         it != source_playing_states_.end();) {
+      PlayingStates& states = it->second;
+      // We cannot use RemovePlayingState as it might invalidate |it|.
+      states.erase(std::remove(states.begin(), states.end(), state),
+                   states.end());
+      if (states.empty())
+        it = source_playing_states_.erase(it);
+      else
+        ++it;
+    }
+    return;
+  }
   blink::WebVector<blink::WebMediaStreamTrack> web_tracks;
   media_stream.audioTracks(web_tracks);