Bug 154861 – [MediaStream] Dynamically generate media capture sandbox extensions

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Access to audio & video capture devices from the WebProcess require sandbox extensions. Instead of changing the sandbox permanently, generate extensions on dynamically when access is required.

[Radar WebKit Bug Importer]

<rdar://problem/24909411>

[Eric Carlson]

Created attachment 273859 [details]
Patch for the bots.

[Eric Carlson]

Created attachment 273993 [details]
Proposed patch

[Eric Carlson]

Created attachment 274003 [details]
Updated patch.

[Build Bot]

Comment on attachment 274003 [details]
Updated patch.

Attachment 274003 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/978511

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 274010 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Eric Carlson]

Created attachment 274013 [details]
Updated patch.

[Anders Carlsson]

Comment on attachment 274013 [details]
Updated patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=274013&action=review

> Source/WebCore/Modules/mediastream/MediaDeviceAccessRequestClient.h:49
> +class MediaDeviceAccessRequestClient {
> +public:
> +    virtual ~MediaDeviceAccessRequestClient() { }
> +
> +    enum class MediaDeviceAccess {
> +        Unknown,
> +        Pending,
> +        Allowed,
> +        Blocked,
> +    };
> +    virtual void didCompleteMediaDeviceAccessRequest(MediaDeviceAccess) = 0;
> +
> +    virtual void ref() = 0;
> +    virtual void deref() = 0;
> +};

Instead of using a client for this, I think you can just use a completion function, something like

std::function<void (MediaDeviceAccess)>> completionHandler;

> Source/WebCore/Modules/mediastream/UserMediaClient.h:57
> +    virtual void requestMediaDeviceAccess(MediaDeviceAccessRequestClient&) { }

This would turn into virtual void requestMdiaDeviceAccess(std::function<void (MediaDeviceAccess)>> completionHandler) { }

Is it OK to no call the client here?

> Source/WebCore/Modules/mediastream/UserMediaRequest.h:77
> +    void ref() override { RefCounted<UserMediaRequest>::ref(); }
> +    void deref() override { RefCounted<UserMediaRequest>::deref(); }

Is it OK to keep UserMediaRequest alive longer here?

> Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:378
> +;; Media capture, microphone access
> +(with-filter (extension "com.apple.webkit.microphone")
> +    (allow device-microphone))
> +
> +;; Media capture, camera access
> +(with-filter (extension "com.apple.webkit.camera")

Didn't know that you could do this with sandbox extensions - that's pretty cool!

[Brent Fulgham]

Comment on attachment 274013 [details]
Updated patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=274013&action=review

>> Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:378
>> +(with-filter (extension "com.apple.webkit.camera")
> 
> Didn't know that you could do this with sandbox extensions - that's pretty cool!

Is this needed on iOS, too? iOS does not support some fancier operations because of performance concerns, so you might need to do some testing on device to ensure things work the way you expect. (annoying, I realize).

[Eric Carlson]

Created attachment 292423 [details]
Updated patch.

[WebKit Commit Bot]

Attachment 292423 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:193:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:193:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 292427 [details]
Updated patch.

[WebKit Commit Bot]

Attachment 292427 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 292427 [details]
Updated patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=292427&action=review

It would be ideal if you didn't land it until it builds.

> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:138
> +        extensionCount += 1;

++?

[Tim Horton]

Should probably have Alexey or Brent or someone look over the sandbox changes a bit more.

[Eric Carlson]

Created attachment 292431 [details]
Updated patch for landing.

[WebKit Commit Bot]

Attachment 292431 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 292433 [details]
Updated patch for landing.

[WebKit Commit Bot]

Attachment 292433 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 292437 [details]
Updated patch for landing.

[WebKit Commit Bot]

Attachment 292437 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 292437 [details]
Updated patch for landing.

Attachment 292437 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2341549

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 292444 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Eric Carlson]

Created attachment 292445 [details]
Updated patch for landing.

[WebKit Commit Bot]

Attachment 292445 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:194:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Committed r207704: https://trac.webkit.org/r207704