New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 648183 link

Starred by 5 users
Status: Fixed
Owner:
guidou@chromium.org
Closed: Dec 2016
Cc:
█ tibell@chromium.org
benwells@chromium.org
webrtc-testing@google.com
niklase@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 657733



Sign in to add a comment

Move device enumeration and notifications from MediaStreamDispatcherHost to a Mojo-based MediaDevicesDispatcherHost.

Project Member Reported by guidou@chromium.org, Sep 19 2016 
MediaStreamDispatcherHost currently handles renderer requests for device enumerations, device-change notifications and getUserMedia, and basically forwards al the work to MediaStreamManager.

As part of a refactoring towards implementing features such as MediaStreamTrack.applyConstraints, it has become necessary to have dedicated interfaces for device information.

The proposed way to achieve this is to provide a Mojo-based interface for enumerations, device-changes and other device-info features such as capabilities; and leave MediaStreamDispatcherHost for requests that actually access the devices such as getUserMedia.

This also includes removing the related code from MediaStreamManager.

Comment 1 by guidou@chromium.org, Sep 19 2016

Components: Blink>WebRTC
Owner: guidou@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by guidou@chromium.org, Sep 19 2016 

Part of this work also includes eliminating the concept of "open" enumeration requests that support both in request/reply and publish/subscribe modes simultaneously.

Clients on the renderer side (i.e., Pepper) can just issue both a regular request and a subscription for notifications.

Comment 3 by guidou@chromium.org, Sep 28 2016

Summary: Move device enumeration and notifications from MediaStreamDispatcherHost to a Mojo-based MediaDevicesDispatcherHost. (was: Move device enumeration and notifications from MediaStreamDispatcherHost to a MediaDevicesDispatcherHost.)

Comment 4 by mcasas@chromium.org, Oct 3 2016

Components: Blink>MediaStream
Labels: OS-All
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 11 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9357cc53ac0d187d3020a8dab9596f1e67db5abe

commit 9357cc53ac0d187d3020a8dab9596f1e67db5abe
Author: guidou <guidou@chromium.org>
Date: Tue Oct 11 19:53:36 2016

Migrate MediaDevices.enumerateDevices to Mojo

Note that Mojo is not called directly from Blink because device enumerations
must also be called by content::PepperMediaDeviceManager.
The Pepper work will come in a follow-up CL.

