Make sure that a MediaStreamVideoSource can be deleted in the context of a failure callback.

TEST= content_unittests MediaStreamVideoSourceTest.ReleaseTrackAndSourceOnFailureCallBack
BUG= 364207

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

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

3 files changed, 49 insertions, 1 deletions

diff --git a/content/renderer/media/media_stream_video_source.cc b/content/renderer/media/media_stream_video_source.cc
index 7a0ec71..73ee6ec 100644
--- a/content/renderer/media/media_stream_video_source.cc
+++ b/content/renderer/media/media_stream_video_source.cc
@@ -437,8 +437,10 @@ void MediaStreamVideoSource::OnSupportedFormats(
                                      &current_format_,
                                      &max_frame_output_size_,
                                      &current_constraints_)) {
-    FinalizeAddTrack();
     SetReadyState(blink::WebMediaStreamSource::ReadyStateEnded);
+    // This object can be deleted after calling FinalizeAddTrack. See comment
+    // in the header file.
+    FinalizeAddTrack();
     return;
   }
 
@@ -493,6 +495,8 @@ void MediaStreamVideoSource::OnStartDone(bool success) {
     StopSourceImpl();
   }
 
+  // This object can be deleted after calling FinalizeAddTrack. See comment in
+  // the header file.
   FinalizeAddTrack();
 }
 
diff --git a/content/renderer/media/media_stream_video_source.h b/content/renderer/media/media_stream_video_source.h
index bd0af43..3bb8b0b 100644
--- a/content/renderer/media/media_stream_video_source.h
+++ b/content/renderer/media/media_stream_video_source.h
@@ -150,6 +150,10 @@ class CONTENT_EXPORT MediaStreamVideoSource
   // Trigger all cached callbacks from AddTrack. AddTrack is successful
   // if the capture delegate has started and the constraints provided in
   // AddTrack match the format that was used to start the device.
+  // Note that it must be ok to delete the MediaStreamVideoSource object
+  // in the context of the callback. If gUM fail, the implementation will
+  // simply drop the references to the blink source and track which will lead
+  // to that this object is deleted.
   void FinalizeAddTrack();
 
   State state_;
diff --git a/content/renderer/media/media_stream_video_source_unittest.cc b/content/renderer/media/media_stream_video_source_unittest.cc
index 1b79ec1..0c3963c 100644
--- a/content/renderer/media/media_stream_video_source_unittest.cc
+++ b/content/renderer/media/media_stream_video_source_unittest.cc
@@ -120,6 +120,11 @@ class MediaStreamVideoSourceTest
     EXPECT_EQ(expected_width, adapter->GetLastFrameHeight());
   }
 
+  void ReleaseTrackAndSourceOnAddTrackCallback(
+      const blink::WebMediaStreamTrack& track_to_release) {
+    track_to_release_ = track_to_release;
+  }
+
  private:
   void OnConstraintsApplied(MediaStreamSource* source, bool success) {
     ASSERT_EQ(source, webkit_source_.extraData());
@@ -128,8 +133,15 @@ class MediaStreamVideoSourceTest
       ++number_of_successful_constraints_applied_;
     else
       ++number_of_failed_constraints_applied_;
+
+    if (!track_to_release_.isNull()) {
+      mock_source_ = NULL;
+      webkit_source_.reset();
+      track_to_release_.reset();
+    }
   }
 
+  blink::WebMediaStreamTrack track_to_release_;
   int number_of_successful_constraints_applied_;
   int number_of_failed_constraints_applied_;
   MockMediaStreamDependencyFactory factory_;
@@ -237,6 +249,34 @@ TEST_F(MediaStreamVideoSourceTest, MandatoryAspectRatioTooHigh) {
   EXPECT_EQ(1, NumberOfFailedConstraintsCallbacks());
 }
 
+// Test that its safe to release the last reference of a blink track and the
+// source during the callback if adding a track succeeds.
+TEST_F(MediaStreamVideoSourceTest, ReleaseTrackAndSourceOnSuccessCallBack) {
+  MockMediaConstraintFactory factory;
+  {
+    blink::WebMediaStreamTrack track =
+        CreateTrack("123", factory.CreateWebMediaConstraints());
+    ReleaseTrackAndSourceOnAddTrackCallback(track);
+  }
+  mock_source()->CompleteGetSupportedFormats();
+  mock_source()->StartMockedSource();
+  EXPECT_EQ(1, NumberOfSuccessConstraintsCallbacks());
+}
+
+// Test that its safe to release the last reference of a blink track and the
+// source during the callback if adding a track fails.
+TEST_F(MediaStreamVideoSourceTest, ReleaseTrackAndSourceOnFailureCallBack) {
+  MockMediaConstraintFactory factory;
+  factory.AddMandatory(MediaStreamVideoSource::kMinAspectRatio, 2);
+  {
+    blink::WebMediaStreamTrack track =
+        CreateTrack("123", factory.CreateWebMediaConstraints());
+    ReleaseTrackAndSourceOnAddTrackCallback(track);
+  }
+  mock_source()->CompleteGetSupportedFormats();
+  EXPECT_EQ(1, NumberOfFailedConstraintsCallbacks());
+}
+
 // Test that the source ignores an optional aspect ratio that is higher than
 // supported.
 TEST_F(MediaStreamVideoSourceTest, OptionalAspectRatioTooHigh) {