Bug 1438134 - Make the return value of MediaEngineSource::Reconfigure well defined. r=jib

MozReview-Commit-ID: DR3sdtdZkob

--HG--
extra : rebase_source : 35d60c11c8bd50547062af708cb009f7835b5893

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;
     }
   }
 

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,

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,
+  nsresult rv = ReevaluateAllocation(aHandle, &constraints, aPrefs, aDeviceId,
-                              aOutBadConstraint);
+                                     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)