Bug 220133 - Fail speech recognition when page is muted for audio capture
Summary: Fail speech recognition when page is muted for audio capture
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-23 16:41 PST by Sihui Liu
Modified: 2021-01-05 10:16 PST (History)
8 users (show)

See Also:

Attachments
Patch (18.65 KB, patch)
2020-12-23 16:49 PST, Sihui Liu 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (29.53 KB, patch)
2021-01-04 22:58 PST, Sihui Liu 		no flags Details | Formatted Diff | Diff
Patch (29.53 KB, patch)
2021-01-05 00:08 PST, Sihui Liu 		no flags Details | Formatted Diff | Diff
Patch for landing (27.44 KB, patch)
2021-01-05 09:45 PST, Sihui Liu 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2020-12-23 16:41:55 PST 
...
Comment 1 Sihui Liu 2020-12-23 16:49:36 PST 
Created attachment 416734 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2020-12-30 16:42:12 PST 
<rdar://problem/72745232>
Comment 3 Sihui Liu 2021-01-04 22:58:37 PST 
Created attachment 416973 [details]
Patch
Comment 4 Sihui Liu 2021-01-05 00:08:43 PST 
Created attachment 416975 [details]
Patch
Comment 5 youenn fablet 2021-01-05 02:08:06 PST 
Comment on attachment 416975 [details]
Patch

Looks good overall.
Some style improvements below.

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

> Source/WebKit/ChangeLog:11
> +        1. when page is muted, mute audio capture source used for speech recognition audion on the page. This will

s/audion/audio/

> Source/WebKit/UIProcess/UserMediaProcessManager.cpp:73
> +    }

I am not sure we need to create a HashSet here.
We could either add a SpeechRecognitionPermissionManager::muteCaptureExceptIn(WebPageProxy&) or rely on WebProcessProxy::globalPageMap() and remove UserMediaProcessManager::muteCaptureMediaStreamsExceptIn

> Source/WebKit/UIProcess/WebProcessProxy.cpp:1779
> +    speechRecognitionServer->mute();

I would write it as if (auto ...)
Comment 6 Sihui Liu 2021-01-05 09:37:29 PST 
(In reply to youenn fablet from comment #5)
> Comment on attachment 416975 [details]
> Patch
> 
> Looks good overall.
> Some style improvements below.

Thanks for review!

> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416975&action=review
> 
> > Source/WebKit/ChangeLog:11
> > +        1. when page is muted, mute audio capture source used for speech recognition audion on the page. This will
> 
> s/audion/audio/
> 
> > Source/WebKit/UIProcess/UserMediaProcessManager.cpp:73
> > +    }
> 
> I am not sure we need to create a HashSet here.
> We could either add a
> SpeechRecognitionPermissionManager::muteCaptureExceptIn(WebPageProxy&) or
> rely on WebProcessProxy::globalPageMap() and remove
> UserMediaProcessManager::muteCaptureMediaStreamsExceptIn

Okay, removing UserMediaProcessManager::muteCaptureMediaStreamsExceptIn and add it in WebProcessProxy.

> 
> > Source/WebKit/UIProcess/WebProcessProxy.cpp:1779
> > +    speechRecognitionServer->mute();
> 
> I would write it as if (auto ...)
Comment 7 Sihui Liu 2021-01-05 09:45:12 PST 
Created attachment 417013 [details]
Patch for landing
Comment 8 EWS 2021-01-05 10:16:29 PST 
Committed r271154: <https://trac.webkit.org/changeset/271154>

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