Skip to content

Commit ccae124

Browse files
szewaiwebkit-commit-queue
szewai
authored and
webkit-commit-queue
committed
SpeechRecognitionPermissionManager should not handle requests that are already cancelled
https://bugs.webkit.org/show_bug.cgi?id=221296 Patch by Sihui Liu <sihui_liu@appe.com> on 2021-02-05 Reviewed by Youenn Fablet. It is possible that client asks to stop/abort pending requests, which are waiting for permission check to be done. In our current implementation, SpeechRecognitionPermissionManager would perform permission checks for these cancelled requests, which is unnecessary and may cause extra prompts. We should let SpeechRecognitionPermissionManager check if request is still valid before checking permissions. No test is added as this behavior change is not observable without resetting the TCC permission between requests. * UIProcess/SpeechRecognitionPermissionManager.cpp: (WebKit::SpeechRecognitionPermissionManager::request): (WebKit::SpeechRecognitionPermissionManager::startNextRequest): (WebKit::SpeechRecognitionPermissionManager::startProcessingRequest): (WebKit::SpeechRecognitionPermissionManager::continueProcessingRequest): (WebKit::SpeechRecognitionPermissionManager::requestUserPermission): * UIProcess/SpeechRecognitionPermissionManager.h: * UIProcess/SpeechRecognitionPermissionRequest.h: (WebKit::SpeechRecognitionPermissionRequest::create): (WebKit::SpeechRecognitionPermissionRequest::~SpeechRecognitionPermissionRequest): (WebKit::SpeechRecognitionPermissionRequest::request): (WebKit::SpeechRecognitionPermissionRequest::SpeechRecognitionPermissionRequest): (WebKit::SpeechRecognitionPermissionRequest::origin const): Deleted. (WebKit::SpeechRecognitionPermissionRequest::lang const): Deleted. (WebKit::SpeechRecognitionPermissionRequest::frameIdentifier const): Deleted. * UIProcess/SpeechRecognitionServer.cpp: (WebKit::SpeechRecognitionServer::requestPermissionForRequest): (WebKit::SpeechRecognitionServer::sendUpdate): * UIProcess/SpeechRecognitionServer.h: * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::requestSpeechRecognitionPermission): * UIProcess/WebPageProxy.h: * UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::createSpeechRecognitionServer): Canonical link: https://commits.webkit.org/233741@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272417 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 1543357 commit ccae124

9 files changed

Lines changed: 82 additions & 37 deletions

‎Source/WebKit/ChangeLog‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,42 @@
1+
2021-02-05 Sihui Liu <sihui_liu@appe.com>
2+
3+
SpeechRecognitionPermissionManager should not handle requests that are already cancelled
4+
https://bugs.webkit.org/show_bug.cgi?id=221296
5+
6+
Reviewed by Youenn Fablet.
7+
8+
It is possible that client asks to stop/abort pending requests, which are waiting for permission check to be
9+
done. In our current implementation, SpeechRecognitionPermissionManager would perform permission checks for
10+
these cancelled requests, which is unnecessary and may cause extra prompts. We should let
11+
SpeechRecognitionPermissionManager check if request is still valid before checking permissions.
12+
13+
No test is added as this behavior change is not observable without resetting the TCC permission between requests.
14+
15+
* UIProcess/SpeechRecognitionPermissionManager.cpp:
16+
(WebKit::SpeechRecognitionPermissionManager::request):
17+
(WebKit::SpeechRecognitionPermissionManager::startNextRequest):
18+
(WebKit::SpeechRecognitionPermissionManager::startProcessingRequest):
19+
(WebKit::SpeechRecognitionPermissionManager::continueProcessingRequest):
20+
(WebKit::SpeechRecognitionPermissionManager::requestUserPermission):
21+
* UIProcess/SpeechRecognitionPermissionManager.h:
22+
* UIProcess/SpeechRecognitionPermissionRequest.h:
23+
(WebKit::SpeechRecognitionPermissionRequest::create):
24+
(WebKit::SpeechRecognitionPermissionRequest::~SpeechRecognitionPermissionRequest):
25+
(WebKit::SpeechRecognitionPermissionRequest::request):
26+
(WebKit::SpeechRecognitionPermissionRequest::SpeechRecognitionPermissionRequest):
27+
(WebKit::SpeechRecognitionPermissionRequest::origin const): Deleted.
28+
(WebKit::SpeechRecognitionPermissionRequest::lang const): Deleted.
29+
(WebKit::SpeechRecognitionPermissionRequest::frameIdentifier const): Deleted.
30+
* UIProcess/SpeechRecognitionServer.cpp:
31+
(WebKit::SpeechRecognitionServer::requestPermissionForRequest):
32+
(WebKit::SpeechRecognitionServer::sendUpdate):
33+
* UIProcess/SpeechRecognitionServer.h:
34+
* UIProcess/WebPageProxy.cpp:
35+
(WebKit::WebPageProxy::requestSpeechRecognitionPermission):
36+
* UIProcess/WebPageProxy.h:
37+
* UIProcess/WebProcessProxy.cpp:
38+
(WebKit::WebProcessProxy::createSpeechRecognitionServer):
39+
140
2021-02-05 Chris Dumez <cdumez@apple.com>
241

