Close video capture devices on the device thread.

Close video capture devices on the device thread.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110780

[mflodman_chromium_OOO, 2011-11-17 09:20:37]

Wei,
Can you have a look at this change?

[wjia(left Chromium), 2011-11-17 19:06:30]

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
content/browser/renderer_host/media/video_capture_manager.cc:61: new
MessageLoop::QuitTask);
QuitTask is not needed. vc_device_thread_.Stop() will do that.

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
content/browser/renderer_host/media/video_capture_manager.cc:513: delete
it->second;
There was a bug here since multiple capture_session_id can be mapped to one
video_device. device can't be deleted here. 

There is a fix at
http://codereview.chromium.org/8480028/diff/12001/content/browser/renderer_ho....

It would be great if you can integrate that change.

[scherkus (not reviewing), 2011-11-17 22:33:22]

owners lgtm

[mflodman_chromium_OOO, 2011-11-18 08:22:44]

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
content/browser/renderer_host/media/video_capture_manager.cc:61: new
MessageLoop::QuitTask);
On 2011/11/17 19:06:30, wjia wrote:
> QuitTask is not needed. vc_device_thread_.Stop() will do that.

Done.

http://codereview.chromium.org/8589018/diff/1/content/browser/renderer_host/m...
content/browser/renderer_host/media/video_capture_manager.cc:513: delete
it->second;
On 2011/11/17 19:06:30, wjia wrote:
> There was a bug here since multiple capture_session_id can be mapped to one
> video_device. device can't be deleted here. 
> 
> There is a fix at
>
http://codereview.chromium.org/8480028/diff/12001/content/browser/renderer_ho....
> 
> It would be great if you can integrate that change.

Done.

[wjia(left Chromium), 2011-11-18 14:40:35]

lgtm

[commit-bot: I haz the power, 2011-11-18 21:45:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mflodman@chromium.org/8589018/5001

[commit-bot: I haz the power, 2011-11-18 23:13:58]

Change committed as 110780