Unreviewed, rolling out r237784 and r237788.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 18:03:14 +0000 (18:03 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 18:03:14 +0000 (18:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191270

Caused mediastream layout test failures (Requested by
ryanhaddad on #webkit).

Reverted changesets:

"[MediaStream] User should not be prompted again after denying
getDisplayMedia request"
https://bugs.webkit.org/show_bug.cgi?id=191227
https://trac.webkit.org/changeset/237784

"[MediaStream] User should not be prompted again after denying
getDisplayMedia request"
https://bugs.webkit.org/show_bug.cgi?id=191227
https://trac.webkit.org/changeset/237788

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237812 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 4fb852e..7051c11 100644 (file)
@@ -1,3 +1,23 @@
+2018-11-05  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r237784 and r237788.
+        https://bugs.webkit.org/show_bug.cgi?id=191270
+
+        Caused mediastream layout test failures (Requested by
+        ryanhaddad on #webkit).
+
+        Reverted changesets:
+
+        "[MediaStream] User should not be prompted again after denying
+        getDisplayMedia request"
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        https://trac.webkit.org/changeset/237784
+
+        "[MediaStream] User should not be prompted again after denying
+        getDisplayMedia request"
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        https://trac.webkit.org/changeset/237788
+
 2018-11-04  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [MediaStream] User should not be prompted again after denying getDisplayMedia request
index 41c8e7c..62774c9 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(), request->requiresDisplayCapture() });
+        m_deniedRequests.append(DeniedRequest { request->mainFrameID(), request->userMediaDocumentSecurityOrigin(), request->topLevelDocumentSecurityOrigin(), request->requiresAudioCapture(), request->requiresVideoCapture() });
 
     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 needsScreenCapture)
+bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrameID, const SecurityOrigin& userMediaDocumentOrigin, const SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo)
 {
     for (const auto& deniedRequest : m_deniedRequests) {
         if (!deniedRequest.userMediaDocumentOrigin->isSameSchemeHostPort(userMediaDocumentOrigin))
@@ -226,8 +226,6 @@ bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrame
             return true;
         if (deniedRequest.isVideoDenied && needsVideo)
             return true;
-        if (deniedRequest.isScreenCaptureDenied && needsScreenCapture)
-            return true;
     }
     return false;
 }
@@ -261,28 +259,6 @@ void UserMediaPermissionRequestManagerProxy::scheduleNextRejection()
         m_rejectionTimer.startOneShot(Seconds(mimimumDelayBeforeReplying + randomNumber()));
 }
 
-#if ENABLE(MEDIA_STREAM)
-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;
-}
-#endif
-
 void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(uint64_t userMediaID, uint64_t frameID, Ref<SecurityOrigin>&& userMediaDocumentOrigin, Ref<SecurityOrigin>&& topLevelDocumentOrigin, const MediaStreamRequest& userRequest)
 {
 #if ENABLE(MEDIA_STREAM)
@@ -303,29 +279,32 @@ void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(
         if (!m_page.isValid() || !m_page.mainFrame())
             return;
 
-        auto action = getRequestAction(m_page.mainFrame()->frameID(), userMediaDocumentOrigin.get(), topLevelDocumentOrigin.get(), localUserRequest, audioDevices, videoDevices);
-        if (action == RequestAction::Deny) {
+        if (videoDevices.isEmpty() && audioDevices.isEmpty()) {
             denyRequest(userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
             return;
         }
 
-        if (action == RequestAction::Grant) {
-            ASSERT(localUserRequest.type != MediaStreamRequest::Type::DisplayMedia);
+        if (wasRequestDenied(m_page.mainFrame()->frameID(), userMediaDocumentOrigin.get(), topLevelDocumentOrigin.get(), !audioDevices.isEmpty(), !videoDevices.isEmpty())) {
+            denyRequest(userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
+            return;
+        }
 
+        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), WTFMove(deviceIdentifierHashSalt));
+                grantAccess(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), grantedRequest->deviceIdentifierHashSalt());
             } else
-                m_pregrantedRequests.append(createPermissionRequest(userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), WTFMove(audioDevices), WTFMove(videoDevices), WTFMove(deviceIdentifierHashSalt), WTFMove(localUserRequest)));
+                m_pregrantedRequests.append(createPermissionRequest(userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), WTFMove(audioDevices), WTFMove(videoDevices), String(grantedRequest->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 1ccefe0..ee1ea62 100644 (file)
@@ -73,17 +73,9 @@ 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 needsScreenCapture);
-
-    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);
+    bool wasRequestDenied(uint64_t mainFrameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo);
 #endif
+    void getUserMediaPermissionInfo(uint64_t userMediaID, uint64_t frameID, UserMediaPermissionCheckProxy::CompletionHandler&&, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin);
 
     void watchdogTimerFired();
 
@@ -104,7 +96,6 @@ private:
         Ref<WebCore::SecurityOrigin> topLevelDocumentOrigin;
         bool isAudioDenied;
         bool isVideoDenied;
-        bool isScreenCaptureDenied;
     };
     Vector<DeniedRequest> m_deniedRequests;
 
index e69f43c..ba851ee 100644 (file)
@@ -1,3 +1,23 @@
+2018-11-05  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r237784 and r237788.
+        https://bugs.webkit.org/show_bug.cgi?id=191270
+
+        Caused mediastream layout test failures (Requested by
+        ryanhaddad on #webkit).
+
+        Reverted changesets:
+
+        "[MediaStream] User should not be prompted again after denying
+        getDisplayMedia request"
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        https://trac.webkit.org/changeset/237784
+
+        "[MediaStream] User should not be prompted again after denying
+        getDisplayMedia request"
+        https://bugs.webkit.org/show_bug.cgi?id=191227
+        https://trac.webkit.org/changeset/237788
+
 2018-11-05  Lucas Forschler  <lforschler@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=191268
index 14e9bb2..adf8439 100644 (file)
@@ -38,7 +38,6 @@
 #import <WebKit/WKWebViewConfiguration.h>
 
 static bool wasPrompted = false;
-static bool shouldDeny = false;
 static _WKCaptureDevices requestedDevices = 0;
 static bool receivedScriptMessage = false;
 static RetainPtr<WKScriptMessage> lastScriptMessage;
@@ -65,11 +64,6 @@ static RetainPtr<WKScriptMessage> lastScriptMessage;
 {
     wasPrompted = true;
 
-    if (shouldDeny) {
-        decisionHandler(NO);
-        return;
-    }
-
     requestedDevices = devices;
     BOOL needsMicrophoneAuthorization = !!(requestedDevices & _WKCaptureDeviceMicrophone);
     BOOL needsCameraAuthorization = !!(requestedDevices & _WKCaptureDeviceCamera);
@@ -146,10 +140,7 @@ public:
         } else {
             EXPECT_STREQ([(NSString *)[lastScriptMessage body] UTF8String], "denied");
             EXPECT_TRUE(haveStream(false));
-            if (shouldDeny)
-                EXPECT_TRUE(wasPrompted);
-            else
-                EXPECT_FALSE(wasPrompted);
+            EXPECT_FALSE(wasPrompted);
         }
     }
 
@@ -174,16 +165,6 @@ 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)