[webkit-reviews] review granted: [Bug 172859] getUserMedia is prompting too often : [Attachment 312151] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 172859: getUserMedia is prompting too often
https://bugs.webkit.org/show_bug.cgi?id=172859

Attachment 312151: Patch

https://bugs.webkit.org/attachment.cgi?id=312151&action=review




--- Comment #9 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 312151
  --> https://bugs.webkit.org/attachment.cgi?id=312151
Patch

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

> Source/WebKit2/ChangeLog:14
> +	   A granted request is now keeping its mainFrameID.

Nit: "is now keeping" => "now keeps"

> Source/WebKit2/ChangeLog:15
> +	   Whenever the document of the main frame is changing, the granted
requests for that mainFrameID will be removed.

Nit: "main frame is changing" => "main frame changes"

> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:241
> -	   if (!m_page.isValid())
> +	   if (!m_page.isValid() || !m_page.mainFrame())

Why is this change necessary?

> LayoutTests/fast/mediastream/getUserMedia-grant-persistency.html:48
> +    }).then(() => {
> +	   if (!window.internals)
> +	       return;
> +	  
assert_true(internals.pageMediaState().includes("HasActiveVideoCaptureDevice"),
"Check active video");
> +	  
assert_true(internals.pageMediaState().includes("HasActiveAudioCaptureDevice"),
"Check active audio");
> +    });

This seems wrong. Why would we allow a call to getUserMedia not triggered by a
user gesture to un-mute capture?

> LayoutTests/fast/mediastream/getUserMedia-grant-persistency.html:59
> +    }).then(() => {
> +    }).then(() => {

Nit: are both of these needed?

>
LayoutTests/http/tests/media/media-stream/get-user-media-prompt-expected.txt:20
>  ** Request a stream with video and audio, the user should be prompted again
**

Nit: this comment should be updated, the user is NOT prompted again.