342
[GPUProcess][iOS] Audio is lost after media playback recovers from the GPUProcess crash

‎Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,18 @@ SpeechRecognitionPermissionManager::~SpeechRecognitionPermissionManager()
7575
request->complete(WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::NotAllowed, "Permission manager has exited"_s });
7676
}
7777

78-
void SpeechRecognitionPermissionManager::request(const String& lang, const WebCore::ClientOrigin& origin, WebCore::FrameIdentifier frameIdentifier, SpeechRecognitionPermissionRequestCallback&& completiontHandler)
78+
void SpeechRecognitionPermissionManager::request(WebCore::SpeechRecognitionRequest& request, SpeechRecognitionPermissionRequestCallback&& completiontHandler)
7979
{
80-
m_requests.append(SpeechRecognitionPermissionRequest::create(lang, origin, frameIdentifier, WTFMove(completiontHandler)));
80+
m_requests.append(SpeechRecognitionPermissionRequest::create(request, WTFMove(completiontHandler)));
8181
if (m_requests.size() == 1)
8282
startNextRequest();
8383
}
8484

8585
void SpeechRecognitionPermissionManager::startNextRequest()
8686
{
87+
while (!m_requests.isEmpty() && !m_requests.first()->request())
88+
m_requests.removeFirst();
89+
8790
if (m_requests.isEmpty())
8891
return;
8992

@@ -118,7 +121,7 @@ void SpeechRecognitionPermissionManager::startProcessingRequest()
118121
}
119122

120123
#if HAVE(SPEECHRECOGNIZER)
121-
if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->lang())) {
124+
if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->request()->lang())) {
122125
completeCurrentRequest(WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::ServiceNotAllowed, "Speech recognition service is not available"_s });
123126
return;
124127
}
@@ -135,6 +138,13 @@ void SpeechRecognitionPermissionManager::startProcessingRequest()
135138

136139
void SpeechRecognitionPermissionManager::continueProcessingRequest()
137140
{
141+
ASSERT(!m_requests.isEmpty());
142+
auto recognitionRequest = m_requests.first()->request();
143+
if (!recognitionRequest) {
144+
completeCurrentRequest();
145+
return;
146+
}
147+
138148
if (m_speechRecognitionServiceCheck == CheckResult::Unknown) {
139149
requestSpeechRecognitionServiceAccess();
140150
return;
@@ -148,13 +158,13 @@ void SpeechRecognitionPermissionManager::continueProcessingRequest()
148158
ASSERT(m_microphoneCheck == CheckResult::Granted);
149159

150160
if (m_userPermissionCheck == CheckResult::Unknown) {
151-
requestUserPermission();
161+
requestUserPermission(*recognitionRequest);
152162
return;
153163
}
154164
ASSERT(m_userPermissionCheck == CheckResult::Granted);
155165

156166
if (!m_page.isViewVisible()) {
157-
completeCurrentRequest(WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::NotAllowed, "Page is not visible to user" });
167+
completeCurrentRequest(WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::NotAllowed, "Page is not visible to user"_s });
158168
return;
159169
}
160170

