Bug 220033 - Check availability of speech recognition service before requesting permissions
Summary: Check availability of speech recognition service before requesting permissions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-18 17:24 PST by Sihui Liu
Modified: 2020-12-21 11:35 PST (History)
9 users (show)

See Also:


Attachments
Patch (15.58 KB, patch)
2020-12-18 17:33 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (15.61 KB, patch)
2020-12-20 17:44 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (16.58 KB, patch)
2020-12-21 10:57 PST, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2020-12-18 17:24:11 PST
...
Comment 1 Sihui Liu 2020-12-18 17:33:21 PST
Created attachment 416561 [details]
Patch
Comment 2 Sihui Liu 2020-12-20 17:44:48 PST
Created attachment 416593 [details]
Patch
Comment 3 youenn fablet 2020-12-21 09:20:03 PST
Comment on attachment 416593 [details]
Patch

LGTM.
I am unsure whether we can bypass the speech service TCC prompt to get a valid isAvailable check.
Did you test that manually?

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

> Source/WebKit/UIProcess/Cocoa/MediaPermissionUtilities.mm:267
> +    return recognizer && [recognizer isAvailable];

Is it ok to try creating allocSFSpeechRecognizerInstance and do we get a valid isAvailable result when the TCC prompt is unknown?

> Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp:110
> +        m_speechRecognitionServiceCheck = m_microphoneCheck = CheckResult::Denied;

Can we just do:
if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->lang())) {
    completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
    return;
}

> Source/WebKit/UIProcess/SpeechRecognitionServer.cpp:66
> +    m_permissionChecker(request.lang(), request.clientOrigin(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](SpeechRecognitionPermissionDecision decision) mutable {

auto decision

> Source/WebKit/UIProcess/WebProcessProxy.cpp:1725
> +    auto permissionChecker = [weakPage = makeWeakPtr(targetPage)](auto lang, auto& origin, auto&& completionHandler) mutable {

auto& lang
Comment 4 Sihui Liu 2020-12-21 10:57:14 PST
Created attachment 416610 [details]
Patch for landing
Comment 5 Sihui Liu 2020-12-21 10:58:53 PST
(In reply to youenn fablet from comment #3)
> Comment on attachment 416593 [details]
> Patch
> 
> LGTM.
> I am unsure whether we can bypass the speech service TCC prompt to get a
> valid isAvailable check.
> Did you test that manually?

Yes, I tested on a newly built MiniBrowser (no permission granted yet), and it did not crash or error when checking the availability.

> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416593&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/MediaPermissionUtilities.mm:267
> > +    return recognizer && [recognizer isAvailable];
> 
> Is it ok to try creating allocSFSpeechRecognizerInstance and do we get a
> valid isAvailable result when the TCC prompt is unknown?
> 
> > Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp:110
> > +        m_speechRecognitionServiceCheck = m_microphoneCheck = CheckResult::Denied;
> 
> Can we just do:
> if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->lang())) {
>     completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
>     return;
> }
> 
> > Source/WebKit/UIProcess/SpeechRecognitionServer.cpp:66
> > +    m_permissionChecker(request.lang(), request.clientOrigin(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](SpeechRecognitionPermissionDecision decision) mutable {
> 
> auto decision
> 
> > Source/WebKit/UIProcess/WebProcessProxy.cpp:1725
> > +    auto permissionChecker = [weakPage = makeWeakPtr(targetPage)](auto lang, auto& origin, auto&& completionHandler) mutable {
> 
> auto& lang
Comment 6 Radar WebKit Bug Importer 2020-12-21 11:05:34 PST
<rdar://problem/72555917>
Comment 7 EWS 2020-12-21 11:35:49 PST
Committed r271031: <https://trac.webkit.org/changeset/271031>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416610 [details].