Bug 154861 - [MediaStream] Dynamically generate media capture sandbox extensions
Summary: [MediaStream] Dynamically generate media capture sandbox extensions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-03-01 09:31 PST by Eric Carlson
Modified: 2016-10-21 20:39 PDT (History)
5 users (show)

See Also:


Attachments
Patch for the bots. (1.51 MB, patch)
2016-03-12 16:27 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Proposed patch (1.51 MB, patch)
2016-03-14 11:18 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch. (1.51 MB, patch)
2016-03-14 12:52 PDT, Eric Carlson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (160.39 KB, application/zip)
2016-03-14 13:27 PDT, Build Bot
no flags Details
Updated patch. (1.51 MB, patch)
2016-03-14 13:34 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch. (30.92 KB, patch)
2016-10-21 15:51 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch. (30.96 KB, patch)
2016-10-21 16:36 PDT, Eric Carlson
thorton: review+
Details | Formatted Diff | Diff
Updated patch for landing. (30.98 KB, patch)
2016-10-21 16:58 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch for landing. (31.00 KB, patch)
2016-10-21 17:10 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch for landing. (31.02 KB, patch)
2016-10-21 17:24 PDT, Eric Carlson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (564.16 KB, application/zip)
2016-10-21 18:16 PDT, Build Bot
no flags Details
Updated patch for landing. (31.08 KB, patch)
2016-10-21 18:50 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2016-03-01 09:31:38 PST
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.
Comment 1 Radar WebKit Bug Importer 2016-03-01 09:36:05 PST
<rdar://problem/24909411>
Comment 2 Eric Carlson 2016-03-12 16:27:01 PST
Created attachment 273859 [details]
Patch for the bots.
Comment 3 Eric Carlson 2016-03-14 11:18:22 PDT
Created attachment 273993 [details]
Proposed patch
Comment 4 Eric Carlson 2016-03-14 12:52:13 PDT
Created attachment 274003 [details]
Updated patch.
Comment 5 Build Bot 2016-03-14 13:27:25 PDT
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.
Comment 6 Build Bot 2016-03-14 13:27:28 PDT
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
Comment 7 Eric Carlson 2016-03-14 13:34:36 PDT
Created attachment 274013 [details]
Updated patch.
Comment 8 Anders Carlsson 2016-03-14 14:22:41 PDT
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!
Comment 9 Brent Fulgham 2016-04-13 22:20:09 PDT
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).
Comment 10 Eric Carlson 2016-10-21 15:51:38 PDT
Created attachment 292423 [details]
Updated patch.
Comment 11 WebKit Commit Bot 2016-10-21 15:52:32 PDT
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.
Comment 12 Eric Carlson 2016-10-21 16:36:08 PDT
Created attachment 292427 [details]
Updated patch.
Comment 13 WebKit Commit Bot 2016-10-21 16:38:50 PDT
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.
Comment 14 Tim Horton 2016-10-21 16:47:48 PDT
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;

++?
Comment 15 Tim Horton 2016-10-21 16:48:14 PDT
Should probably have Alexey or Brent or someone look over the sandbox changes a bit more.
Comment 16 Eric Carlson 2016-10-21 16:58:39 PDT
Created attachment 292431 [details]
Updated patch for landing.
Comment 17 WebKit Commit Bot 2016-10-21 16:59:54 PDT
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.
Comment 18 Eric Carlson 2016-10-21 17:10:13 PDT
Created attachment 292433 [details]
Updated patch for landing.
Comment 19 WebKit Commit Bot 2016-10-21 17:13:44 PDT
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.
Comment 20 Eric Carlson 2016-10-21 17:24:25 PDT
Created attachment 292437 [details]
Updated patch for landing.
Comment 21 WebKit Commit Bot 2016-10-21 17:28:06 PDT
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.
Comment 22 Build Bot 2016-10-21 18:16:41 PDT
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.
Comment 23 Build Bot 2016-10-21 18:16:45 PDT
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
Comment 24 Eric Carlson 2016-10-21 18:50:58 PDT
Created attachment 292445 [details]
Updated patch for landing.
Comment 25 WebKit Commit Bot 2016-10-21 18:52:42 PDT
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.
Comment 26 Eric Carlson 2016-10-21 20:39:07 PDT
Committed r207704: https://trac.webkit.org/r207704