@@ -210,12 +220,9 @@ void SpeechRecognitionPermissionManager::requestMicrophoneAccess()
210220
#endif
211221
}
212222

213-
void SpeechRecognitionPermissionManager::requestUserPermission()
223+
void SpeechRecognitionPermissionManager::requestUserPermission(WebCore::SpeechRecognitionRequest& recognitionRequest)
214224
{
215-
ASSERT(!m_requests.isEmpty());
216-
217-
auto& currentRequest = m_requests.first();
218-
auto clientOrigin = currentRequest->origin();
225+
auto clientOrigin = recognitionRequest.clientOrigin();
219226
auto requestingOrigin = clientOrigin.clientOrigin.securityOrigin();
220227
auto topOrigin = clientOrigin.topOrigin.securityOrigin();
221228
auto decisionHandler = [this, weakThis = makeWeakPtr(*this)](bool granted) {
@@ -230,7 +237,7 @@ void SpeechRecognitionPermissionManager::requestUserPermission()
230237

231238
continueProcessingRequest();
232239
};
233-
m_page.requestUserMediaPermissionForSpeechRecognition(currentRequest->frameIdentifier(), requestingOrigin, topOrigin, WTFMove(decisionHandler));
240+
m_page.requestUserMediaPermissionForSpeechRecognition(recognitionRequest.frameIdentifier(), requestingOrigin, topOrigin, WTFMove(decisionHandler));
234241
}
235242

236243
void SpeechRecognitionPermissionManager::decideByDefaultAction(const WebCore::SecurityOrigin& origin, CompletionHandler<void(bool)>&& completionHandler)

‎Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class SpeechRecognitionPermissionManager : public CanMakeWeakPtr<SpeechRecogniti
3939
enum class CheckResult { Denied, Granted, Unknown };
4040
explicit SpeechRecognitionPermissionManager(WebPageProxy&);
4141
~SpeechRecognitionPermissionManager();
42-
void request(const String& lang, const WebCore::ClientOrigin&, WebCore::FrameIdentifier, SpeechRecognitionPermissionRequestCallback&&);
42+
void request(WebCore::SpeechRecognitionRequest&, SpeechRecognitionPermissionRequestCallback&&);
4343

4444
void decideByDefaultAction(const WebCore::SecurityOrigin&, CompletionHandler<void(bool)>&&);
4545
WebPageProxy& page() { return m_page; }
@@ -51,7 +51,7 @@ class SpeechRecognitionPermissionManager : public CanMakeWeakPtr<SpeechRecogniti
5151
void completeCurrentRequest(Optional<WebCore::SpeechRecognitionError>&& = WTF::nullopt);
5252
void requestMicrophoneAccess();
5353
void requestSpeechRecognitionServiceAccess();
54-
void requestUserPermission();
54+
void requestUserPermission(WebCore::SpeechRecognitionRequest& request);
5555

5656
WebPageProxy& m_page;
5757
Deque<Ref<SpeechRecognitionPermissionRequest>> m_requests;

‎Source/WebKit/UIProcess/SpeechRecognitionPermissionRequest.h‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <WebCore/ClientOrigin.h>
3030
#include <WebCore/FrameIdentifier.h>
3131
#include <WebCore/SpeechRecognitionError.h>
32+
#include <WebCore/SpeechRecognitionRequest.h>
3233
#include <wtf/CompletionHandler.h>
3334

