Changeset 271381 in webkit

Timestamp:

Jan 11, 2021 2:29:21 PM (18 months ago)

Author:

commit-queue@webkit.org

Message:

Make SpeechRecognition permission error more informative
​https://bugs.webkit.org/show_bug.cgi?id=220436

Patch by Sihui Liu <​sihui_liu@appe.com> on 2021-01-11
Reviewed by Youenn Fablet.

Source/WebKit:

Make SpeechRecognitionPermissionManager complete requests with an optional SpeechRecognitionError instead of a
boolean value.

• UIProcess/SpeechRecognitionPermissionManager.cpp:

(WebKit::SpeechRecognitionPermissionManager::~SpeechRecognitionPermissionManager):
(WebKit::SpeechRecognitionPermissionManager::request):
(WebKit::SpeechRecognitionPermissionManager::startProcessingRequest):
(WebKit::SpeechRecognitionPermissionManager::continueProcessingRequest):
(WebKit::SpeechRecognitionPermissionManager::completeCurrentRequest):
(WebKit::SpeechRecognitionPermissionManager::requestSpeechRecognitionServiceAccess):
(WebKit::SpeechRecognitionPermissionManager::requestMicrophoneAccess):
(WebKit::SpeechRecognitionPermissionManager::requestUserPermission):

• UIProcess/SpeechRecognitionPermissionManager.h:
• UIProcess/SpeechRecognitionPermissionRequest.h:

(WebKit::SpeechRecognitionPermissionRequest::create):
(WebKit::SpeechRecognitionPermissionRequest::complete):
(WebKit::SpeechRecognitionPermissionRequest::SpeechRecognitionPermissionRequest):

• UIProcess/SpeechRecognitionServer.cpp:

(WebKit::SpeechRecognitionServer::requestPermissionForRequest):

• UIProcess/SpeechRecognitionServer.h:
• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::requestSpeechRecognitionPermission):

• UIProcess/WebPageProxy.h:
• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::createSpeechRecognitionServer):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/SpeechRecognition.mm:

(TestWebKitAPI::TEST):

LayoutTests:

• fast/speechrecognition/permission-error-expected.txt:
• fast/speechrecognition/permission-error.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/speechrecognition/permission-error-expected.txt (modified) (1 diff)
• LayoutTests/fast/speechrecognition/permission-error.html (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp (modified) (8 diffs)
• Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.h (modified) (2 diffs)
• Source/WebKit/UIProcess/SpeechRecognitionPermissionRequest.h (modified) (3 diffs)
• Source/WebKit/UIProcess/SpeechRecognitionServer.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/SpeechRecognitionServer.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (3 diffs)
• Source/WebKit/UIProcess/WebProcessProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/SpeechRecognition.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-01-11  Sihui Liu  <sihui_liu@appe.com>
+
+        Make SpeechRecognition permission error more informative
+        https://bugs.webkit.org/show_bug.cgi?id=220436
+
+        Reviewed by Youenn Fablet.
+
+        * fast/speechrecognition/permission-error-expected.txt:
+        * fast/speechrecognition/permission-error.html:
+
 2021-01-11  Sihui Liu  <sihui_liu@appe.com>
 

trunk/LayoutTests/fast/speechrecognition/permission-error-expected.txt

@@ -7 +7 @@
 PASS recognition.start() did not throw exception.
 PASS event.error is "not-allowed"
-PASS event.message is "Permission check failed"
+PASS event.message is "User permission check has failed"
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/speechrecognition/permission-error.html

@@ -15 +15 @@
 recognition.onerror = (event) => {
     shouldBeEqualToString("event.error", "not-allowed");
-    shouldBeEqualToString("event.message", "Permission check failed");
+    shouldBeEqualToString("event.message", "User permission check has failed");
 
     finishJSTest();

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-01-11  Sihui Liu  <sihui_liu@appe.com>
+
+        Make SpeechRecognition permission error more informative
+        https://bugs.webkit.org/show_bug.cgi?id=220436
+
+        Reviewed by Youenn Fablet.
+
+        Make SpeechRecognitionPermissionManager complete requests with an optional SpeechRecognitionError instead of a 
+        boolean value.
+
+        * UIProcess/SpeechRecognitionPermissionManager.cpp:
+        (WebKit::SpeechRecognitionPermissionManager::~SpeechRecognitionPermissionManager):
+        (WebKit::SpeechRecognitionPermissionManager::request):
+        (WebKit::SpeechRecognitionPermissionManager::startProcessingRequest):
+        (WebKit::SpeechRecognitionPermissionManager::continueProcessingRequest):
+        (WebKit::SpeechRecognitionPermissionManager::completeCurrentRequest):
+        (WebKit::SpeechRecognitionPermissionManager::requestSpeechRecognitionServiceAccess):
+        (WebKit::SpeechRecognitionPermissionManager::requestMicrophoneAccess):
+        (WebKit::SpeechRecognitionPermissionManager::requestUserPermission):
+        * UIProcess/SpeechRecognitionPermissionManager.h:
+        * UIProcess/SpeechRecognitionPermissionRequest.h:
+        (WebKit::SpeechRecognitionPermissionRequest::create):
+        (WebKit::SpeechRecognitionPermissionRequest::complete):
+        (WebKit::SpeechRecognitionPermissionRequest::SpeechRecognitionPermissionRequest):
+        * UIProcess/SpeechRecognitionServer.cpp:
+        (WebKit::SpeechRecognitionServer::requestPermissionForRequest):
+        * UIProcess/SpeechRecognitionServer.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::requestSpeechRecognitionPermission):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::createSpeechRecognitionServer):
+
 2021-01-11  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.cpp

