| Summary: | [MediaStream] Dynamically generate media capture sandbox extensions | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||||||||||||||||||||||
| Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
| Severity: | Normal | CC: | agouaillard, buildbot, commit-queue, rniwa, webkit-bug-importer | ||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||
|
Description
Eric Carlson
2016-03-01 09:31:38 PST
Created attachment 273859 [details]
Patch for the bots.
Created attachment 273993 [details]
Proposed patch
Created attachment 274003 [details]
Updated patch.
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. 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
Created attachment 274013 [details]
Updated patch.
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 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). Created attachment 292423 [details]
Updated patch.
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.
Created attachment 292427 [details]
Updated patch.
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 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; ++? Should probably have Alexey or Brent or someone look over the sandbox changes a bit more. Created attachment 292431 [details]
Updated patch for landing.
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.
Created attachment 292433 [details]
Updated patch for landing.
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.
Created attachment 292437 [details]
Updated patch for landing.
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 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. 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
Created attachment 292445 [details]
Updated patch for landing.
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.
Committed r207704: https://trac.webkit.org/r207704 |