3435
namespace WebKit {
@@ -37,9 +38,15 @@ using SpeechRecognitionPermissionRequestCallback = CompletionHandler<void(Option
3738

3839
class SpeechRecognitionPermissionRequest : public RefCounted<SpeechRecognitionPermissionRequest> {
3940
public:
40-
static Ref<SpeechRecognitionPermissionRequest> create(const String& lang, const WebCore::ClientOrigin& origin, WebCore::FrameIdentifier frameIdentifier, SpeechRecognitionPermissionRequestCallback&& completionHandler)
41+
static Ref<SpeechRecognitionPermissionRequest> create(WebCore::SpeechRecognitionRequest& request, SpeechRecognitionPermissionRequestCallback&& completionHandler)
4142
{
42-
return adoptRef(*new SpeechRecognitionPermissionRequest(lang, origin, frameIdentifier, WTFMove(completionHandler)));
43+
return adoptRef(*new SpeechRecognitionPermissionRequest(request, WTFMove(completionHandler)));
44+
}
45+
46+
~SpeechRecognitionPermissionRequest()
47+
{
48+
if (m_completionHandler)
49+
m_completionHandler(WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::NotAllowed, "Request is cancelled"_s });
4350
}
4451

4552
void complete(Optional<WebCore::SpeechRecognitionError>&& error)
@@ -48,21 +55,16 @@ class SpeechRecognitionPermissionRequest : public RefCounted<SpeechRecognitionPe
4855
completionHandler(WTFMove(error));
4956
}
5057

51-
const WebCore::ClientOrigin& origin() const { return m_origin; }
52-
const String& lang() const { return m_lang; }
53-
WebCore::FrameIdentifier frameIdentifier() const { return m_frameIdentifier; }
58+
WebCore::SpeechRecognitionRequest* request() { return m_request.get(); }
5459

5560
private:
56-
SpeechRecognitionPermissionRequest(const String& lang, const WebCore::ClientOrigin& origin, WebCore::FrameIdentifier frameIdentifier, SpeechRecognitionPermissionRequestCallback&& completionHandler)
57-
: m_lang(lang)
58-
, m_origin(origin)
59-
, m_frameIdentifier(frameIdentifier)
61+
SpeechRecognitionPermissionRequest(WebCore::SpeechRecognitionRequest& request, SpeechRecognitionPermissionRequestCallback&& completionHandler)
62+
: m_request(makeWeakPtr(request))
6063
, m_completionHandler(WTFMove(completionHandler))
6164
{ }
6265

63-
String m_lang;
64-
WebCore::ClientOrigin m_origin;
65-
WebCore::FrameIdentifier m_frameIdentifier;
66+
67+
WeakPtr<WebCore::SpeechRecognitionRequest> m_request;
6668
SpeechRecognitionPermissionRequestCallback m_completionHandler;
6769
};
6870