@@ -73 +73 @@
 {
     for (auto& request : m_requests)
-        request->complete(SpeechRecognitionPermissionDecision::Deny);
+        request->complete(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Permission manager has exited"_s });
 }
     
-void SpeechRecognitionPermissionManager::request(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&& completiontHandler)
+void SpeechRecognitionPermissionManager::request(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(Optional<SpeechRecognitionError>&&)>&& completiontHandler)
 {
     m_requests.append(SpeechRecognitionPermissionRequest::create(lang, origin, WTFMove(completiontHandler)));
@@ -95 +95 @@
 #if PLATFORM(COOCA)
     if (!checkSandboxRequirementForType(MediaPermissionType::Audio)) {
-        completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
-        return;
-    }
-#endif
-
-    // TCC status may have changed between requests.
-    m_microphoneCheck = computeMicrophoneAccess();
-    m_speechRecognitionServiceCheck = computeSpeechRecognitionServiceAccess();
+        completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Sandbox check has failed"_s });
+        return;
+    }
+#endif
 
     m_page.syncIfMockDevicesEnabledChanged();
-    bool mockCaptureDevicesEnabled = m_page.preferences().mockCaptureDevicesEnabled();
-    if (mockCaptureDevicesEnabled) {
+    if (m_page.preferences().mockCaptureDevicesEnabled()) {
         m_microphoneCheck = CheckResult::Granted;
         m_speechRecognitionServiceCheck = CheckResult::Granted;
-    }
+    } else {
+        // TCC status may have changed between requests.
+        m_microphoneCheck = computeMicrophoneAccess();
+        if (m_microphoneCheck == CheckResult::Denied) {
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Microphone permission check has failed"_s });
+            return;
+        }
+
+        m_speechRecognitionServiceCheck = computeSpeechRecognitionServiceAccess();
+        if (m_speechRecognitionServiceCheck == CheckResult::Denied) {
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::ServiceNotAllowed, "Speech recognition service permission check has failed"_s });
+            return;
+        }
 
 #if HAVE(SPEECHRECOGNIZER)
-    if (!mockCaptureDevicesEnabled && m_speechRecognitionServiceCheck != CheckResult::Denied) {
-        // Speech recognition service can be unavailable when user does not enable dictation in system settings.
-        // Let's avoid prompting user in that case.
-        if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->lang()))
-            m_speechRecognitionServiceCheck = CheckResult::Denied;
-    }
-#endif
-
-    if (m_microphoneCheck == CheckResult::Denied || m_speechRecognitionServiceCheck == CheckResult::Denied) {
-        completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
-        return;
+        if (!checkSpeechRecognitionServiceAvailability(m_requests.first()->lang())) {
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::ServiceNotAllowed, "Speech recognition service is not available"_s });
+            return;
+        }
+#endif
     }
 
@@ -135 +136 @@
 
     if (m_userPermissionCheck == CheckResult::Denied) {
-        completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
+        completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "User permission check has failed"_s });
         return;
     }
@@ -163 +164 @@
 
     if (!m_page.isViewVisible()) {
-        completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
-        return;
-    }
-
-    completeCurrentRequest(SpeechRecognitionPermissionDecision::Grant);
-}
-
-void SpeechRecognitionPermissionManager::completeCurrentRequest(SpeechRecognitionPermissionDecision decision)
+        completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Page is not visible to user" });
+        return;
+    }
+
+    completeCurrentRequest();
+}
+
+void SpeechRecognitionPermissionManager::completeCurrentRequest(Optional<SpeechRecognitionError>&& error)
 {
     ASSERT(!m_requests.isEmpty());
     auto currentRequest = m_requests.takeFirst();
-    currentRequest->complete(decision);
+    currentRequest->complete(WTFMove(error));
 
     startNextRequest();
@@ -190 +191 @@
         m_speechRecognitionServiceCheck = authorized ? CheckResult::Granted : CheckResult::Denied;
         if (m_speechRecognitionServiceCheck == CheckResult::Denied) {
-            completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::ServiceNotAllowed, "Speech recognition service permission check has failed"_s });
             return;
         }
