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

Issue 764680 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 787254



Sign in to add a comment

Migrate MediaDevices.enumerateDevices() processing from content to Blink

Project Member Reported by guidou@chromium.org, Sep 13 2017

Issue description

enumerateDevices() is currently implemented by UserMediaClientImpl and basically depends on MediaDevicesDispatcherHost, which is already mojified.

Migrating this to Blink should be relatively straightforward.
First, it would be necessary to make MediaDevicesDispatcherHost accessible from Blink, which requires migrating some types from being mapped content types to full Mojo types.

Once this is done, implementing enumerateDevices should be as simple as just calling MediaDevicesDispatcherHost::EnumerateDevices directly from Blink.

 

Comment 1 by guidou@chromium.org, Sep 13 2017

Description: Show this description

Comment 2 by guidou@chromium.org, Sep 13 2017

Labels: -Type-Bug Type-Task
c.padhi@: This task seems to align well with your other contributions. Would you be interested in working on it?

Comment 3 by c.pa...@samsung.com, Sep 13 2017

Sure. I can take this up. Before I proceed, I would like to skim through the related code.
Also, if you could point out the code that refers to "some types from being mapped content types to full Mojo types"?

Comment 4 by guidou@chromium.org, Sep 14 2017

media_devices.mojom uses some types mapped from content/media types.
enumerateDevices() in particular uses content::MediaDeviceInfo, which cannot be used in Mojo. 
It would be good to check with Mojo people on what's the right approach for this.

Comment 5 by guidou@chromium.org, Sep 14 2017

In #4 I meant that content::MediaDeviceInfo cannot be used in Blink.

Comment 6 by guidou@chromium.org, Sep 14 2017

Cc: -c.pa...@samsung.com guidou@chromium.org
Owner: c.pa...@samsung.com

Comment 7 by c.pa...@samsung.com, Nov 24 2017

Hi Guido,

Now that MediaStream IPC migration is done, I would like to proceed with this task.

At the first look, I think we can setup a mojo connection with browser by creating a MediaDevicesDispatcherHost(MDDH) service in MediaDevices(MediaDevices.cpp) and call MDDH::EnumerateDevices directly.

Since the number of fields in the deviceinfo struct is less, we can manually do the conversion between content types and mojo types for now. Later, we can add the required typemapping using StructTraits.

WDYT? Please correct me if I am wrong.

Comment 8 by guidou@chromium.org, Nov 24 2017

c.padhi@: Sounds good to me.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30 2017

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

commit b1704afc626122a6a8adbafc03f67043fe114888
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Thu Nov 30 18:57:41 2017

Move media_devices.mojom to //WebKit/public

media_devices.mojom is moved from //content to //WebKit/public to allow
MediaDevicesDispatcherHost mojom interface's access from Blink.

This CL is the first step towards migration of MediaDevices.enumerateDevices()
processing from content to Blink.

Bug:  764680 
Change-Id: I2abdf63b7601cc24aa74434384cd0b7d28d69656
Reviewed-on: https://chromium-review.googlesource.com/790119
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#520625}
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/browser/BUILD.gn
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/browser/DEPS
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/common/BUILD.gn
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/common/media/media_devices.typemap
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/BUILD.gn
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/apply_constraints_processor.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/apply_constraints_processor.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_devices_event_dispatcher.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_devices_event_dispatcher.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_devices_event_dispatcher_unittest.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_devices_listener_impl.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_devices_listener_impl.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_audio.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_audio.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_audio_unittest.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_video_device.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_video_device.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/media_stream_constraints_util_video_device_unittest.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/user_media_processor.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/media/user_media_processor.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/pepper/pepper_media_device_manager.cc
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/content/renderer/pepper/pepper_media_device_manager.h
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/third_party/WebKit/Source/modules/mediastream/BUILD.gn
[modify] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/third_party/WebKit/public/platform/modules/mediastream/OWNERS
[rename] https://crrev.com/b1704afc626122a6a8adbafc03f67043fe114888/third_party/WebKit/public/platform/modules/mediastream/media_devices.mojom

