Enforce user gesture for getUserMedia in case a previous getUserMedia call was denied

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

Enforce user gesture for getUserMedia in case a previous getUserMedia call was denied

[youenn fablet]

Created attachment 381801 [details]
Patch

[youenn fablet]

Created attachment 381817 [details]
Patch

[Eric Carlson]

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?

[youenn fablet]

Comment on attachment 381817 [details]
Patch

Will fix test

[youenn fablet]

Created attachment 381936 [details]
Patch

[Eric Carlson]

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.

[youenn fablet]

> 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.

[WebKit Commit Bot]

Comment on attachment 381936 [details]
Patch

Clearing flags on attachment: 381936

Committed r251639: <https://trac.webkit.org/changeset/251639>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56648232>

[Fujii Hironori]

Committed r251646: <https://trac.webkit.org/changeset/251646>

[Matt Lewis]

This caused internal and Internal API Test to flake much more frequently.

[Truitt Savell]

Reverted r251646 for reason:

Caused flakey API failures for GetDisplayMediaTest.Constraints

Committed r251711: <https://trac.webkit.org/changeset/251711>

[Truitt Savell]

Reverted r251639 for reason:

Caused flakey API failures for GetDisplayMediaTest.Constraints

Committed r251712: <https://trac.webkit.org/changeset/251712>

[youenn fablet]

Created attachment 382724 [details]
Patch

[youenn fablet]

Created attachment 382737 [details]
Patch

[WebKit Commit Bot]

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

[youenn fablet]

Created attachment 382807 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 382807 [details]
Patch

Clearing flags on attachment: 382807

Committed r252046: <https://trac.webkit.org/changeset/252046>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

Committed r252048: <https://trac.webkit.org/changeset/252048>