Introduce WKWebViewConfiguration mediaDevicesEnabled
Created attachment 409463 [details] Patch
Created attachment 409465 [details] Patch
Comment on attachment 409465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409465&action=review > Source/WebKit/ChangeLog:11 > + Add a new boolean configuration to enable/disable mediaDevices exposure. > + Value is off by default. > + A future patch will remove the WebPreferences based API, given there is no need > + to enable/disable mediaDevices throughout the course of a page. Can you explain a bit more about why you are moving this? Most feature enablement is currently done through the preferences mechanism, so having this one be on the configuration seems odd and inconsistent. If there is new thought process around this, it would be good to have that articulated so we can start the process of migrating everything.
(In reply to Sam Weinig from comment #3) > Comment on attachment 409465 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=409465&action=review > > > Source/WebKit/ChangeLog:11 > > + Add a new boolean configuration to enable/disable mediaDevices exposure. > > + Value is off by default. > > + A future patch will remove the WebPreferences based API, given there is no need > > + to enable/disable mediaDevices throughout the course of a page. > > Can you explain a bit more about why you are moving this? Most feature > enablement is currently done through the preferences mechanism, so having > this one be on the configuration seems odd and inconsistent. I am not sure how much this is inconsistent. There is not much API available in WKPreferences.h. There are some precedents in WKWebViewConfiguration.h, like allowsAirPlayForMediaPlayback. > If there is new thought process around this, it would be good to have that > articulated so we can start the process of migrating everything. WebPreferences can be changed during the lifetime of the page. This can be useful, say you want to enable/disable mock devices. In that particular case, I see no use case for enabling/disabling this setting during the lifetime of the page. There are some downsides of allowing it (some contexts may have navigator.mediaDevices, some may not). It seems best to enforce a fixed setting hence the idea to move it to WKWebViewConfiguration. Wdyt?
Comment on attachment 409465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409465&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:172 > + BOOL _mediaDevicesEnabled; This should go on API::PageConfiguration instead. > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfigurationPrivate.h:123 > +@property (nonatomic, setter=_setMediaDevicesEnabled:) BOOL _mediaDevicesEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); Could you deprecate the preference SPI with this as a replacement?
(In reply to Alex Christensen from comment #5) > Comment on attachment 409465 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=409465&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:172 > > + BOOL _mediaDevicesEnabled; > > This should go on API::PageConfiguration instead. OK. > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfigurationPrivate.h:123 > > +@property (nonatomic, setter=_setMediaDevicesEnabled:) BOOL _mediaDevicesEnabled WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > Could you deprecate the preference SPI with this as a replacement? Sure, I plan to remove the preference SPI as a follow-up though.
Created attachment 409475 [details] Patch
<rdar://problem/69440832>
(In reply to youenn fablet from comment #4) > (In reply to Sam Weinig from comment #3) > > Comment on attachment 409465 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=409465&action=review > > > > > Source/WebKit/ChangeLog:11 > > > + Add a new boolean configuration to enable/disable mediaDevices exposure. > > > + Value is off by default. > > > + A future patch will remove the WebPreferences based API, given there is no need > > > + to enable/disable mediaDevices throughout the course of a page. > > > > Can you explain a bit more about why you are moving this? Most feature > > enablement is currently done through the preferences mechanism, so having > > this one be on the configuration seems odd and inconsistent. > > I am not sure how much this is inconsistent. > There is not much API available in WKPreferences.h. > There are some precedents in WKWebViewConfiguration.h, like > allowsAirPlayForMediaPlayback. The preferences expose all the preferences set in the WebPreferences*.yaml files. There are a ton of them. They don't each necessarily get their own corresponding WKPreferences API call site, but most can accessed via the enumerations for internal debug or experimental features accessors. > > > If there is new thought process around this, it would be good to have that > > articulated so we can start the process of migrating everything. > > WebPreferences can be changed during the lifetime of the page. > This can be useful, say you want to enable/disable mock devices. > > In that particular case, I see no use case for enabling/disabling this > setting during the lifetime of the page. There are some downsides of > allowing it (some contexts may have navigator.mediaDevices, some may not). > It seems best to enforce a fixed setting hence the idea to move it to > WKWebViewConfiguration. > Wdyt? It's hard to judge as I don't know what your intended use case for this is. Who is the target audience of this SPI?
<rdar://problem/69441004>
> > In that particular case, I see no use case for enabling/disabling this > > setting during the lifetime of the page. There are some downsides of > > allowing it (some contexts may have navigator.mediaDevices, some may not). > > It seems best to enforce a fixed setting hence the idea to move it to > > WKWebViewConfiguration. > > Wdyt? > > It's hard to judge as I don't know what your intended use case for this is. > Who is the target audience of this SPI? Goal is not to stay at SPI.
Comment on attachment 409475 [details] Patch Actually, we may want to put this on WKWebpagePreferences.
Comment on attachment 409475 [details] Patch The WKUIDelegate needs to give permission, and that can be per-site. This is great.
We might want to rename mediaDevicesEnabled to mediaCaptureEnabled which intent is clearer.
Created attachment 409676 [details] Patch for landing
Created attachment 409678 [details] Patch for landing
Committed r267563: <https://trac.webkit.org/changeset/267563> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409678 [details].
Removing preference in https://bugs.webkit.org/show_bug.cgi?id=216970