@@ -210 +211 @@
         m_microphoneCheck = authorized ? CheckResult::Granted : CheckResult::Denied;
         if (m_microphoneCheck == CheckResult::Denied) {
-            completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Microphone permission check has failed"_s });
             return;
         }
@@ -232 +233 @@
         m_userPermissionCheck = granted ? CheckResult::Granted : CheckResult::Denied;
         if (m_userPermissionCheck == CheckResult::Denied) {
-            completeCurrentRequest(SpeechRecognitionPermissionDecision::Deny);
+            completeCurrentRequest(SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "User permission check has failed"_s });
             return;
         }
@@ -240 +241 @@
     m_page.uiClient().decidePolicyForSpeechRecognitionPermissionRequest(m_page, API::SecurityOrigin::create(topOrigin.get()).get(), WTFMove(decisionHandler));
 }
-
-
 
 void SpeechRecognitionPermissionManager::decideByDefaultAction(const WebCore::SecurityOrigin& origin, CompletionHandler<void(bool)>&& completionHandler)

trunk/Source/WebKit/UIProcess/SpeechRecognitionPermissionManager.h

@@ -40 +40 @@
     explicit SpeechRecognitionPermissionManager(WebPageProxy&);
     ~SpeechRecognitionPermissionManager();
-    void request(const String& lang, const WebCore::ClientOrigin&, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&&);
+    void request(const String& lang, const WebCore::ClientOrigin&, CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)>&&);
 
     void decideByDefaultAction(const WebCore::SecurityOrigin&, CompletionHandler<void(bool)>&&);
@@ -49 +49 @@
     void startProcessingRequest();
     void continueProcessingRequest();
-    void completeCurrentRequest(SpeechRecognitionPermissionDecision);
+    void completeCurrentRequest(Optional<WebCore::SpeechRecognitionError>&& = WTF::nullopt);
     void requestMicrophoneAccess();
     void requestSpeechRecognitionServiceAccess();

trunk/Source/WebKit/UIProcess/SpeechRecognitionPermissionRequest.h

@@ -28 +28 @@
 #include "APIObject.h"
 #include <WebCore/ClientOrigin.h>
+#include <WebCore/SpeechRecognitionError.h>
 #include <wtf/CompletionHandler.h>
 
 namespace WebKit {
 
-enum class SpeechRecognitionPermissionDecision : bool { Deny, Grant };
-
 class SpeechRecognitionPermissionRequest : public RefCounted<SpeechRecognitionPermissionRequest> {
 public:
-    static Ref<SpeechRecognitionPermissionRequest> create(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&& completionHandler)
+    static Ref<SpeechRecognitionPermissionRequest> create(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)>&& completionHandler)
     {
         return adoptRef(*new SpeechRecognitionPermissionRequest(lang, origin, WTFMove(completionHandler)));
     }
 
-    void complete(SpeechRecognitionPermissionDecision decision)
+    void complete(Optional<WebCore::SpeechRecognitionError>&& error)
     {
         auto completionHandler = std::exchange(m_completionHandler, { });
-        completionHandler(decision);
+        completionHandler(WTFMove(error));
     }
 
@@ -51 +50 @@
 
 private:
-    SpeechRecognitionPermissionRequest(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&& completionHandler)
+    SpeechRecognitionPermissionRequest(const String& lang, const WebCore::ClientOrigin& origin, CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)>&& completionHandler)
         : m_lang(lang)
         , m_origin(origin)
@@ -59 +58 @@
     String m_lang;
     WebCore::ClientOrigin m_origin;
-    CompletionHandler<void(SpeechRecognitionPermissionDecision)> m_completionHandler;
+    CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)> m_completionHandler;
 };
 

trunk/Source/WebKit/UIProcess/SpeechRecognitionServer.cpp