BUG= 648183 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2380793002
Cr-Commit-Position: refs/heads/master@{#424517}

[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/BUILD.gn
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/bad_message.h
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/frame_host/render_frame_host_impl.cc
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/BUILD.gn
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/OWNERS
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/media_devices.h
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/media_devices.mojom
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/media_devices.typemap
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/media_devices_param_traits.cc
[add] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/media/media_devices_param_traits.h
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/common/typemaps.gni
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/renderer/media/mock_media_stream_dispatcher.cc
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/content/test/BUILD.gn
[modify] https://crrev.com/9357cc53ac0d187d3020a8dab9596f1e67db5abe/tools/metrics/histograms/histograms.xml

Comment 6 by guidou@chromium.org, Oct 20 2016

Blocking: 657733
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 31 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df036b3c1846ea332f5196edd6f578f802cb8ef7

commit df036b3c1846ea332f5196edd6f578f802cb8ef7
Author: guidou <guidou@chromium.org>
Date: Mon Oct 31 14:51:14 2016

Introduce MediaDevicesPermissionChecker.

This class makes it easier to check device permissions.
It is similar to MediaStreamUIProxy, but it has the following advantages:
* Easier to check permissions for multiple device types in a single call.
* Uses MediaDeviceType instead of MediaStreamType to refer to device types.
* Simpler design.

Current users are MediaDevicesDispatcherHost and AudioRendererHost.
This will make it easier to check for permissions in the mojo version of
devicechange notifications.

Once MediaDevice permissions are revamped, maybe this class might substitute
MediaStreamUIProxy.

Originally, the intention was to use PermissionManager, but several tests expect
permissions to be checked via the RenderFrameHostDelegate, so this will have
to wait.

Also, the plan was to place this in content/browser/renderer_host/media since
it is used only by *Host classes, but it is illegal to include
"content/browser/frame_host" from there (MediaStreamUIProxy got a free pass).

BUG= 648183 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2436113002
Cr-Commit-Position: refs/heads/master@{#428709}

[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/BUILD.gn
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/frame_host/render_frame_host_impl.cc
[add] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/media/media_devices_permission_checker.cc
[add] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/media/media_devices_permission_checker.h
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[modify] https://crrev.com/df036b3c1846ea332f5196edd6f578f802cb8ef7/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57409acb300fa06da3679b20d3474436fadb68c5

commit 57409acb300fa06da3679b20d3474436fadb68c5
Author: guidou <guidou@chromium.org>
Date: Mon Nov 07 17:39:31 2016

Add mojo-based support for media device-change notifications.

Users of device-change notifications are not yet using this.
An  upcoming CL will include wiring of the Blink devicechange event and PepperMediaDeviceManager to this and removal of the corresponding code in MediaStreamDispatcher and MediaStreamDispatcherHost.

BUG= 648183 

Review-Url: https://codereview.chromium.org/2471543003
Cr-Commit-Position: refs/heads/master@{#430310}

[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/bad_message.h
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_manager.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_manager.h
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/browser/renderer_host/media/media_devices_manager_unittest.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/common/media/media_devices.mojom
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/public/app/mojo/content_renderer_manifest.json
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/BUILD.gn
[add] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/media_devices_event_dispatcher.cc
[add] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/media_devices_event_dispatcher.h
[add] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/media_devices_event_dispatcher_unittest.cc
[add] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/media_devices_listener_impl.cc
[add] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/media_devices_listener_impl.h
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/content/test/BUILD.gn
[modify] https://crrev.com/57409acb300fa06da3679b20d3474436fadb68c5/tools/metrics/histograms/histograms.xml
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9a27e0a82856485f39a73863431211475bf8789

commit d9a27e0a82856485f39a73863431211475bf8789
Author: guidou <guidou@chromium.org>
Date: Wed Nov 09 09:12:14 2016

Revert of Use mojo support for Blink devicechange event. (patchset #3 id:120001 of https://codereview.chromium.org/2476153002/ )

Reason for revert:
Reverting due to this CL due to it blocking the revert of another CL that is interfering with perf tests. Will reland after that.

See  crbug.com/662414 

Original issue's description:
> Use mojo support for Blink devicechange event.
>
> BUG= 648183 
>
> Committed: https://crrev.com/6a0339c962198b75c333eb14bafa6374890d738a
> Cr-Commit-Position: refs/heads/master@{#430318}

TBR=hbos@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 648183 

Review-Url: https://codereview.chromium.org/2483413002
Cr-Commit-Position: refs/heads/master@{#430899}

[modify] https://crrev.com/d9a27e0a82856485f39a73863431211475bf8789/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/d9a27e0a82856485f39a73863431211475bf8789/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/d9a27e0a82856485f39a73863431211475bf8789/content/renderer/media/user_media_client_impl_unittest.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/662a6038482e0b7bb1d3893491cdcbac33a807f2

commit 662a6038482e0b7bb1d3893491cdcbac33a807f2
Author: guidou <guidou@chromium.org>
Date: Wed Nov 09 09:19:42 2016

Reland of Use mojo support for Blink devicechange event. (patchset #1 id:1 of https://codereview.chromium.org/2483413002/ )

Reason for revert:
Relanding since the relanding of MediaStreamTrack.getSources (the ermoval of which is interfering with perf tests) will have to be manual anyway.

Original issue's description:
> Revert of Use mojo support for Blink devicechange event. (patchset #3 id:120001 of https://codereview.chromium.org/2476153002/ )
>
> Reason for revert:
> Reverting due to this CL due to it blocking the revert of another CL that is interfering with perf tests. Will reland after that.
>
> See  crbug.com/662414 
>
> Original issue's description:
> > Use mojo support for Blink devicechange event.
> >
> > BUG= 648183 
> >
> > Committed: https://crrev.com/6a0339c962198b75c333eb14bafa6374890d738a
> > Cr-Commit-Position: refs/heads/master@{#430318}
>
> TBR=hbos@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 648183 
>
> Committed: https://crrev.com/d9a27e0a82856485f39a73863431211475bf8789
> Cr-Commit-Position: refs/heads/master@{#430899}

TBR=hbos@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 648183 

Review-Url: https://codereview.chromium.org/2488843003
Cr-Commit-Position: refs/heads/master@{#430901}

[modify] https://crrev.com/662a6038482e0b7bb1d3893491cdcbac33a807f2/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/662a6038482e0b7bb1d3893491cdcbac33a807f2/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/662a6038482e0b7bb1d3893491cdcbac33a807f2/content/renderer/media/user_media_client_impl_unittest.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85000e1ea2df05933a6f85c311f6a68f996e78a4

commit 85000e1ea2df05933a6f85c311f6a68f996e78a4
Author: guidou <guidou@chromium.org>
Date: Tue Nov 15 17:28:55 2016

Remove dead code related to media device enumerations and monitoring.

This is the final CL in the migration to Mojo for this functionality.

BUG= 648183 

Make Pepper use Mojo-based support for media-device enumeration and monitoring.

BUG= 648183 

Review-Url: https://codereview.chromium.org/2487133003
Cr-Commit-Position: refs/heads/master@{#432208}

[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/chrome/browser/media/webrtc/media_stream_devices_controller.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_devices_manager.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_dispatcher_host.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_manager.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_manager_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/media_stream_requester.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/browser/renderer_host/media/video_capture_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/common/media/media_stream_messages.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/common/media/media_stream_options.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/common/media/media_stream_options.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/public/common/media_stream_request.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/public/common/media_stream_request.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/media_stream_dispatcher.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/media_stream_dispatcher.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/media_stream_dispatcher_eventhandler.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/media_stream_dispatcher_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/mock_media_stream_dispatcher.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/mock_media_stream_dispatcher.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/mock_media_stream_registry.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/rtc_peer_connection_handler_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/speech_recognition_audio_sink_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/webrtc/processed_local_audio_source_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/pepper/pepper_media_device_manager.cc
[modify] https://crrev.com/85000e1ea2df05933a6f85c311f6a68f996e78a4/content/renderer/pepper/pepper_media_device_manager.h

Comment 15 by anatolid@chromium.org, Nov 15 2016 

Is this finished? Also, please set correct milestone.
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc72b20c8530beda5a17f30678f071a14565cce0

commit dc72b20c8530beda5a17f30678f071a14565cce0
Author: guidou <guidou@chromium.org>
Date: Mon Nov 28 09:34:48 2016

Remove old media-device enumeration IPC message

We forgot to remove this message in a previous CL.

BUG= 648183 

Review-Url: https://codereview.chromium.org/2532473003
Cr-Commit-Position: refs/heads/master@{#434625}

[modify] https://crrev.com/dc72b20c8530beda5a17f30678f071a14565cce0/content/common/media/media_stream_messages.h

Comment 17 by guidou@chromium.org, Dec 1 2016

Status: Fixed (was: Assigned)

Comment 18 by anatolid@chromium.org, Jan 12 2017

Labels: M-57

Comment 19 by benwells@chromium.org, Mar 23 2017

Cc: benwells@chromium.org tibell@chromium.org
Thanks for converting your thing to Mojo!

Is someone looking after the rest of the media_stream_messages file?

Comment 20 by guidou@chromium.org, Mar 23 2017 

Yes, it will be a subtask of issue 704136.
►

Sign in to add a comment