‎Source/WebKit/UIProcess/SpeechRecognitionServer.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,8 @@ void SpeechRecognitionServer::start(WebCore::SpeechRecognitionConnectionClientId
6363

6464
void SpeechRecognitionServer::requestPermissionForRequest(WebCore::SpeechRecognitionRequest& request)
6565
{
66-
m_permissionChecker(request.lang(), request.clientOrigin(), request.frameIdentifier(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto error) mutable {
67-
if (!weakThis)
68-
return;
69-
70-
if (!weakRequest)
66+
m_permissionChecker(request, [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto error) mutable {
67+
if (!weakThis || !weakRequest)
7168
return;
7269

7370
auto identifier = weakRequest->clientIdentifier();
@@ -159,7 +156,6 @@ void SpeechRecognitionServer::sendUpdate(WebCore::SpeechRecognitionConnectionCli
159156

160157
void SpeechRecognitionServer::sendUpdate(const WebCore::SpeechRecognitionUpdate& update)
161158
{
162-
WTFLogAlways("[%p]SpeechRecognitionServer::sendUpdate update.type[%d], update.clientIdentifier[%" PRIu64 "]", this, update.type(), update.clientIdentifier().toUInt64());
163159
send(Messages::WebSpeechRecognitionConnection::DidReceiveUpdate(update));
164160
}
165161

‎Source/WebKit/UIProcess/SpeechRecognitionServer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ namespace WebKit {
4646
class WebProcessProxy;
4747

4848
using SpeechRecognitionServerIdentifier = WebCore::PageIdentifier;
49-
using SpeechRecognitionPermissionChecker = Function<void(const String&, const WebCore::ClientOrigin&, WebCore::FrameIdentifier, SpeechRecognitionPermissionRequestCallback&&)>;
49+
using SpeechRecognitionPermissionChecker = Function<void(WebCore::SpeechRecognitionRequest&, SpeechRecognitionPermissionRequestCallback&&)>;
5050
using SpeechRecognitionCheckIfMockSpeechRecognitionEnabled = Function<bool()>;
5151

5252
class SpeechRecognitionServer : public CanMakeWeakPtr<SpeechRecognitionServer>, public IPC::MessageReceiver, private IPC::MessageSender {

‎Source/WebKit/UIProcess/WebPageProxy.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10406,12 +10406,12 @@ void WebPageProxy::dispatchActivityStateUpdateForTesting()
1040610406
});
1040710407
}
1040810408

10409-
void WebPageProxy::requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin& clientOrigin, WebCore::FrameIdentifier frameIdentifier, CompletionHandler<void(Optional<SpeechRecognitionError>&&)>&& completionHandler)
10409+
void WebPageProxy::requestSpeechRecognitionPermission(WebCore::SpeechRecognitionRequest& request, CompletionHandler<void(Optional<SpeechRecognitionError>&&)>&& completionHandler)
1041010410
{
1041110411
if (!m_speechRecognitionPermissionManager)
1041210412
m_speechRecognitionPermissionManager = makeUnique<SpeechRecognitionPermissionManager>(*this);
1041310413

10414-
m_speechRecognitionPermissionManager->request(lang, clientOrigin, frameIdentifier, WTFMove(completionHandler));
10414+
m_speechRecognitionPermissionManager->request(request, WTFMove(completionHandler));
1041510415
}
1041610416

1041710417
void WebPageProxy::requestSpeechRecognitionPermissionByDefaultAction(const WebCore::SecurityOrigin& origin, CompletionHandler<void(bool)>&& completionHandler)

‎Source/WebKit/UIProcess/WebPageProxy.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ class ProtectionSpace;
227227
class RunLoopObserver;
228228
class SelectionData;
229229
class SharedBuffer;
230+
class SpeechRecognitionRequest;
230231
class TextIndicator;
231232
class ValidationBubble;
232233

@@ -1825,7 +1826,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
18251826
void setMediaCaptureReportingDelay(Seconds captureReportingDelay) { m_mediaCaptureReportingDelay = captureReportingDelay; }
18261827
size_t suspendMediaPlaybackCounter() { return m_suspendMediaPlaybackCounter; }
18271828

1828-
void requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin&, WebCore::FrameIdentifier, SpeechRecognitionPermissionRequestCallback&&);
1829+
void requestSpeechRecognitionPermission(WebCore::SpeechRecognitionRequest&, SpeechRecognitionPermissionRequestCallback&&);
18291830
void requestSpeechRecognitionPermissionByDefaultAction(const WebCore::SecurityOrigin&, CompletionHandler<void(bool)>&&);
18301831
void requestUserMediaPermissionForSpeechRecognition(WebCore::FrameIdentifier, const WebCore::SecurityOrigin&, const WebCore::SecurityOrigin&, CompletionHandler<void(bool)>&&);
18311832

‎Source/WebKit/UIProcess/WebProcessProxy.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,13 +1722,13 @@ void WebProcessProxy::createSpeechRecognitionServer(SpeechRecognitionServerIdent
17221722

17231723
ASSERT(!m_speechRecognitionServerMap.contains(identifier));
17241724
auto& speechRecognitionServer = m_speechRecognitionServerMap.add(identifier, nullptr).iterator->value;
1725-
auto permissionChecker = [weakPage = makeWeakPtr(targetPage)](auto& lang, auto& origin, auto frameIdentifier, auto&& completionHandler) mutable {
1725+
auto permissionChecker = [weakPage = makeWeakPtr(targetPage)](auto& request, auto&& completionHandler) mutable {
17261726
if (!weakPage) {
17271727
completionHandler(WebCore::SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Page no longer exists"_s });
17281728
return;
17291729
}
17301730

1731-
weakPage->requestSpeechRecognitionPermission(lang, origin, frameIdentifier, WTFMove(completionHandler));
1731+
weakPage->requestSpeechRecognitionPermission(request, WTFMove(completionHandler));
17321732
};
17331733
auto checkIfMockCaptureDevicesEnabled = [weakPage = makeWeakPtr(targetPage)]() {
17341734
return weakPage && weakPage->preferences().mockCaptureDevicesEnabled();

0 commit comments

Comments
 (0)