@@ -65 +65 @@
 void SpeechRecognitionServer::requestPermissionForRequest(WebCore::SpeechRecognitionRequest& request)
 {
-    m_permissionChecker(request.lang(), request.clientOrigin(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto decision) mutable {
+    m_permissionChecker(request.lang(), request.clientOrigin(), [this, weakThis = makeWeakPtr(this), weakRequest = makeWeakPtr(request)](auto error) mutable {
         if (!weakThis)
             return;
@@ -73 +73 @@
 
         auto identifier = weakRequest->clientIdentifier();
-        if (decision == SpeechRecognitionPermissionDecision::Deny) {
+        if (error) {
             m_requests.remove(identifier);
-            auto error = WebCore::SpeechRecognitionError { WebCore::SpeechRecognitionErrorType::NotAllowed, "Permission check failed"_s };
-            sendUpdate(identifier, WebCore::SpeechRecognitionUpdateType::Error, error);
+            sendUpdate(identifier, WebCore::SpeechRecognitionUpdateType::Error, WTFMove(error));
             return;
         }

trunk/Source/WebKit/UIProcess/SpeechRecognitionServer.h

@@ -44 +44 @@
 
 class WebProcessProxy;
-enum class SpeechRecognitionPermissionDecision : bool;
 
 using SpeechRecognitionServerIdentifier = WebCore::PageIdentifier;
-using SpeechRecognitionPermissionChecker = Function<void(const String&, const WebCore::ClientOrigin&, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&&)>;
+using SpeechRecognitionPermissionChecker = Function<void(const String&, const WebCore::ClientOrigin&, CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)>&&)>;
 using SpeechRecognitionCheckIfMockSpeechRecognitionEnabled = Function<bool()>;
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -10302 +10302 @@
 }
 
-void WebPageProxy::requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin& clientOrigin, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&& completionHandler)
+void WebPageProxy::requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin& clientOrigin, CompletionHandler<void(Optional<SpeechRecognitionError>&&)>&& completionHandler)
 {
     if (!m_speechRecognitionPermissionManager)

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -265 +265 @@
 struct SecurityOriginData;
 struct ShareData;
+struct SpeechRecognitionError;
 struct TextAlternativeWithRange;
 struct TextCheckingResult;
@@ -368 +369 @@
 enum class NegotiatedLegacyTLS : bool;
 enum class ProcessSwapRequestedByClient : bool;
-enum class SpeechRecognitionPermissionDecision : bool;
 enum class UndoOrRedo : bool;
 enum class WebContentMode : uint8_t;
@@ -1811 +1811 @@
     size_t suspendMediaPlaybackCounter() { return m_suspendMediaPlaybackCounter; }
 
-    void requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin&, CompletionHandler<void(SpeechRecognitionPermissionDecision)>&&);
+    void requestSpeechRecognitionPermission(const String& lang, const WebCore::ClientOrigin&, CompletionHandler<void(Optional<WebCore::SpeechRecognitionError>&&)>&&);
     void requestSpeechRecognitionPermissionByDefaultAction(const WebCore::SecurityOrigin&, CompletionHandler<void(bool)>&&);
 

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -1725 +1725 @@
     auto permissionChecker = [weakPage = makeWeakPtr(targetPage)](auto& lang, auto& origin, auto&& completionHandler) mutable {
         if (!weakPage) {
-            completionHandler(SpeechRecognitionPermissionDecision::Deny);
+            completionHandler(WebCore::SpeechRecognitionError { SpeechRecognitionErrorType::NotAllowed, "Page no longer exists"_s });
             return;
         }

trunk/Tools/ChangeLog

@@ +1 @@
+2021-01-11  Sihui Liu  <sihui_liu@appe.com>
+
+        Make SpeechRecognition permission error more informative
+        https://bugs.webkit.org/show_bug.cgi?id=220436
+
+        Reviewed by Youenn Fablet.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/SpeechRecognition.mm:
+        (TestWebKitAPI::TEST):
+
 2021-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SpeechRecognition.mm

@@ -120 +120 @@
     [webView loadTestPageNamed:@"speechrecognition-user-permission-persistence"];
     TestWebKitAPI::Util::run(&receivedScriptMessage);
-    EXPECT_WK_STREQ(@"Error: not-allowed - Permission check failed", [lastScriptMessage body]);
+    EXPECT_WK_STREQ(@"Error: not-allowed - User permission check has failed", [lastScriptMessage body]);
     EXPECT_TRUE(permissionRequested);
 
@@ -128 +128 @@
     [webView stringByEvaluatingJavaScript:@"start()"];
     TestWebKitAPI::Util::run(&receivedScriptMessage);
-    EXPECT_WK_STREQ(@"Error: not-allowed - Permission check failed", [lastScriptMessage body]);
+    EXPECT_WK_STREQ(@"Error: not-allowed - User permission check has failed", [lastScriptMessage body]);
     EXPECT_FALSE(permissionRequested);
 
@@ -232 +232 @@
     [webView evaluateJavaScript:@"start()" completionHandler:nil];
     TestWebKitAPI::Util::run(&receivedScriptMessage);
-    EXPECT_WK_STREQ(@"Error: not-allowed - Permission check failed", [lastScriptMessage body]);
+    EXPECT_WK_STREQ(@"Error: not-allowed - Page is not visible to user", [lastScriptMessage body]);
 }