Refactor MediaStreamManager to never output real device id. It now always output sourceId in the fo…

content/browser/renderer_host/media/media_stream_manager.cc

Index: content/browser/renderer_host/media/media_stream_manager.cc
diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc
index 2a0a868eb4f4ae56e257230359e72cbe99311331..983224849c2c4ec07919f24453ec0c65f540a872 100644
--- a/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/content/browser/renderer_host/media/media_stream_manager.cc
@@ -207,7 +207,7 @@ std::string MediaStreamManager::MakeMediaAccessRequest(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Create a new request based on options.
   MediaStreamRequest stream_request(
-      render_process_id, render_view_id, page_request_id, std::string(),
+      render_process_id, render_view_id, page_request_id,
       security_origin, MEDIA_DEVICE_ACCESS, std::string(), std::string(),
       options.audio_type, options.video_type);
   DeviceRequest* request = new DeviceRequest(NULL, stream_request,
@@ -215,9 +215,10 @@ std::string MediaStreamManager::MakeMediaAccessRequest(
   const std::string& label = AddRequest(request);
 
   request->callback = callback;
-
-  HandleRequest(label);
-
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaStreamManager::HandleRequest,
+                 base::Unretained(this), label));
   return label;
 }
 
@@ -238,91 +239,45 @@ std::string MediaStreamManager::GenerateStream(
     UseFakeUI(scoped_ptr<FakeMediaStreamUIProxy>());
   }
 
-  int target_render_process_id = render_process_id;
-  int target_render_view_id = render_view_id;
-  std::string tab_capture_device_id;
-
-  // Customize options for a WebContents based capture.
-  if (options.audio_type == MEDIA_TAB_AUDIO_CAPTURE ||
-      options.video_type == MEDIA_TAB_VIDEO_CAPTURE) {
-    // TODO(justinlin): Can't plumb audio mirroring using stream type right
-    // now, so plumbing by device_id. Will revisit once it's refactored.
-    // http://crbug.com/163100
-    tab_capture_device_id =
-        WebContentsCaptureUtil::AppendWebContentsDeviceScheme(
-            !options.video_device_id.empty() ?
-            options.video_device_id : options.audio_device_id);
-
-    bool has_valid_device_id = WebContentsCaptureUtil::ExtractTabCaptureTarget(
-        tab_capture_device_id, &target_render_process_id,
-        &target_render_view_id);
-    if (!has_valid_device_id ||
-        (options.audio_type != MEDIA_TAB_AUDIO_CAPTURE &&
-         options.audio_type != MEDIA_NO_SERVICE) ||
-        (options.video_type != MEDIA_TAB_VIDEO_CAPTURE &&
-         options.video_type != MEDIA_NO_SERVICE)) {
-      LOG(ERROR) << "Invalid request.";
-      return std::string();
-    }
-  }
-
-  std::string translated_audio_device_id;
-  std::string translated_video_device_id;
-  if (options.audio_type == MEDIA_DEVICE_AUDIO_CAPTURE) {
-    bool found_match = TranslateGUIDToRawId(
-        MEDIA_DEVICE_AUDIO_CAPTURE, security_origin, options.audio_device_id,
-        &translated_audio_device_id);
-    DCHECK(found_match || translated_audio_device_id.empty());
-  }
-
-  if (options.video_type == MEDIA_DEVICE_VIDEO_CAPTURE) {
-    bool found_match = TranslateGUIDToRawId(
-        MEDIA_DEVICE_VIDEO_CAPTURE, security_origin, options.video_device_id,
-        &translated_video_device_id);
-    DCHECK(found_match || translated_video_device_id.empty());
-  }
-
-  if (options.video_type == MEDIA_DESKTOP_VIDEO_CAPTURE ||
-      options.audio_type == MEDIA_LOOPBACK_AUDIO_CAPTURE) {
-    // For screen capture we only support two valid combinations:
-    // (1) screen video capture only, or
-    // (2) screen video capture with loopback audio capture.
-    if (options.video_type != MEDIA_DESKTOP_VIDEO_CAPTURE ||
-        (options.audio_type != MEDIA_NO_SERVICE &&
-         options.audio_type != MEDIA_LOOPBACK_AUDIO_CAPTURE)) {
-      // TODO(sergeyu): Surface error message to the calling JS code.
-      LOG(ERROR) << "Invalid screen capture request.";
-      return std::string();
-    }
-    translated_video_device_id = options.video_device_id;
-  }
-
   // Create a new request based on options.
   MediaStreamRequest stream_request(
-      target_render_process_id, target_render_view_id, page_request_id,
-      tab_capture_device_id, security_origin, MEDIA_GENERATE_STREAM,
-      translated_audio_device_id, translated_video_device_id,
+      render_process_id, render_view_id, page_request_id,
+      security_origin, MEDIA_GENERATE_STREAM,
+      options.audio_device_id, options.video_device_id,
       options.audio_type, options.video_type);
   DeviceRequest* request = new DeviceRequest(requester, stream_request,
                                              render_process_id,
                                              render_view_id);
   const std::string& label = AddRequest(request);