Status: Assigned (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 6 2017

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

commit c91d62675b11cfd969a6721bf4b3e389de2c896a
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Wed Dec 06 02:53:03 2017

Define EnumTraits for content::MediaDeviceType

This CL introduces Mojom enum for content::MediaDeviceType and removes
its IPC_ENUM_TRAITS in favor of EnumTraits.
This is in preparation for migration of MediaDevices.enumerateDevices()
processing from content to Blink.

Bug:  764680 
Change-Id: I5e3c96adb7fd96e1d0ea41dff933ecac017228fa
Reviewed-on: https://chromium-review.googlesource.com/806054
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#521971}
[modify] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/content/common/DEPS
[modify] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/content/common/media/media_devices.typemap
[modify] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/content/common/media/media_devices_param_traits.h
[add] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/content/common/media/media_devices_typemap_traits.cc
[add] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/content/common/media/media_devices_typemap_traits.h
[modify] https://crrev.com/c91d62675b11cfd969a6721bf4b3e389de2c896a/third_party/WebKit/public/platform/modules/mediastream/media_devices.mojom

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 6 2017

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

commit 97d63b001414e70d3527c025a0bbc9ce86f29b51
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Wed Dec 06 03:08:32 2017

Define StructTraits for content::MediaDeviceInfo

This CL introduces Mojom struct for content::MediaDeviceInfo and removes
its IPC_STRUCT_TRAITS in favor of StructTraits.
This is in preparation for migration of MediaDevices.enumerateDevices()
processing from content to Blink.

Bug:  764680 
Change-Id: Idc17f1b53963d2fb7a80b4ac98890913cb5ad521
Reviewed-on: https://chromium-review.googlesource.com/807840
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521977}
[modify] https://crrev.com/97d63b001414e70d3527c025a0bbc9ce86f29b51/content/common/BUILD.gn
[modify] https://crrev.com/97d63b001414e70d3527c025a0bbc9ce86f29b51/content/common/media/media_devices.typemap
[delete] https://crrev.com/ae4d5c3e3c8eab6e1b409a198287f5d8b11c9a1d/content/common/media/media_devices_param_traits.cc
[delete] https://crrev.com/ae4d5c3e3c8eab6e1b409a198287f5d8b11c9a1d/content/common/media/media_devices_param_traits.h
[modify] https://crrev.com/97d63b001414e70d3527c025a0bbc9ce86f29b51/content/common/media/media_devices_typemap_traits.cc
[modify] https://crrev.com/97d63b001414e70d3527c025a0bbc9ce86f29b51/content/common/media/media_devices_typemap_traits.h
[modify] https://crrev.com/97d63b001414e70d3527c025a0bbc9ce86f29b51/third_party/WebKit/public/platform/modules/mediastream/media_devices.mojom

guidou@: With https://crrev.com/c/808128 ready for landing, is there anything else to be done w.r.t this bug? If not, I am looking forward to some more interesting contributions in this area. Please let me know.
The obvious next step is handling of the devicechange event.
Similar to this, but moving the MediaDevicesListener object from content to Blink, and sending Subscribe/UnsubscribeDeviceNotifications directly from Blink.
We can file a separate bug for that.
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 8 2017

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

commit 6ea472cef68e19fa5fbfac3a0adad10108ea4f99
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Fri Dec 08 13:27:36 2017

Migrate MediaDevices.enumerateDevices() processing from content to Blink

Currently, UserMediaClientImpl calls MediaDevicesDispatcherHost::EnumerateDevices(),
processes its result and then forwards the same to Blink.
Since Mojo MediaDevicesDispatcherHost can now be accessed from Blink [1],
blink::MediaDevices itself sets up a mojo connection with browser by creating
a MediaDevicesDispatcherHost service and calls its EnumerateDevices() directly.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/790119

Bug:  764680 
Change-Id: I04ef7df43710470c4684b794a965f8f327cb2f5d
Reviewed-on: https://chromium-review.googlesource.com/808128
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522772}
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/exported/BUILD.gn
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/Source/modules/exported/WebMediaDevicesRequest.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/BUILD.gn
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/DEPS
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/MediaDeviceInfo.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/MediaDeviceInfo.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/MediaDevices.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/MediaDevices.h
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/Source/modules/mediastream/MediaDevicesRequest.cpp
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/Source/modules/mediastream/MediaDevicesRequest.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/MediaDevicesTest.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/NavigatorMediaStream.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/Source/platform/BUILD.gn
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/Source/platform/exported/WebMediaDeviceInfo.cpp
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/public/BUILD.gn
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/public/platform/WebMediaDeviceInfo.h
[delete] https://crrev.com/17dd72fb55aaa071bf1077a02e45b9f2f44d1ed2/third_party/WebKit/public/web/WebMediaDevicesRequest.h
[modify] https://crrev.com/6ea472cef68e19fa5fbfac3a0adad10108ea4f99/third_party/WebKit/public/web/WebUserMediaClient.h

Blocking: 787254
I thought we would close this bug now and file another for devicechange handling. So, should this be blocking 787254?
Status: Fixed (was: Assigned)
I added it, just to keep track of all the bugs that are blocking bug 787254.
We can close this now. I filed  bug 793297  to track the work for the devicechange event.
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 22 2018

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

commit 6ffc648f946e3ef2a9fa9101d74edbd66a282407
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Thu Mar 22 19:23:22 2018

Make blink_headers depend on media_devices_mojo_bindings

Media devices mojo bindings are not generated for targets that depend
on blink_headers causing compilation failures.
This CL adds missing dependency for blink_headers.

blink_headers --> generate_mojo_bindings --> media_devices_mojo_bindings

Bug:  764680 
Change-Id: Idaa73dc237b93d16b3d11038e5a7ebdbb101bfdd
Reviewed-on: https://chromium-review.googlesource.com/954805
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545191}
[modify] https://crrev.com/6ffc648f946e3ef2a9fa9101d74edbd66a282407/third_party/WebKit/public/BUILD.gn

Sign in to add a comment