Bug 167430 - Implement <iframe allow="camera; microphone">
Summary: Implement <iframe allow="camera; microphone">
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-25 13:47 PST by Simon Pieters
Modified: 2018-07-20 20:18 PDT (History)
14 users (show)

See Also:


Attachments
Patch (142.33 KB, patch)
2017-10-09 08:25 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (1.12 MB, application/zip)
2017-10-09 09:50 PDT, Build Bot
no flags Details
Patch (144.86 KB, patch)
2017-10-09 10:13 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (29.31 KB, patch)
2017-12-14 16:30 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (29.32 KB, patch)
2017-12-14 16:33 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (2.70 MB, application/zip)
2017-12-14 17:45 PST, Build Bot
no flags Details
Patch for landing with fixed message (30.18 KB, patch)
2017-12-14 17:46 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing with fixed message (32.69 KB, patch)
2017-12-14 17:49 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.27 MB, application/zip)
2017-12-14 19:14 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.71 MB, application/zip)
2017-12-14 19:15 PST, Build Bot
no flags Details
Patch (36.26 KB, patch)
2017-12-14 19:26 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-elcapitan (3.00 MB, application/zip)
2017-12-14 21:22 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters 2017-01-25 13:47:58 PST
+++ This bug was initially created as a clone of Bug #167417 +++

I guess WebKit doesn't implement getUserMedia() yet(?), but filing this anyway...

<iframe allowusermedia> controls if an iframe can access the getUserMedia() API.

On the spec side:

* When a document is initialized (https://html.spec.whatwg.org/#creating-a-new-browsing-context and https://html.spec.whatwg.org/#initialise-the-document-object ), https://html.spec.whatwg.org/#set-the-allow*-flags is run, which can set "allowusemedia flag" on Document.
* getUserMedia also call allowed to use: https://github.com/w3c/mediacapture-main/pull/427

Relevant whatwg/html PRs:

https://github.com/whatwg/html/pull/2195
https://github.com/whatwg/html/pull/2217

Relevant w3c/web-platform-tests issue to add tests:

https://github.com/w3c/web-platform-tests/issues/4625
Comment 1 Dave Collie 2017-10-04 12:29:02 PDT
The recent release of Safari 11 adds support for WebRTC.  This opens up an opportunity to finally replace Flash based camera recording.  However, the following checks prevent calling getUserMedia from an iframe.

https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp#L116

https://github.com/WebKit/webkit/blob/master/Source/WebCore/page/SecurityOrigin.cpp#L563

If 'allowusermedia' was implemented it would allow different webrtc camera recording implementations that are iframed into client sites to function.  Currently, they have to fallback to flash.
Comment 2 Jon Lee 2017-10-04 17:04:25 PDT
(In reply to Dave Collie from comment #1)
> The recent release of Safari 11 adds support for WebRTC.  This opens up an
> opportunity to finally replace Flash based camera recording.  However, the
> following checks prevent calling getUserMedia from an iframe.
> 
> https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/
> mediastream/UserMediaRequest.cpp#L116
> 
> https://github.com/WebKit/webkit/blob/master/Source/WebCore/page/
> SecurityOrigin.cpp#L563
> 
> If 'allowusermedia' was implemented it would allow different webrtc camera
> recording implementations that are iframed into client sites to function. 
> Currently, they have to fallback to flash.

What different implementations are out there that have wide deployment?
Comment 3 Dave Collie 2017-10-05 05:51:14 PDT
Cameratag (https://cameratag.com/demo) currently doesn't work with Safari 11 unless flash fallback is forced.

It doesn't require iframing but there are use cases where it's easier to implement cameratag on a subdomain and embed in other services.  

The company I work for is using the same strategy for our own recording platform and partner ecosystem.  We were hoping with Safari 11 that we could finally relegate flash camera recording to IE11 only.

For Chrome we use the 'allow' attribute https://wicg.github.io/feature-policy/#iframe-allow-attribute to permit accessing the camera and microphone.  This doesn't appear to be implemented in Firefox.
Comment 4 youenn fablet 2017-10-07 17:13:01 PDT
A mechanism to allow iframes to access camera/mic on behalf of the top document makes sense to me.
That said, allowusermedia might be removed from the spec and replaced by something like https://wicg.github.io/feature-policy/
Comment 5 youenn fablet 2017-10-09 08:25:52 PDT
Created attachment 323176 [details]
Patch
Comment 6 Radar WebKit Bug Importer 2017-10-09 09:42:55 PDT
<rdar://problem/34887226>
Comment 7 Build Bot 2017-10-09 09:50:57 PDT
Comment on attachment 323176 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-audio-only.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-id-manual.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-idl.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStreamTrack-MediaElement-disabled-audio-is-silence.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStreamTrack-getSettings.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-default-feature-policy.https.sub.html
imported/w3c/web-platform-tests/mediacapture-streams/GUM-trivial-constraint.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaDevices-IDL-enumerateDevices.html
imported/w3c/web-platform-tests/mediacapture-streams/GUM-deny.https.html
imported/w3c/web-platform-tests/mediacapture-streams/GUM-impossible-constraint.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-video-only.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-preload-none.https.html
imported/w3c/web-platform-tests/mediacapture-streams/GUM-optional-constraint.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-removetrack.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-add-audio-track.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStreamTrack-init.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStreamTrack-id.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-gettrackid.https.html
Comment 8 Build Bot 2017-10-09 09:50:58 PDT
Created attachment 323179 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 youenn fablet 2017-10-09 10:13:00 PDT
Created attachment 323185 [details]
Patch
Comment 10 Jon Lee 2017-10-09 12:12:40 PDT
Comment on attachment 323185 [details]
Patch

We need to discuss this change in policy. Third-party frames are disallowed for a reason.
Comment 11 Adam Szmyd 2017-11-06 05:06:51 PST
Is there any progress on this one? We're implementing a voice client that uses iFrames to allow users to plug our embedded iFrame on their side and allow communication. 

Current Safari11 gives us this error "Trying to call getUserMedia from a document with a different security origin than its top-level frame." 

Can someone give me an answer if capturing microphone from cross-domain iFrame (both host and child iFrame on HTTPS of course) will be possible?
Comment 12 youenn fablet 2017-11-08 09:41:32 PST
(In reply to Adam Szmyd from comment #11)
> Is there any progress on this one? We're implementing a voice client that
> uses iFrames to allow users to plug our embedded iFrame on their side and
> allow communication. 
> 
> Current Safari11 gives us this error "Trying to call getUserMedia from a
> document with a different security origin than its top-level frame." 
> 
> Can someone give me an answer if capturing microphone from cross-domain
> iFrame (both host and child iFrame on HTTPS of course) will be possible?

It was discussed at W3C TPAC and getUserMedia spec should be updated to support feature policy. That should help adoption here.
Comment 13 Eric Carlson 2017-11-09 08:59:03 PST
(In reply to youenn fablet from comment #12)
> 
> It was discussed at W3C TPAC and getUserMedia spec should be updated to
> support feature policy. That should help adoption here.

https://github.com/w3c/mediacapture-main/pull/440
Comment 14 Eric Carlson 2017-11-09 09:40:20 PST
Comment on attachment 323185 [details]
Patch

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

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:132
> +    for (auto allowItem : StringView { allow }.split(';')) {
> +        auto item = allowItem.stripLeadingAndTrailingMatchedCharacters(isHTMLSpace<UChar>);
> +        if (requiresAudio && item == "microphone")
> +            requiresAudio = false;
> +        if (requiresVideo && item == "camera")
> +            requiresVideo = false;
> +    }
> +    return !requiresVideo && !requiresAudio;

Nit: I think using local variable names like "allowCameraAccess" and "allowMicrophoneAccess" and revising this logic would make it easier to follow.
Comment 15 youenn fablet 2017-12-14 16:30:13 PST
Created attachment 329418 [details]
Patch for landing
Comment 16 youenn fablet 2017-12-14 16:33:06 PST
Created attachment 329419 [details]
Patch for landing
Comment 17 Jon Lee 2017-12-14 16:51:37 PST
Comment on attachment 329419 [details]
Patch for landing

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

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:107
> +static bool isAllowedToUse(Document& document, Document& topDocument, bool requiresAudio, bool requiresVideo)

Don't we normally eschew bool parameters and go for enums instead?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:153
> +                return false;

Is there an appropriate errorMessage we can return here?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:161
>                  errorMessage = "Trying to call getUserMedia from a document with a different security origin than its top-level frame.";

This doesn't seem like the right message now. It should be something along the lines of "The top-level frame has prevented a document with a different security origin to call getUserMedia."
Comment 18 youenn fablet 2017-12-14 17:28:46 PST
Comment on attachment 329419 [details]
Patch for landing

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

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:107
>> +static bool isAllowedToUse(Document& document, Document& topDocument, bool requiresAudio, bool requiresVideo)
> 
> Don't we normally eschew bool parameters and go for enums instead?

We usually do that for class methods or helper functions defined in headers.
Since this is a static inline here and given the fact that we would need to compute the enum value at the call site, it does not seem very attractive.

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:153
>> +                return false;
> 
> Is there an appropriate errorMessage we can return here?

I'll dig into it, I was thinking this could be some disconnected frame cases but I should check further.

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:161
>>                  errorMessage = "Trying to call getUserMedia from a document with a different security origin than its top-level frame.";
> 
> This doesn't seem like the right message now. It should be something along the lines of "The top-level frame has prevented a document with a different security origin to call getUserMedia."

ok, will update the message.
Comment 19 Build Bot 2017-12-14 17:45:31 PST
Comment on attachment 329419 [details]
Patch for landing

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

New failing tests:
imported/w3c/web-platform-tests/XMLHttpRequest/xmlhttprequest-sync-default-feature-policy.sub.html
Comment 20 Build Bot 2017-12-14 17:45:33 PST
Created attachment 329429 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 21 youenn fablet 2017-12-14 17:46:13 PST
Created attachment 329430 [details]
Patch for landing with fixed message
Comment 22 youenn fablet 2017-12-14 17:49:08 PST
Created attachment 329431 [details]
Patch for landing with fixed message
Comment 23 youenn fablet 2017-12-14 17:50:06 PST
(In reply to youenn fablet from comment #22)
> Created attachment 329431 [details]
> Patch for landing with fixed message

Skipped xhr test which is timing out.
Time out is triggered by the fact that the test is now going further due to added support for allow attribute.
Comment 24 youenn fablet 2017-12-14 17:51:56 PST
> > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:153
> > +                return false;
> 
> Is there an appropriate errorMessage we can return here?

I removed this additional check.
This is already in the code base and I saw another place which follows the same pattern without checking document.
There may be a bug though since for a disconnected frame, we might have a document but no parent document. Maybe we should check early for a frame.
This should be fixed in a follow-up anyway.
Comment 25 Build Bot 2017-12-14 19:14:03 PST
Comment on attachment 329431 [details]
Patch for landing with fixed message

Attachment 329431 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5667449

New failing tests:
http/tests/ssl/media-stream/get-user-media-nested.html
http/tests/ssl/media-stream/get-user-media-different-host.html
Comment 26 Build Bot 2017-12-14 19:14:04 PST
Created attachment 329438 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 27 Build Bot 2017-12-14 19:15:49 PST
Comment on attachment 329431 [details]
Patch for landing with fixed message

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

New failing tests:
http/tests/ssl/media-stream/get-user-media-nested.html
http/tests/ssl/media-stream/get-user-media-different-host.html
Comment 28 Build Bot 2017-12-14 19:15:51 PST
Created attachment 329440 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 29 youenn fablet 2017-12-14 19:26:44 PST
Created attachment 329444 [details]
Patch
Comment 30 Build Bot 2017-12-14 21:22:12 PST
Comment on attachment 329444 [details]
Patch

Attachment 329444 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5669030

New failing tests:
webgl/1.0.2/conformance/context/context-release-upon-reload.html
Comment 31 Build Bot 2017-12-14 21:22:13 PST
Created attachment 329454 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 32 WebKit Commit Bot 2017-12-14 21:32:58 PST
Comment on attachment 329444 [details]
Patch

Clearing flags on attachment: 329444

Committed r225963: <https://trac.webkit.org/changeset/225963>
Comment 33 WebKit Commit Bot 2017-12-14 21:33:00 PST
All reviewed patches have been landed.  Closing bug.