-  HandleRequest(label);
+
+  // Need to post a task since the requester won't have label till
+  // this function returns.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaStreamManager::HandleRequest,
+                 base::Unretained(this), label));
   return label;
 }
 
 void MediaStreamManager::CancelRequest(const std::string& label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaStreamManager::DoCancelRequest,
+                 base::Unretained(this), label));
+}
+
+void MediaStreamManager::DoCancelRequest(const std::string& label) {
   DeviceRequests::iterator request_it = requests_.find(label);
   if (request_it == requests_.end()) {
     NOTREACHED();
     return;
   }
-  scoped_ptr<DeviceRequest> request(request_it->second);
-  RemoveRequest(request_it);
 
+  DeviceRequest* request(request_it->second);
   if (request->request.request_type == MEDIA_ENUMERATE_DEVICES) {
+    DeleteRequest(request_it);
     return;
   }
 
@@ -341,6 +296,7 @@ void MediaStreamManager::CancelRequest(const std::string& label) {
 
   // Cancel the request if still pending at UI side.
   request->SetState(NUM_MEDIA_TYPES, MEDIA_REQUEST_STATE_CLOSING);
+  DeleteRequest(request_it);
 }
 
 void MediaStreamManager::CancelAllRequests(int render_process_id) {
@@ -353,7 +309,10 @@ void MediaStreamManager::CancelAllRequests(int render_process_id) {
 
     std::string label = request_it->first;
     ++request_it;
-    CancelRequest(label);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&MediaStreamManager::DoCancelRequest,
+                   base::Unretained(this), label));
   }
 }
 
@@ -363,7 +322,16 @@ void MediaStreamManager::StopStreamDevice(int render_process_id,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << "StopStreamDevice({render_view_id = " << render_view_id <<  "} "
            << ", {device_id = " << device_id << "})";
+  BrowserThread::PostTask(
+         BrowserThread::IO, FROM_HERE,
+         base::Bind(&MediaStreamManager::DoStopStreamDevice,
+                    base::Unretained(this), render_process_id, render_view_id,
+                    device_id));
+}
 
+void MediaStreamManager::DoStopStreamDevice(int render_process_id,
+                                            int render_view_id,
+                                            const std::string& device_id) {
   // Find all requests for this |render_process_id| and |render_view_id| of type
   // MEDIA_GENERATE_STREAM that has requested to use |device_id|.
   DeviceRequests::iterator request_it = requests_.begin();
@@ -395,8 +363,7 @@ void MediaStreamManager::StopStreamDevice(int render_process_id,
     if (devices->empty()) {
       DeviceRequests::iterator del_itor(request_it);
       ++request_it;
-      scoped_ptr<DeviceRequest> request(del_itor->second);
-      RemoveRequest(del_itor);
+      DeleteRequest(del_itor);
     } else {
       ++request_it;
     }
@@ -447,42 +414,51 @@ std::string MediaStreamManager::EnumerateDevices(
 
   // Create a new request.
   StreamOptions options;
-  EnumerationCache* cache = NULL;
   if (type == MEDIA_DEVICE_AUDIO_CAPTURE) {
     options.audio_type = type;
-    cache = &audio_enumeration_cache_;
   } else if (type == MEDIA_DEVICE_VIDEO_CAPTURE) {
     options.video_type = type;
-    cache = &video_enumeration_cache_;
   } else {
     NOTREACHED();
     return std::string();
   }
 
   MediaStreamRequest stream_request(
-      render_process_id, render_view_id, page_request_id, std::string(),
+      render_process_id, render_view_id, page_request_id,
       security_origin, MEDIA_ENUMERATE_DEVICES, std::string(), std::string(),
       options.audio_type, options.video_type);
   DeviceRequest* request = new DeviceRequest(requester, stream_request,
                                              render_process_id,
                                              render_view_id);
   const std::string& label = AddRequest(request);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaStreamManager::DoEnumerateDevices,
+                 base::Unretained(this), label));
+
+  return label;
+}
+
+void MediaStreamManager::DoEnumerateDevices(const std::string& label) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DeviceRequest* request = requests_[label];
+
+  const MediaStreamType type =
+      request->request.audio_type == MEDIA_DEVICE_AUDIO_CAPTURE ?

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

