Bug 203362 - Enforce user gesture for getUserMedia in case a previous getUserMedia call was denied
Summary: Enforce user gesture for getUserMedia in case a previous getUserMedia call wa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-24 05:30 PDT by youenn fablet
Modified: 2019-11-05 02:13 PST (History)
16 users (show)

See Also:


Attachments
Patch (16.97 KB, patch)
2019-10-24 05:43 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (18.51 KB, patch)
2019-10-24 09:31 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (18.44 KB, patch)
2019-10-25 10:00 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (25.23 KB, patch)
2019-11-04 02:17 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (37.13 KB, patch)
2019-11-04 07:19 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (37.13 KB, patch)
2019-11-04 23:12 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2019-10-24 05:30:37 PDT
Enforce user gesture for getUserMedia in case a previous getUserMedia call was denied
Comment 1 youenn fablet 2019-10-24 05:43:45 PDT
Created attachment 381801 [details]
Patch
Comment 2 youenn fablet 2019-10-24 09:31:50 PDT
Created attachment 381817 [details]
Patch
Comment 3 Eric Carlson 2019-10-24 13:17:53 PDT
Comment on attachment 381817 [details]
Patch

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

> LayoutTests/fast/mediastream/getUserMedia-deny-persistency-expected.txt:3
> +FAIL Testing getUserMedia with and without user gesture after user denied access assert_unreached: [object MediaStream] Reached unreachable code

This seems wrong, do you really want a FAIL in the results?
Comment 4 youenn fablet 2019-10-24 14:04:02 PDT
Comment on attachment 381817 [details]
Patch

Will fix test
Comment 5 youenn fablet 2019-10-25 10:00:55 PDT
Created attachment 381936 [details]
Patch
Comment 6 Eric Carlson 2019-10-25 18:31:08 PDT
Comment on attachment 381936 [details]
Patch

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

> Source/WebCore/Modules/mediastream/MediaDevices.cpp:118
> +    auto* currentGestureToken = UserGestureIndicator::currentUserGesture().get();
> +    if (m_currentGestureToken != currentGestureToken) {
> +        m_currentGestureToken = currentGestureToken;
> +        m_requestTypesForCurrentGesture = { };
> +    }
> +
> +    bool isUserGesturePriviledged = m_currentGestureToken && !m_requestTypesForCurrentGesture.contains(requestType);
> +    m_requestTypesForCurrentGesture.add(requestType);
> +    return isUserGesturePriviledged;

I'm not completely comfortable with allowing script to reprompt the user on every user gesture, forever, but we can start with this and talk about rate (or other) limits to the reprompts later.
Comment 7 youenn fablet 2019-10-26 13:07:52 PDT
> I'm not completely comfortable with allowing script to reprompt the user on
> every user gesture, forever, but we can start with this and talk about rate
> (or other) limits to the reprompts later.

Sounds good.
I am also thinking we should try to remove the iframe check, at least for deny cases.
Comment 8 WebKit Commit Bot 2019-10-26 13:50:47 PDT
Comment on attachment 381936 [details]
Patch

Clearing flags on attachment: 381936

Committed r251639: <https://trac.webkit.org/changeset/251639>
Comment 9 WebKit Commit Bot 2019-10-26 13:50:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-10-26 13:51:16 PDT
<rdar://problem/56648232>
Comment 11 Fujii Hironori 2019-10-27 19:50:08 PDT
Committed r251646: <https://trac.webkit.org/changeset/251646>
Comment 12 Matt Lewis 2019-10-29 10:15:25 PDT
This caused internal and Internal API Test to flake much more frequently.
Comment 13 Truitt Savell 2019-10-29 10:29:37 PDT
Reverted r251646 for reason:

Caused flakey API failures for GetDisplayMediaTest.Constraints

Committed r251711: <https://trac.webkit.org/changeset/251711>
Comment 14 Truitt Savell 2019-10-29 10:33:48 PDT
Reverted r251639 for reason:

Caused flakey API failures for GetDisplayMediaTest.Constraints

Committed r251712: <https://trac.webkit.org/changeset/251712>
Comment 15 youenn fablet 2019-11-04 02:17:38 PST
Created attachment 382724 [details]
Patch
Comment 16 youenn fablet 2019-11-04 07:19:28 PST
Created attachment 382737 [details]
Patch
Comment 17 WebKit Commit Bot 2019-11-04 08:23:25 PST
Comment on attachment 382737 [details]
Patch

Rejecting attachment 382737 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 382737, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13210872
Comment 18 youenn fablet 2019-11-04 23:12:45 PST
Created attachment 382807 [details]
Patch
Comment 19 WebKit Commit Bot 2019-11-05 01:14:40 PST
Comment on attachment 382807 [details]
Patch

Clearing flags on attachment: 382807

Committed r252046: <https://trac.webkit.org/changeset/252046>
Comment 20 WebKit Commit Bot 2019-11-05 01:14:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Fujii Hironori 2019-11-05 02:13:15 PST
Committed r252048: <https://trac.webkit.org/changeset/252048>