From 4c95dc3099a29b09cb5b5b9d71ff69970ac0c4d2 Mon Sep 17 00:00:00 2001
From: Andreas Pehrson <pehrsons@mozilla.com>
Date: Thu, 22 Feb 2018 15:35:16 +0100
Subject: [PATCH] Bug 1438134 - Make the return value of
 MediaEngineSource::Reconfigure well defined. r=jib

MozReview-Commit-ID: DR3sdtdZkob

--HG--
extra : rebase_source : 35d60c11c8bd50547062af708cb009f7835b5893
---
 .../webrtc/MediaEngineRemoteVideoSource.cpp   | 16 +++++++++++++---
 dom/media/webrtc/MediaEngineSource.h          |  8 ++++++++
 dom/media/webrtc/MediaEngineWebRTCAudio.cpp   | 19 +++++++++++++++++--
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
index e5dcbd8342ac..2967301d0c24 100644
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -9,6 +9,7 @@
 #include "CamerasChild.h"
 #include "MediaManager.h"
 #include "MediaTrackConstraints.h"
+#include "mozilla/ErrorNames.h"
 #include "mozilla/RefPtr.h"
 #include "nsIPrefService.h"
 #include "VideoFrameUtils.h"
@@ -333,6 +334,7 @@ MediaEngineRemoteVideoSource::Stop(const RefPtr<const AllocationHandle>& aHandle
   if (camera::GetChildAndCall(&camera::CamerasChild::StopCapture,
                               mCapEngine, mCaptureIndex)) {
     MOZ_DIAGNOSTIC_ASSERT(false, "Stopping a started capture failed");
+    return NS_ERROR_FAILURE;
   }
 
   {
@@ -366,7 +368,7 @@ MediaEngineRemoteVideoSource::Reconfigure(const RefPtr<AllocationHandle>& aHandl
   if (!ChooseCapability(constraints, aPrefs, aDeviceId, newCapability, kFitness)) {
     *aOutBadConstraint =
       MediaConstraintsHelper::FindBadConstraint(constraints, this, aDeviceId);
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_INVALID_ARG;
   }
   LOG(("ChooseCapability(kFitness) for mTargetCapability (Reconfigure) --"));
 
@@ -380,7 +382,11 @@ MediaEngineRemoteVideoSource::Reconfigure(const RefPtr<AllocationHandle>& aHandl
     // We can safely pass nullptr below.
     nsresult rv = Stop(nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
+      nsAutoCString name;
+      GetErrorName(rv, name);
+      LOG(("Video source %p for video device %d Reconfigure() failed "
+           "unexpectedly in Stop(). rv=%s", this, mCaptureIndex, name.Data()));
+      return NS_ERROR_UNEXPECTED;
     }
   }
 
@@ -393,7 +399,11 @@ MediaEngineRemoteVideoSource::Reconfigure(const RefPtr<AllocationHandle>& aHandl
   if (started) {
     nsresult rv = Start(nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
+      nsAutoCString name;
+      GetErrorName(rv, name);
+      LOG(("Video source %p for video device %d Reconfigure() failed "
+           "unexpectedly in Start(). rv=%s", this, mCaptureIndex, name.Data()));
+      return NS_ERROR_UNEXPECTED;
     }
   }
 
diff --git a/dom/media/webrtc/MediaEngineSource.h b/dom/media/webrtc/MediaEngineSource.h
index 8761be83862d..c5b2ed85df60 100644
--- a/dom/media/webrtc/MediaEngineSource.h
+++ b/dom/media/webrtc/MediaEngineSource.h
@@ -158,6 +158,14 @@ public:
    * Should the constraints lead to choosing a new capability while the device
    * is actively being captured, the device will restart using the new
    * capability.
+   *
+   * We return one of the following:
+   * NS_OK                - Successful reconfigure.
+   * NS_ERROR_INVALID_ARG - Couldn't find a capability fitting aConstraints.
+   *                        See aBadConstraint for details.
+   * NS_ERROR_UNEXPECTED  - Reconfiguring the underlying device failed
+   *                        unexpectedly. This leaves the device in a stopped
+   *                        state.
    */
   virtual nsresult Reconfigure(const RefPtr<AllocationHandle>& aHandle,
                                const dom::MediaTrackConstraints& aConstraints,
diff --git a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
index ea9bad6a6dc1..4a518aad6905 100644
--- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -14,6 +14,7 @@
 #include "MediaStreamGraphImpl.h"
 #include "MediaTrackConstraints.h"
 #include "mozilla/Assertions.h"
+#include "mozilla/ErrorNames.h"
 #include "mtransport/runnable_utils.h"
 #include "nsAutoPtr.h"
 
@@ -232,8 +233,22 @@ MediaEngineWebRTCMicrophoneSource::Reconfigure(const RefPtr<AllocationHandle>& a
   LOG(("Mic source %p allocation %p Reconfigure()", this, aHandle.get()));
 
   NormalizedConstraints constraints(aConstraints);
-  return ReevaluateAllocation(aHandle, &constraints, aPrefs, aDeviceId,
-                              aOutBadConstraint);
+  nsresult rv = ReevaluateAllocation(aHandle, &constraints, aPrefs, aDeviceId,
+                                     aOutBadConstraint);
+  if (NS_FAILED(rv)) {
+    if (aOutBadConstraint) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    nsAutoCString name;
+    GetErrorName(rv, name);
+    LOG(("Mic source %p Reconfigure() failed unexpectedly. rv=%s",
+         this, name.Data()));
+    Stop(aHandle);
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  return NS_OK;
 }
 
 bool operator == (const MediaEnginePrefs& a, const MediaEnginePrefs& b)