Looks like we're assuming that audio_type will either be set to 'audio capture'
or that if it isn't, then the only other option can be that video_capture is
MEDIA_DEVICE_VIDEO_CAPTURE.
Should we have a DCHECK for that?
E.g. something like:
DCHECK(type == MEDIA_DEVICE_AUDIO_CAPTURE ||
       request->request.video_type == MEDIA_DEVICE_VIDEO_CAPTURE);

[perkj_chrome, 2013/10/29 08:52:17]

Yes- you can apparently only enumerate one type at the time. I am not changing
this in this cl.

[tommi (sloooow) - chröme, 2013/10/29 17:21:16]

My concern is that there are several other possible values and we're checking
for one and if it's not the one we check for we assume that it must be another
one but don't actually check for it.  That's why I think that the DCHECK is a
good thing.

+          MEDIA_DEVICE_AUDIO_CAPTURE : MEDIA_DEVICE_VIDEO_CAPTURE;
+
+  EnumerationCache* cache =
+      request->request.audio_type == MEDIA_DEVICE_AUDIO_CAPTURE ?

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

nit: Could you check |type| here instead? (since you've already done this same
check on the line above)?

Alternatively you could do (and add a DCHECK at the same time):

MediaStreamType type;
EnumerationCache* cache;
if (request->request.audio_type == MEDIA_DEVICE_AUDIO_CAPTURE) {
  type = MEDIA_DEVICE_AUDIO_CAPTURE;
  cache = &audio_enumeration_cache_;
} else {
  DCHECK(request->request.video_type == MEDIA_DEVICE_VIDEO_CAPTURE);
  type = MEDIA_DEVICE_VIDEO_CAPTURE;
  cache = &video_enumeration_cache_;
}

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+          &audio_enumeration_cache_ : &video_enumeration_cache_;
 
   if (cache->valid) {
     // Cached device list of this type exists. Just send it out.
     request->SetState(type, MEDIA_REQUEST_STATE_REQUESTED);
-
-    // Need to post a task since the requester won't have label till
-    // this function returns.
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(&MediaStreamManager::SendCachedDeviceList,
-                   base::Unretained(this), cache, label));
+    request->devices = cache->devices;
+    FinalizeEnumerateDevices(label, request);
   } else {
     StartEnumeration(request);
   }
-
-  return label;
 }
 
 std::string MediaStreamManager::OpenDevice(
@@ -500,8 +476,8 @@ std::string MediaStreamManager::OpenDevice(
   // Create a new request.
   StreamOptions options;
   if (IsAudioMediaType(type)) {
-    options.audio_type = type;
-    options.audio_device_id = device_id;
+     options.audio_type = type;

[Jói, 2013/10/28 11:48:17]

This and next line are indented one space too much.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+     options.audio_device_id = device_id;
   } else if (IsVideoMediaType(type)) {
     options.video_type = type;
     options.video_device_id = device_id;
@@ -511,30 +487,21 @@ std::string MediaStreamManager::OpenDevice(
   }
 
   MediaStreamRequest stream_request(
-      render_process_id, render_view_id, page_request_id, std::string(),
+      render_process_id, render_view_id, page_request_id,
       security_origin, MEDIA_OPEN_DEVICE, options.audio_device_id,
       options.video_device_id, options.audio_type, options.video_type);
   DeviceRequest* request = new DeviceRequest(requester, stream_request,
                                              render_process_id,
                                              render_view_id);
   const std::string& label = AddRequest(request);
-  StartEnumeration(request);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaStreamManager::HandleRequest,
+                 base::Unretained(this), label));
 
   return label;
 }
 
-void MediaStreamManager::SendCachedDeviceList(
-    EnumerationCache* cache,
-    const std::string& label) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (cache->valid) {
-    DeviceRequests::iterator it = requests_.find(label);
-    if (it != requests_.end()) {
-      it->second->requester->DevicesEnumerated(label, cache->devices);
-    }
-  }
-}
-
 void MediaStreamManager::StartMonitoring() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!base::SystemMonitor::Get())
@@ -563,13 +530,48 @@ void MediaStreamManager::StopMonitoring() {
   }
 }
 
