[MediaStream] User should not be prompted again after denying getDisplayMedia request
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 4 Nov 2018 16:04:19 +0000 (16:04 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 4 Nov 2018 16:04:19 +0000 (16:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191227
<rdar://problem/45784512>

Reviewed by Youenn Fablet.

Source/WebKit:

* UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
(WebKit::UserMediaPermissionRequestManagerProxy::userMediaAccessWasDenied):
(WebKit::UserMediaPermissionRequestManagerProxy::wasRequestDenied):
(WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame):
* UIProcess/UserMediaPermissionRequestManagerProxy.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/GetDisplayMedia.mm:
(-[GetDisplayMediaUIDelegate _webView:requestUserMediaAuthorizationForDevices:url:mainFrameURL:decisionHandler:]):
(TestWebKitAPI::GetDisplayMediaTest::promptForCapture):
(TestWebKitAPI::TEST_F):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237784 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/GetDisplayMedia.mm

index 77d30cd..222d6a0 100644 (file)
@@ -1,3 +1,17 @@
+2018-11-04  Eric Carlson  <eric.carlson@apple.com>
+
+        [MediaStream] User should not be prompted again after denying getDisplayMedia request
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        <rdar://problem/45784512>
+
+        Reviewed by Youenn Fablet.
+
+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
+        (WebKit::UserMediaPermissionRequestManagerProxy::userMediaAccessWasDenied):
+        (WebKit::UserMediaPermissionRequestManagerProxy::wasRequestDenied):
+        (WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame):
+        * UIProcess/UserMediaPermissionRequestManagerProxy.h:
+
 2018-11-03  Alex Christensen  <achristensen@webkit.org>
 
         Mac production builds should sign the network process xpc service with entitlements
index 62774c9..af0ccf4 100644 (file)
@@ -133,7 +133,7 @@ void UserMediaPermissionRequestManagerProxy::userMediaAccessWasDenied(uint64_t u
         return;
 
     if (reason == UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied)
-        m_deniedRequests.append(DeniedRequest { request->mainFrameID(), request->userMediaDocumentSecurityOrigin(), request->topLevelDocumentSecurityOrigin(), request->requiresAudioCapture(), request->requiresVideoCapture() });
+        m_deniedRequests.append(DeniedRequest { request->mainFrameID(), request->userMediaDocumentSecurityOrigin(), request->topLevelDocumentSecurityOrigin(), request->requiresAudioCapture(), request->requiresVideoCapture(), request->requiresDisplayCapture() });
 
     denyRequest(userMediaID, reason, emptyString());
 }
@@ -213,7 +213,7 @@ const UserMediaPermissionRequestProxy* UserMediaPermissionRequestManagerProxy::s
     return nullptr;
 }
 
-bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrameID, const SecurityOrigin& userMediaDocumentOrigin, const SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo)
+bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrameID, const SecurityOrigin& userMediaDocumentOrigin, const SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo, bool needsScreenCapture)
 {
     for (const auto& deniedRequest : m_deniedRequests) {
         if (!deniedRequest.userMediaDocumentOrigin->isSameSchemeHostPort(userMediaDocumentOrigin))
@@ -226,6 +226,8 @@ bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrame
             return true;
         if (deniedRequest.isVideoDenied && needsVideo)
             return true;
+        if (deniedRequest.isScreenCaptureDenied && needsScreenCapture)
+            return true;
     }
     return false;
 }
@@ -259,6 +261,26 @@ void UserMediaPermissionRequestManagerProxy::scheduleNextRejection()
         m_rejectionTimer.startOneShot(Seconds(mimimumDelayBeforeReplying + randomNumber()));
 }
 
