...
Created attachment 419087 [details] Patch
Created attachment 419097 [details] Patch
Created attachment 419099 [details] Patch
Comment on attachment 419099 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419099&action=review > Source/WebCore/ChangeLog:9 > + user permission granting. We should be able to write a permission delegate that delays the user permission fo ever. Either through test runner or an API test. > Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp:95 > + if (requestIterator == m_requests.begin() || requestIterator == m_requests.end()) Why do we need both, I would think if (requestIterator == m_requests.end()) should work? > Source/WebKit/UIProcess/SpeechRecognitionServer.cpp:75 > + auto permissionRequestIdentifier = permissionManager->request(request.lang(), request.clientOrigin(), request.frameIdentifier(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto error) mutable { If we pass request directly to permissionManager, SpeechRecognitionPermissionRequest could take a WeakPtr<SpeechRecognitionRequest>. If its WeakPtr<SpeechRecognitionRequest> is null at the start of processing the permission request, we could just bail out. In that case, SpeechRecognitionServer::cancel would just need to destroy the request and would not need to call permissionManager->cancelRequest.
Comment on attachment 419099 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419099&action=review >> Source/WebCore/ChangeLog:9 >> + user permission granting. > > We should be able to write a permission delegate that delays the user permission fo ever. > Either through test runner or an API test. Hmm, after considering about this I think it's possible. Let me try >> Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp:95 >> + if (requestIterator == m_requests.begin() || requestIterator == m_requests.end()) > > Why do we need both, I would think if (requestIterator == m_requests.end()) should work? If requestIterator == m_requests.begin(), it means the request already starts (request being queued gets processed immediately if there is no request ahead). During requesting handling process, SpeechRecognitionPermissionManager would just look at the first request in queue and think it's the request being handled, so here we just let started request run to the end and it will fail in completion handler. >> Source/WebKit/UIProcess/SpeechRecognitionServer.cpp:75 >> + auto permissionRequestIdentifier = permissionManager->request(request.lang(), request.clientOrigin(), request.frameIdentifier(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto error) mutable { > > If we pass request directly to permissionManager, SpeechRecognitionPermissionRequest could take a WeakPtr<SpeechRecognitionRequest>. > If its WeakPtr<SpeechRecognitionRequest> is null at the start of processing the permission request, we could just bail out. > In that case, SpeechRecognitionServer::cancel would just need to destroy the request and would not need to call permissionManager->cancelRequest. That should work too, will change.
Created attachment 419239 [details] Patch
(In reply to Sihui Liu from comment #5) > Comment on attachment 419099 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=419099&action=review > > >> Source/WebCore/ChangeLog:9 > >> + user permission granting. > > > > We should be able to write a permission delegate that delays the user permission fo ever. > > Either through test runner or an API test. > > Hmm, after considering about this I think it's possible. Let me try Nope, current implementation also does not show user permission prompt if pending request is cancelled, because SpeechRecognitionPermissionManager remembers the user permission until top level document is changed. It's possible to present TCC prompt, if permission happens to be reset between requests, but it cannot be tested with WTR I guess. So the patch is mainly for reducing unnecessary workload of SpeechRecognitionPermissionManager and unnecessary TCC prompts I guess.
Comment on attachment 419239 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419239&action=review > Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp:227 > + auto clientOrigin = recognitionRequest->clientOrigin(); Maybe pass recognitionRequest as a parameter to requestUserPermission. That will remove the need for the assert.
Created attachment 419361 [details] Patch
ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!.
Created attachment 419367 [details] Patch for landing
commit-queue failed to commit attachment 419367 [details] to WebKit repository. To retry, please set cq+ flag again.
Created attachment 419410 [details] Patch for landing
Committed r272417: <https://trac.webkit.org/changeset/272417> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419410 [details].
<rdar://problem/74026953>