-bool MediaStreamManager::TranslateGUIDToRawId(MediaStreamType stream_type,
-                                              const GURL& security_origin,
-                                              const std::string& device_guid,
-                                              std::string* raw_device_id) {
+bool MediaStreamManager::TransLateRequestedSourceIdToDeviceId(

[Jói, 2013/10/28 11:48:17]

TransLate -> Translate

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+    MediaStreamRequest* request) {
+  // If a specific device has been requested we need to find the real device id.
+  if (request->audio_type == MEDIA_DEVICE_AUDIO_CAPTURE &&
+      !request->requested_audio_device_id.empty()) {
+    if (!TranslateSourceIdToDeviceId(MEDIA_DEVICE_AUDIO_CAPTURE,
+                                     request->security_origin,
+                                     request->requested_audio_device_id,
+                                     &request->requested_audio_device_id)) {
+      // TODO(perkj): gUM should support mandatory and optional constraints.

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

nit: case of gUM is inconsistent on the next line and in the copy/pasted comment
below.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+      // Ie - if the sourceId is optional but it does not match - guM should
+      // not fail. For now we treat sourceId as a mandatory constraint.
+      return false;
+    }
+  }
+
+  if (request->video_type == MEDIA_DEVICE_VIDEO_CAPTURE &&
+      !request->requested_video_device_id.empty()) {
+    if (!TranslateSourceIdToDeviceId(MEDIA_DEVICE_VIDEO_CAPTURE,
+                                     request->security_origin,
+                                     request->requested_video_device_id,
+                                     &request->requested_video_device_id)) {
+      // TODO(perkj): guM should support mandatory and optional constraints.
+      // Ie - if the sourceId is optional but it does not match - guM should
+      // not fail. For now we treat sourceId as a mandatory constraint.
+      return false;
+    }
+  }
+  DVLOG(1) << "Requested audio device " << request->requested_audio_device_id;
+  DVLOG(1) << "Requested video device " << request->requested_video_device_id;
+  return true;
+}
+
+
+bool MediaStreamManager::TranslateSourceIdToDeviceId(
+    MediaStreamType stream_type,
+    const GURL& security_origin,
+    const std::string& source_id,
+    std::string* device_id) {
   DCHECK(stream_type == MEDIA_DEVICE_AUDIO_CAPTURE ||
          stream_type == MEDIA_DEVICE_VIDEO_CAPTURE);
-  if (device_guid.empty())
+  if (source_id.empty())

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

should this perhaps be a DCHECK?  It seems like that's currently the case and
could help us catch issues early with future code.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

     return false;
 
   EnumerationCache* cache =
@@ -584,8 +586,8 @@ bool MediaStreamManager::TranslateGUIDToRawId(MediaStreamType stream_type,
        it != cache->devices.end();
        ++it) {
     if (content::DoesMediaDeviceIDMatchHMAC(
-            security_origin, device_guid, it->device.id)) {
-      *raw_device_id = it->device.id;
+        security_origin, source_id, it->device.id)) {

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

Looks like indentation was fine before.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+      *device_id = it->device.id;
       return true;
     }
   }
@@ -633,7 +635,8 @@ std::string MediaStreamManager::AddRequest(DeviceRequest* request) {
   return unique_label;
 }
 
-void MediaStreamManager::RemoveRequest(DeviceRequests::iterator it) {
+void MediaStreamManager::DeleteRequest(DeviceRequests::iterator it) {
+  scoped_ptr<DeviceRequest> request(it->second);

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

This is fine as is but I wonder if it would it make sense to change
DeviceRequests to be:

typedef std::map<std::string, scoped_ptr<DeviceRequest> > DeviceRequests;

and use scoped_ptr.Pass() instead of having the raw pointer type.

[perkj_chrome, 2013/10/29 08:52:17]

nope- does not work as discussed.

   requests_.erase(it);
 }
 
@@ -684,9 +687,17 @@ void MediaStreamManager::HandleRequest(const std::string& label) {
   bool is_web_contents_capture =
       audio_type == MEDIA_TAB_AUDIO_CAPTURE ||
       video_type == MEDIA_TAB_VIDEO_CAPTURE;
+  if (is_web_contents_capture && !SetupTabCaptureRequest(request)) {
+    FinalizeRequestFailed(label);
+    return;
+  }
 
   bool is_screen_capture =
       video_type == MEDIA_DESKTOP_VIDEO_CAPTURE;
+  if (is_screen_capture && !SetupScreenCaptureRequest(request)) {
+    FinalizeRequestFailed(label);
+    return;
+  }
 
   if (!is_web_contents_capture &&
       !is_screen_capture &&
@@ -697,6 +708,12 @@ void MediaStreamManager::HandleRequest(const std::string& label) {
     return;
   }
 
+  // If a specific device has been requested we need to find the real device id.
+  if (!TransLateRequestedSourceIdToDeviceId(&request->request)) {
+    FinalizeRequestFailed(label);
+    return;
+  }
+
   // No need to do new device enumerations, post the request to UI
   // immediately.
   if (IsAudioMediaType(audio_type))
@@ -707,15 +724,105 @@ void MediaStreamManager::HandleRequest(const std::string& label) {
   PostRequestToUI(label);
 }
 
+bool MediaStreamManager::SetupTabCaptureRequest(DeviceRequest* request) {
+  DCHECK(request->request.audio_type == MEDIA_TAB_AUDIO_CAPTURE ||
+         request->request.video_type == MEDIA_TAB_VIDEO_CAPTURE);
+
+  MediaStreamRequest* ms_request = &request->request;
+  // Customize options for a WebContents based capture.
+  int target_render_process_id = 0;
+  int target_render_view_id = 0;
+
+  // TODO(justinlin): Can't plumb audio mirroring using stream type right
+  // now, so plumbing by device_id. Will revisit once it's refactored.
+  // http://crbug.com/163100
+  std::string tab_capture_device_id =
+        WebContentsCaptureUtil::AppendWebContentsDeviceScheme(
+            !ms_request->requested_video_device_id.empty() ?
+             ms_request->requested_video_device_id :
+             ms_request->requested_audio_device_id);
+
+  bool has_valid_device_id = WebContentsCaptureUtil::ExtractTabCaptureTarget(
+      tab_capture_device_id, &target_render_process_id,
+      &target_render_view_id);
+  if (!has_valid_device_id ||
+      (ms_request->audio_type != MEDIA_TAB_AUDIO_CAPTURE &&
+       ms_request->audio_type != MEDIA_NO_SERVICE) ||
+      (ms_request->video_type != MEDIA_TAB_VIDEO_CAPTURE &&
+       ms_request->video_type != MEDIA_NO_SERVICE)) {
+    return false;
+  }
+  ms_request->tab_capture_device_id = tab_capture_device_id;
+  ms_request->render_process_id = target_render_process_id;
+  ms_request->render_view_id = target_render_view_id;
+  DVLOG(3) << "SetupTabCaptureRequest "
+           << ", {tab_capture_device_id = " << tab_capture_device_id <<  "}"
+           << ", {target_render_process_id = " << target_render_process_id
+           << "}"
+           << ", {target_render_view_id = " << target_render_view_id << "}";
+  return true;
+}
+
+bool MediaStreamManager::SetupScreenCaptureRequest(DeviceRequest* request) {
+  DCHECK(request->request.audio_type == MEDIA_DESKTOP_VIDEO_CAPTURE ||
+         request->request.video_type == MEDIA_LOOPBACK_AUDIO_CAPTURE);
+  const MediaStreamRequest& ms_request = request->request;
+
+  // For screen capture we only support two valid combinations:
+  // (1) screen video capture only, or
+  // (2) screen video capture with loopback audio capture.
+  if (ms_request.video_type != MEDIA_DESKTOP_VIDEO_CAPTURE ||
+      (ms_request.audio_type != MEDIA_NO_SERVICE &&
+       ms_request.audio_type != MEDIA_LOOPBACK_AUDIO_CAPTURE)) {
+    // TODO(sergeyu): Surface error message to the calling JS code.
+    LOG(ERROR) << "Invalid screen capture request.";
+    return false;
+  }
+  return true;
+}
+
+bool MediaStreamManager::FindRequestedDeviceInfo(
+    const std::string& source_id,
+    int render_process_id,
+    int render_view_id,
+    MediaStreamRequestType type,
+    StreamDeviceInfo* device_info) {
+  for (DeviceRequests::const_iterator it = requests_.begin();
+       it != requests_.end() ; ++it) {
+    const DeviceRequest* request = it->second;
+    if (request->request.render_process_id ==render_process_id &&

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

spaces

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+        request->request.render_view_id == render_view_id &&
+        request->request.request_type == type) {
+      for (StreamDeviceInfoArray::const_iterator device_it =
+               request->devices.begin();
+           device_it != request->devices.end(); ++device_it) {
+        if (source_id == device_it->device.id) {
+          if (device_info)
+            *device_info = *device_it;

[no longer working on chromium, 2013/10/28 09:13:29]

Is allowed to have device_info to be NULL? If not, consider DCHECK it?

also, can you use device_info->push_back(*device_it) instead of copying?

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

If it's not allowed to be NULL, no need to DCHECK it, just assign to it :)
device_info isn't an STL container.  However, perhaps there's a way to return a
pointer to a const StreamDeviceInfo* assuming usage stays on this thread and the
pointer isn't cached?  NULL could be returned in the case of not found.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+          return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
 bool MediaStreamManager::FindExistingRequestedDeviceInfo(

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

return const MediaStreamType*?

[perkj_chrome, 2013/10/29 08:52:17]

This function returns two things- device_info and request_state.

     int render_process_id,
     int render_view_id,
+    GURL security_origin,

[no longer working on chromium, 2013/10/28 09:13:29]

nit, const GURL& security_origin,

[perkj_chrome, 2013/10/29 08:52:17]

Done.

     MediaStreamRequestType type,
     const std::string& device_id,
+    MediaStreamType device_type,
     StreamDeviceInfo* device_info,
     MediaRequestState* request_state) const {
   DCHECK(device_info);
   DCHECK(request_state);
+
+  std::string source_id = content::GetHMACForMediaDeviceID(
+      security_origin,
+      device_id);
+
   for (DeviceRequests::const_iterator it = requests_.begin();
        it != requests_.end() ; ++it) {
     const DeviceRequest* request = it->second;
@@ -725,7 +832,8 @@ bool MediaStreamManager::FindExistingRequestedDeviceInfo(
       for (StreamDeviceInfoArray::const_iterator device_it =
                request->devices.begin();
            device_it != request->devices.end(); ++device_it) {
-        if (device_it->device.id == device_id) {
+        if (device_it->device.id == source_id &&
+            device_it->device.type == device_type) {
             *device_info = *device_it;
             *request_state = request->state(device_it->device.type);
           return true;
@@ -736,6 +844,76 @@ bool MediaStreamManager::FindExistingRequestedDeviceInfo(
   return false;
 }
 
+void MediaStreamManager::FinalizeGenerateStream(const std::string& label,
+                                                DeviceRequest* request) {
+  const StreamDeviceInfoArray& requested_devices = request->devices;
+
+  // Partition the array of devices into audio vs video.
+  StreamDeviceInfoArray audio_devices, video_devices;
+  for (StreamDeviceInfoArray::const_iterator device_it =
+      requested_devices.begin();

[tommi (sloooow) - chröme, 2013/10/28 13:48:23]

fix indentation

[perkj_chrome, 2013/10/29 08:52:17]

Done.

+       device_it != requested_devices.end(); ++device_it) {
+    if (IsAudioMediaType(device_it->device.type)) {
+      audio_devices.push_back(*device_it);
+    } else if (IsVideoMediaType(device_it->device.type)) {
+      video_devices.push_back(*device_it);
+    } else {
+      NOTREACHED();
+    }
+  }
+
+  request->requester->StreamGenerated(label, audio_devices, video_devices);
+}
+
+void MediaStreamManager::FinalizeRequestFailed(
+    const std::string& label) {
+  DeviceRequests::iterator request_it = requests_.find(label);
+  if (request_it == requests_.end())
+    return;
+
+  DeviceRequest* request(request_it->second);
+  if (request->requester)
+    request->requester->StreamGenerationFailed(label);
+
+  if (request->request.request_type == MEDIA_DEVICE_ACCESS &&
+      !request->callback.is_null()) {
+    request->callback.Run(MediaStreamDevices(), request->ui_proxy.Pass());
+  }
+
+  DeleteRequest(request_it);
+}
+
+void MediaStreamManager::FinalizeOpenDevice(const std::string& label,
+                                            DeviceRequest* request) {
+  const StreamDeviceInfoArray& requested_devices = request->devices;
+  request->requester->DeviceOpened(label, requested_devices.front());
+}
+
+void MediaStreamManager::FinalizeEnumerateDevices(const std::string& label,
+                                                  DeviceRequest* request) {
+  for (StreamDeviceInfoArray::iterator it = request->devices.begin();
+       it != request->devices.end(); ++it) {
+    it->device.id = content::GetHMACForMediaDeviceID(
+        request->request.security_origin, it->device.id);
+  }
+  request->requester->DevicesEnumerated(label, request->devices);
+}
+
+void MediaStreamManager::FinalizeMediaAccessRequest(
+    const std::string& label,
+    const MediaStreamDevices& devices) {
+  DeviceRequests::iterator request_it = requests_.find(label);
+  if (request_it == requests_.end())
+    return;
+  DeviceRequest* request(request_it->second);

[no longer working on chromium, 2013/10/28 09:13:29]

nit, move this declaration below the empty line.

[perkj_chrome, 2013/10/29 08:52:17]

rewritten a bit now.

+
+  if (!request->callback.is_null())
+    request->callback.Run(devices, request->ui_proxy.Pass());
+
+  // Delete the request since it is done.
+  DeleteRequest(request_it);
+}
+
 void MediaStreamManager::InitializeDeviceManagersOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (device_thread_)
@@ -790,7 +968,6 @@ void MediaStreamManager::Opened(MediaStreamType stream_type,
             const StreamDeviceInfo* info =
                 audio_input_device_manager_->GetOpenedDeviceInfoById(
                     device_it->session_id);
-            DCHECK_EQ(info->device.id, device_it->device.id);
             device_it->device.input = info->device.input;
             device_it->device.matched_output = info->device.matched_output;
           }
@@ -809,27 +986,12 @@ void MediaStreamManager::HandleRequestDone(const std::string& label,
   DVLOG(1) << "HandleRequestDone("
            << ", {label = " << label <<  "})";
 
-  const StreamDeviceInfoArray& requested_devices = request->devices;
   switch (request->request.request_type) {
     case MEDIA_OPEN_DEVICE:
-      request->requester->DeviceOpened(label, requested_devices.front());
+      FinalizeOpenDevice(label, request);
       break;
     case MEDIA_GENERATE_STREAM: {
-      // Partition the array of devices into audio vs video.
-      StreamDeviceInfoArray audio_devices, video_devices;
-      for (StreamDeviceInfoArray::const_iterator device_it =
-               requested_devices.begin();
-           device_it != requested_devices.end(); ++device_it) {
-        if (IsAudioMediaType(device_it->device.type)) {
-          audio_devices.push_back(*device_it);
-        } else if (IsVideoMediaType(device_it->device.type)) {
-          video_devices.push_back(*device_it);
-        } else {
-          NOTREACHED();
-        }
-      }
-
-      request->requester->StreamGenerated(label, audio_devices, video_devices);
+      FinalizeGenerateStream(label, request);
       break;
     }
     default:
@@ -889,8 +1051,10 @@ void MediaStreamManager::DevicesEnumerated(
     DeviceRequest* request = requests_[*it];
     switch (request->request.request_type) {
       case MEDIA_ENUMERATE_DEVICES:
-        if (need_update_clients && request->requester)
-          request->requester->DevicesEnumerated(*it, devices);
+        if (need_update_clients && request->requester) {
+          request->devices = devices;
+          FinalizeEnumerateDevices(*it, request);
+        }
         break;
       default:
         if (request->state(request->request.audio_type) ==
@@ -903,8 +1067,8 @@ void MediaStreamManager::DevicesEnumerated(
           break;
         }
 
-        // Post the request to UI for permission approval.
-        PostRequestToUI(*it);
+        // Continue to handle the request.
+        HandleRequest(*it);
         break;
     }
   }
@@ -913,69 +1077,6 @@ void MediaStreamManager::DevicesEnumerated(
   DCHECK_GE(active_enumeration_ref_count_[stream_type], 0);
 }
 
-void MediaStreamManager::Error(MediaStreamType stream_type,
-                               int capture_session_id,
-                               MediaStreamProviderError error) {
-  // Find the device for the error call.
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DVLOG(1) << "Error("
-           << "{stream_type = " << stream_type << "} ,"
-           << "{capture_session_id = " << capture_session_id <<  "})";
-
-
-  for (DeviceRequests::iterator it = requests_.begin(); it != requests_.end();
-       ++it) {
-    StreamDeviceInfoArray& devices = it->second->devices;
-
-    // TODO(miu): BUG.  It's possible for the audio (or video) device array in
-    // the "requester" to become out-of-sync with the order of devices we have
-    // here.  See http://crbug.com/147650
-    int audio_device_idx = -1;
-    int video_device_idx = -1;
-    for (StreamDeviceInfoArray::iterator device_it = devices.begin();
-         device_it != devices.end(); ++device_it) {
-      if (IsAudioMediaType(device_it->device.type)) {
-        ++audio_device_idx;
-      } else if (IsVideoMediaType(device_it->device.type)) {
-        ++video_device_idx;
-      } else {
-        NOTREACHED();
-        continue;
-      }
-      if (device_it->device.type != stream_type ||
-          device_it->session_id != capture_session_id) {
-        continue;
-      }
-      // We've found the failing device. Find the error case:
-      // An error should only be reported to the MediaStreamManager if
-      // the request has not been fulfilled yet.
-      DCHECK(it->second->state(stream_type) != MEDIA_REQUEST_STATE_DONE);
-      if (it->second->state(stream_type) != MEDIA_REQUEST_STATE_DONE) {
-        // Request is not done, devices are not opened in this case.
-        if (devices.size() <= 1) {
-          scoped_ptr<DeviceRequest> request(it->second);
-          // 1. Device not opened and no other devices for this request ->
-          //    signal stream error and remove the request.
-          if (request->requester)
-            request->requester->StreamGenerationFailed(it->first);
-
-          RemoveRequest(it);
-        } else {
-          // 2. Not opened but other devices exists for this request -> remove
-          //    device from list, but don't signal an error.
-          devices.erase(device_it);  // NOTE: This invalidates device_it!
-          it->second->SetState(stream_type, MEDIA_REQUEST_STATE_ERROR);
-          DVLOG(1) << "Error("
-                   << ", {capture_session_id = " << capture_session_id <<  "})";
-        }
-      }
-      if (RequestDone(*it->second))
-        HandleRequestDone(it->first, it->second);
-      break;
-    }
-  }
-}
-
 void MediaStreamManager::HandleAccessRequestResponse(
     const std::string& label,
     const MediaStreamDevices& devices) {
@@ -988,34 +1089,19 @@ void MediaStreamManager::HandleAccessRequestResponse(
     return;
   }
 
-  // Handle the case when the request was denied.
-  if (devices.empty()) {
-    // Notify the users about the request result.
-    scoped_ptr<DeviceRequest> request(request_it->second);
-    if (request->requester)
-      request->requester->StreamGenerationFailed(label);
-
-    if (request->request.request_type == MEDIA_DEVICE_ACCESS &&
-        !request->callback.is_null()) {
-      request->callback.Run(MediaStreamDevices(), request->ui_proxy.Pass());
-    }
-
-    RemoveRequest(request_it);
+  DeviceRequest* request = request_it->second;
+  if (request->request.request_type == MEDIA_DEVICE_ACCESS) {
+    FinalizeMediaAccessRequest(label, devices);
     return;
   }
 
-  if (request_it->second->request.request_type == MEDIA_DEVICE_ACCESS) {
-    scoped_ptr<DeviceRequest> request(request_it->second);
-    if (!request->callback.is_null())
-      request->callback.Run(devices, request->ui_proxy.Pass());
-
-    // Delete the request since it is done.
-    RemoveRequest(request_it);
+  // Handle the case when the request was denied.
+  if (devices.empty()) {
+    FinalizeRequestFailed(label);
     return;
   }
 
   // Process all newly-accepted devices for this request.
-  DeviceRequest* request = request_it->second;
   bool found_audio = false;
   bool found_video = false;
   for (MediaStreamDevices::const_iterator device_it = devices.begin();
@@ -1056,13 +1142,14 @@ void MediaStreamManager::HandleAccessRequestResponse(
     // per render view. This is so that the permission to use a device can be
     // revoked by a single call to StopStreamDevice regardless of how many
     // MediaStreams it is being used in.
-
     if (request->request.request_type == MEDIA_GENERATE_STREAM) {
       MediaRequestState state;
       if (FindExistingRequestedDeviceInfo(request->requesting_process_id,
                                           request->requesting_view_id,
+                                          request->request.security_origin,
                                           request->request.request_type,
-                                          device_it->id,
+                                          device_info.device.id,
+                                          device_info.device.type,
                                           &device_info,
                                           &state)) {
         request->devices.push_back(device_info);
@@ -1075,11 +1162,15 @@ void MediaStreamManager::HandleAccessRequestResponse(
     }
     device_info.session_id =
         GetDeviceManager(device_info.device.type)->Open(device_info);
+    device_info.device.id = content::GetHMACForMediaDeviceID(
+        request->request.security_origin,
+        device_info.device.id);
     request->devices.push_back(device_info);
+
     request->SetState(device_info.device.type, MEDIA_REQUEST_STATE_OPENING);
     DVLOG(1) << "HandleAccessRequestResponse - opening device "
              << ", {label = " << label <<  "}"
-             << ", {device_id = " << device_it->id << "}"
+             << ", {device_id = " << device_info.device.id << "}"
              << ", {session_id = " << device_info.session_id << "}";
   }
 
@@ -1125,6 +1216,7 @@ void MediaStreamManager::UseFakeUI(scoped_ptr<FakeMediaStreamUIProxy> fake_ui) {
 }
 
 void MediaStreamManager::WillDestroyCurrentMessageLoop() {
+  DVLOG(3) << "MediaStreamManager::WillDestroyCurrentMessageLoop()";
   DCHECK_EQ(base::MessageLoop::current(), io_loop_);
   DCHECK(requests_.empty());
   if (device_thread_) {