+UserMediaPermissionRequestManagerProxy::RequestAction UserMediaPermissionRequestManagerProxy::getRequestAction(uint64_t frameID, SecurityOrigin& userMediaDocumentOrigin, SecurityOrigin& topLevelDocumentOrigin, const MediaStreamRequest& userRequest, Vector<CaptureDevice>& audioDevices, Vector<CaptureDevice>& videoDevices)
+{
+    if (videoDevices.isEmpty() && audioDevices.isEmpty())
+        return RequestAction::Deny;
+
+    bool requestingScreenCapture = userRequest.type == MediaStreamRequest::Type::DisplayMedia;
+    ASSERT(!(requestingScreenCapture && videoDevices.isEmpty()));
+    ASSERT(!(requestingScreenCapture && !audioDevices.isEmpty()));
+    bool requestingCamera = !requestingScreenCapture && !videoDevices.isEmpty();
+    bool requestingMicrophone = !audioDevices.isEmpty();
+
+    if (wasRequestDenied(frameID, userMediaDocumentOrigin, topLevelDocumentOrigin, requestingMicrophone, requestingCamera, requestingScreenCapture))
+        return RequestAction::Deny;
+
+    if (userRequest.type == MediaStreamRequest::Type::DisplayMedia)
+        return RequestAction::Prompt;
+
+    return searchForGrantedRequest(frameID, userMediaDocumentOrigin, topLevelDocumentOrigin, requestingMicrophone, requestingCamera) ? RequestAction::Grant : RequestAction::Prompt;
+}
+
 void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(uint64_t userMediaID, uint64_t frameID, Ref<SecurityOrigin>&& userMediaDocumentOrigin, Ref<SecurityOrigin>&& topLevelDocumentOrigin, const MediaStreamRequest& userRequest)
 {
 #if ENABLE(MEDIA_STREAM)
@@ -279,32 +301,29 @@ void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(
         if (!m_page.isValid() || !m_page.mainFrame())
             return;
 
-        if (videoDevices.isEmpty() && audioDevices.isEmpty()) {
+        auto action = getRequestAction(m_page.mainFrame()->frameID(), userMediaDocumentOrigin.get(), topLevelDocumentOrigin.get(), localUserRequest, audioDevices, videoDevices);
+        if (action == RequestAction::Deny) {
             denyRequest(userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
             return;
         }
 
-        if (wasRequestDenied(m_page.mainFrame()->frameID(), userMediaDocumentOrigin.get(), topLevelDocumentOrigin.get(), !audioDevices.isEmpty(), !videoDevices.isEmpty())) {
-            denyRequest(userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
-            return;
-        }
+        if (action == RequestAction::Grant) {
+            ASSERT(localUserRequest.type != MediaStreamRequest::Type::DisplayMedia);
 
-        auto* grantedRequest = searchForGrantedRequest(frameID, userMediaDocumentOrigin.get(), topLevelDocumentOrigin.get(), !audioDevices.isEmpty(), !videoDevices.isEmpty());
-        if (grantedRequest) {
             if (m_page.isViewVisible()) {
-            // We select the first available devices, but the current client API allows client to select which device to pick.
-            // FIXME: Remove the possiblity for the client to do the device selection.
+                // We select the first available devices, but the current client API allows client to select which device to pick.
+                // FIXME: Remove the possiblity for the client to do the device selection.
                 auto audioDevice = !audioDevices.isEmpty() ? audioDevices[0] : CaptureDevice();
                 auto videoDevice = !videoDevices.isEmpty() ? videoDevices[0] : CaptureDevice();
-                grantAccess(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), grantedRequest->deviceIdentifierHashSalt());
+                grantAccess(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(deviceIdentifierHashSalt));
             } else
-                m_pregrantedRequests.append(createPermissionRequest(userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), WTFMove(audioDevices), WTFMove(videoDevices), String(grantedRequest->deviceIdentifierHashSalt()), WTFMove(localUserRequest)));
+                m_pregrantedRequests.append(createPermissionRequest(userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), WTFMove(audioDevices), WTFMove(videoDevices), WTFMove(deviceIdentifierHashSalt), WTFMove(localUserRequest)));
 
             return;
         }
+
         auto userMediaOrigin = API::SecurityOrigin::create(userMediaDocumentOrigin.get());
         auto topLevelOrigin = API::SecurityOrigin::create(topLevelDocumentOrigin.get());
-
         auto pendingRequest = createPermissionRequest(userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), WTFMove(audioDevices), WTFMove(videoDevices), WTFMove(deviceIdentifierHashSalt), WTFMove(localUserRequest));
 
         if (m_page.isControlledByAutomation()) {
index ee1ea62..8cb8258 100644 (file)
@@ -73,10 +73,17 @@ private:
     bool grantAccess(uint64_t userMediaID, const WebCore::CaptureDevice audioDevice, const WebCore::CaptureDevice videoDevice, const String& deviceIdentifierHashSalt);
 
     const UserMediaPermissionRequestProxy* searchForGrantedRequest(uint64_t frameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo) const;
-    bool wasRequestDenied(uint64_t mainFrameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo);
+    bool wasRequestDenied(uint64_t mainFrameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo, bool needsScreenCapture);
 #endif
     void getUserMediaPermissionInfo(uint64_t userMediaID, uint64_t frameID, UserMediaPermissionCheckProxy::CompletionHandler&&, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin);
 
+    enum class RequestAction {
+        Deny,
+        Grant,
+        Prompt        
+    };
+    RequestAction getRequestAction(uint64_t frameID, WebCore::SecurityOrigin& userMediaDocumentOrigin, WebCore::SecurityOrigin& topLevelDocumentOrigin, const WebCore::MediaStreamRequest&, Vector<WebCore::CaptureDevice>& audioDevices, Vector<WebCore::CaptureDevice>& videoDevices);
+
     void watchdogTimerFired();
 
     HashMap<uint64_t, RefPtr<UserMediaPermissionRequestProxy>> m_pendingUserMediaRequests;
@@ -96,6 +103,7 @@ private:
         Ref<WebCore::SecurityOrigin> topLevelDocumentOrigin;
         bool isAudioDenied;
         bool isVideoDenied;
+        bool isScreenCaptureDenied;
     };
     Vector<DeniedRequest> m_deniedRequests;
 
index aec4a17..f2b0954 100644 (file)
@@ -1,3 +1,16 @@
+2018-11-04  Eric Carlson  <eric.carlson@apple.com>
+
+        [MediaStream] User should not be prompted again after denying getDisplayMedia request
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        <rdar://problem/45784512>
+
+        Reviewed by Youenn Fablet.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/GetDisplayMedia.mm:
+        (-[GetDisplayMediaUIDelegate _webView:requestUserMediaAuthorizationForDevices:url:mainFrameURL:decisionHandler:]):
+        (TestWebKitAPI::GetDisplayMediaTest::promptForCapture):
+        (TestWebKitAPI::TEST_F):
+
 2018-11-04  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][BFC] Add support for percentage height in quirks mode.
index adf8439..14e9bb2 100644 (file)
@@ -38,6 +38,7 @@
 #import <WebKit/WKWebViewConfiguration.h>
 
 static bool wasPrompted = false;
+static bool shouldDeny = false;
 static _WKCaptureDevices requestedDevices = 0;
 static bool receivedScriptMessage = false;
 static RetainPtr<WKScriptMessage> lastScriptMessage;
@@ -64,6 +65,11 @@ static RetainPtr<WKScriptMessage> lastScriptMessage;
 {
     wasPrompted = true;
 
+    if (shouldDeny) {
+        decisionHandler(NO);
+        return;
+    }
+
     requestedDevices = devices;
     BOOL needsMicrophoneAuthorization = !!(requestedDevices & _WKCaptureDeviceMicrophone);
     BOOL needsCameraAuthorization = !!(requestedDevices & _WKCaptureDeviceCamera);
@@ -140,7 +146,10 @@ public:
         } else {
             EXPECT_STREQ([(NSString *)[lastScriptMessage body] UTF8String], "denied");
             EXPECT_TRUE(haveStream(false));
-            EXPECT_FALSE(wasPrompted);
+            if (shouldDeny)
+                EXPECT_TRUE(wasPrompted);
+            else
+                EXPECT_FALSE(wasPrompted);
         }
     }
 
@@ -165,6 +174,16 @@ TEST_F(GetDisplayMediaTest, Constraints)
     promptForCapture(@"{ video: {width: { exact: 640} } }", false);
 }
 
+TEST_F(GetDisplayMediaTest, PromptOnceAfterDenial)
+{
+    promptForCapture(@"{ video: true }", true);
+    shouldDeny = true;
+    promptForCapture(@"{ video: true }", false);
+    shouldDeny = false;
+    promptForCapture(@"{ video: true }", false);
+    promptForCapture(@"{ video: true }", false);
+}
+
 } // namespace TestWebKitAPI
 
 #endif // ENABLE(MEDIA_STREAM)