Implement ConstraintNotSatisfiedError for getusermedia

content/renderer/media/media_stream_impl.cc

Index: content/renderer/media/media_stream_impl.cc
diff --git a/content/renderer/media/media_stream_impl.cc b/content/renderer/media/media_stream_impl.cc
index 887b593b9cf42eb91f91e2783d2b82532dcce765..adad891478890588ec568c800ef56ab22dd7da63 100644
--- a/content/renderer/media/media_stream_impl.cc
+++ b/content/renderer/media/media_stream_impl.cc
@@ -336,7 +336,7 @@ void MediaStreamImpl::OnStreamGenerated(
 // The requested stream failed to be generated.
 void MediaStreamImpl::OnStreamGenerationFailed(
     int request_id,
-    content::MediaStreamRequestResult result) {
+    MediaStreamRequestResult result) {
   DCHECK(CalledOnValidThread());
   DVLOG(1) << "MediaStreamImpl::OnStreamGenerationFailed("
            << request_id << ")";
@@ -494,14 +494,15 @@ void MediaStreamImpl::CreateAudioTracks(
 
 void MediaStreamImpl::OnCreateNativeTracksCompleted(
     UserMediaRequestInfo* request,
-    content::MediaStreamRequestResult result) {
+    MediaStreamRequestResult result,
+    const blink::WebString& result_name) {
   DVLOG(1) << "MediaStreamImpl::OnCreateNativeTracksComplete("
            << "{request_id = " << request->request_id << "} "
            << "{result = " << result << "})";
   if (result == content::MEDIA_DEVICE_OK)
     GetUserMediaRequestSucceeded(request->web_stream, &request->request);
   else
-    GetUserMediaRequestFailed(&request->request, result);
+    GetUserMediaRequestFailed(&request->request, result, result_name);
 
   DeleteUserMediaRequestInfo(request);
 }
@@ -606,7 +607,7 @@ void MediaStreamImpl::GetUserMediaRequestSucceeded(
 
 void MediaStreamImpl::GetUserMediaRequestFailed(
     blink::WebUserMediaRequest* request_info,
-    content::MediaStreamRequestResult result) {
+    MediaStreamRequestResult result) {
   switch (result) {
     case MEDIA_DEVICE_OK:
       NOTREACHED();
@@ -635,6 +636,20 @@ void MediaStreamImpl::GetUserMediaRequestFailed(
     case MEDIA_DEVICE_CAPTURE_FAILURE:
       request_info->requestFailedUASpecific("DeviceCaptureError");
       break;
+    default:

[miu, 2014/07/30 19:37:23]

It's generally not good practice to use "default" clauses when handling enum
values.  Please add handling for all values so that the compiler will force
someone to account for a new result value (if added in the future).

In addition, if there are cases that should not be reached, use NOTREACHED()
before the request_info->requestFailed() call, to ensure debug builds will crash
on a logic error.

[jiajia.qin, 2014/07/31 13:43:00]

Done.

[miu, 2014/07/31 19:30:09]

You should still call request_info->requestFailed() and break after the
NOTREACHED(), since non-debug binaries will pass by the NOTREACHED().

[jiajia.qin, 2014/08/01 05:25:33]

Done.

+      request_info->requestFailed();
+      break;
+  }
+}
+
+void MediaStreamImpl::GetUserMediaRequestFailed(

[miu, 2014/07/30 19:37:23]

IMHO, this shouldn't be a separate method.  Can you just add the new 3rd arg to
the existing one, and add these additional cases?

If you strongly disagree then, at the least, the name of this method should be
changed to indicate its specific meaning/difference from the other one. 
Something to make it more obvious when this method is supposed to handle a
failure versus the other.  Suggestion: StartUserMediaRequestFailed() or
CompleteUserMediaRequestFailed().

[jiajia.qin, 2014/07/31 13:43:00]

The new 3rd arg is only for 'TrackStartError'. For other errors, we don't need
to pass the 3rd arg. So I separate this method. I use
'GetUserMediaRequestTrackStartedFailed' to rename this new function. It seems
more meaningful.

+    blink::WebUserMediaRequest* request_info,
+    MediaStreamRequestResult result,
+    const blink::WebString& result_name) {
+  switch (result) {
+    case MEDIA_DEVICE_CONSTRAINT_NOT_SATISFIED:
+      request_info->requestFailedConstraint(result_name);
+      break;
     case MEDIA_DEVICE_TRACK_START_FAILURE:
       request_info->requestFailedUASpecific("TrackStartError");
       break;
@@ -862,8 +877,10 @@ void MediaStreamImpl::UserMediaRequestInfo::CallbackOnTracksStarted(
 }
 
 void MediaStreamImpl::UserMediaRequestInfo::OnTrackStarted(
-    MediaStreamSource* source, bool success) {
-  DVLOG(1) << "OnTrackStarted result " << success;
+    MediaStreamSource* source,
+    MediaStreamRequestResult result,
+    const blink::WebString& result_name) {
+  DVLOG(1) << "OnTrackStarted result " << result;
   std::vector<MediaStreamSource*>::iterator it =
       std::find(sources_waiting_for_callback_.begin(),
                 sources_waiting_for_callback_.end(),
@@ -872,16 +889,19 @@ void MediaStreamImpl::UserMediaRequestInfo::OnTrackStarted(
   sources_waiting_for_callback_.erase(it);
   // All tracks must be started successfully. Otherwise the request is a
   // failure.
-  if (!success)
+  if (result != MEDIA_DEVICE_OK)
     request_failed_ = true;
-  CheckAllTracksStarted();
+  if (!ready_callback_.is_null() && sources_waiting_for_callback_.empty()) {
+    ready_callback_.Run(this, result, result_name);
+  }
 }
 
 void MediaStreamImpl::UserMediaRequestInfo::CheckAllTracksStarted() {
   if (!ready_callback_.is_null() && sources_waiting_for_callback_.empty()) {
     ready_callback_.Run(
         this,
-        request_failed_ ? MEDIA_DEVICE_TRACK_START_FAILURE : MEDIA_DEVICE_OK);
+        request_failed_ ? MEDIA_DEVICE_TRACK_START_FAILURE : MEDIA_DEVICE_OK,
+        "");
   }
 }