Attachment 360895 Details for Bug 194122 – Patch

[patch] Patch

bug-194122-20190201141245.patch (text/plain), 16.57 KB, created by youenn fablet on 2019-02-01 14:12:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-02-01 14:12:45 PST

Size: 16.57 KB

patch

obsolete

>Subversion Revision: 240633
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 163bcae1cfb85699e34e98f57dadebfc54a8bc82..38d581561dcb333f2a1f785799525b5f4a55b578 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-02-01  Youenn Fablet  <youenn@apple.com>
>+
>+        Capture state should be managed consistently when doing process swapping
>+        https://bugs.webkit.org/show_bug.cgi?id=194122
>+        <rdar://problem/47609293>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When doing PSON, WebPageProxy::resetState is called.
>+        It resets the media state, but does not call the client delegates.
>+        Instead of directly updating the media state, call the routine used to update it so that client delegates are called.
>+
>+        Covered by new API test and layout test.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _mediaCaptureState]):
>+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::resetState):
>+        (WebKit::WebPageProxy::isPlayingMediaDidChange):
>+        (WebKit::WebPageProxy::updatePlayingMediaDidChange):
>+        * UIProcess/WebPageProxy.h:
>+
> 2019-01-31  Youenn Fablet  <youenn@apple.com>
> 
>         Add an API test to cover UIClient checkUserMediaPermissionForOrigin being nullptr
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index c2a4d6a1902ab364d021976036858b2de863c8a8..3c4c8649604cecce998b3598100871a54512ed25 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -6619,7 +6619,7 @@ void WebPageProxy::resetState(ResetStateReason resetStateReason)
>         editCommand->invalidate();
> 
>     m_activePopupMenu = nullptr;
>-    m_mediaState = MediaProducer::IsNotPlaying;
>+    updatePlayingMediaDidChange(MediaProducer::IsNotPlaying);
> 
> #if ENABLE(POINTER_LOCK)
>     requestPointerUnlock();
>@@ -7677,7 +7677,11 @@ void WebPageProxy::isPlayingMediaDidChange(MediaProducer::MediaStateFlags newSta
>     ASSERT(focusManager);
>     focusManager->updatePlaybackAttributesFromMediaState(this, sourceElementID, newState);
> #endif
>+    updatePlayingMediaDidChange(newState);
>+}
> 
>+void WebPageProxy::updatePlayingMediaDidChange(MediaProducer::MediaStateFlags newState)
>+{
>     if (newState == m_mediaState)
>         return;
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 6faacb86633a522055b067bb872a2be92dad91fa..5b90620bf94bef3e61792ca7bf194a14dfb72042 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1238,6 +1238,7 @@ public:
> 
>     bool isPlayingAudio() const { return !!(m_mediaState & WebCore::MediaProducer::IsPlayingAudio); }
>     void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags, uint64_t);
>+    void updatePlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags newState);
>     bool hasActiveAudioStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveAudioCaptureDevice; }
>     bool hasActiveVideoStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveVideoCaptureDevice; }
>     WebCore::MediaProducer::MediaStateFlags mediaStateFlags() const { return m_mediaState; }
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index fc7d52cc45c04874f2c7bf52166ad12d63be7a48..01999b984e804545264e06a8995a8987273d51de 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-02-01  Youenn Fablet  <youenn@apple.com>
>+
>+        Capture state should be managed consistently when doing process swapping
>+        https://bugs.webkit.org/show_bug.cgi?id=194122
>+        <rdar://problem/47609293>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+        (-[GetUserMediaUIDelegate _webView:requestUserMediaAuthorizationForDevices:url:mainFrameURL:decisionHandler:]):
>+        (-[GetUserMediaUIDelegate _webView:checkUserMediaPermissionForURL:mainFrameURL:frameIdentifier:decisionHandler:]):
>+        (-[GetUserMediaUIDelegate _webView:mediaCaptureStateDidChange:]):
>+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
>+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
>+        (WTR::TestRunner::isDoingMediaCapture const):
>+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::isDoingMediaCapture const):
>+        * WebKitTestRunner/TestController.h:
>+        * WebKitTestRunner/TestInvocation.cpp:
>+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
>+        * WebKitTestRunner/cocoa/TestControllerCocoa.mm:
>+        (WTR::TestController::isDoingMediaCapture const):
>+
> 2019-01-31  Youenn Fablet  <youenn@apple.com>
> 
>         Add an API test to cover UIClient checkUserMediaPermissionForOrigin being nullptr
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index af85170f83fa118e593b8c6fa2e75ba7fc4e6dd8..6d04dd5f22fe6900db9162478eea60eefab35e2d 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -28,6 +28,7 @@
> #import "PlatformUtilities.h"
> #import "Test.h"
> #import "TestNavigationDelegate.h"
>+#import "TestWKWebView.h"
> #import <WebKit/WKContentRuleListStore.h>
> #import <WebKit/WKNavigationDelegatePrivate.h>
> #import <WebKit/WKNavigationPrivate.h>
>@@ -4361,4 +4362,88 @@ TEST(ProcessSwap, ContentBlockingAfterProcessSwap)
>     done = false;
> }
> 
>+static bool isCapturing = false;
>+@interface GetUserMediaUIDelegate : NSObject<WKUIDelegate>
>+- (void)_webView:(WKWebView *)webView requestUserMediaAuthorizationForDevices:(_WKCaptureDevices)devices url:(NSURL *)url mainFrameURL:(NSURL *)mainFrameURL decisionHandler:(void (^)(BOOL authorized))decisionHandler;
>+- (void)_webView:(WKWebView *)webView checkUserMediaPermissionForURL:(NSURL *)url mainFrameURL:(NSURL *)mainFrameURL frameIdentifier:(NSUInteger)frameIdentifier decisionHandler:(void (^)(NSString *salt, BOOL authorized))decisionHandler;
>+- (void)_webView:(WKWebView *)webView mediaCaptureStateDidChange:(_WKMediaCaptureState)state;
>+@end
>+
>+@implementation GetUserMediaUIDelegate
>+- (void)_webView:(WKWebView *)webView requestUserMediaAuthorizationForDevices:(_WKCaptureDevices)devices url:(NSURL *)url mainFrameURL:(NSURL *)mainFrameURL decisionHandler:(void (^)(BOOL authorized))decisionHandler
>+{
>+    decisionHandler(YES);
>+}
>+
>+- (void)_webView:(WKWebView *)webView checkUserMediaPermissionForURL:(NSURL *)url mainFrameURL:(NSURL *)mainFrameURL frameIdentifier:(NSUInteger)frameIdentifier decisionHandler:(void (^)(NSString *salt, BOOL authorized))decisionHandler
>+{
>+    decisionHandler(@"0x987654321", YES);
>+}
>+
>+- (void)_webView:(WKWebView *)webView mediaCaptureStateDidChange:(_WKMediaCaptureState)state
>+{
>+    isCapturing = state == _WKMediaCaptureStateActiveCamera;
>+}
>+@end
>+
>+static const char* getUserMediaBytes = R"PSONRESOURCE(
>+<head>
>+<body>
>+<script>
>+navigator.mediaDevices.getUserMedia({video: true});
>+</script>
>+</body>
>+</head>
>+)PSONRESOURCE";
>+
>+TEST(ProcessSwap, GetUserMediaCaptureState)
>+{
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
>+    processPoolConfiguration.get().prewarmsProcessesAutomatically = YES;
>+
>+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
>+
>+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    auto preferences = [webViewConfiguration.get() preferences];
>+    preferences._mediaCaptureRequiresSecureConnection = NO;
>+    preferences._mediaDevicesEnabled = YES;
>+    preferences._mockCaptureDevicesEnabled = YES;
>+
>+    [webViewConfiguration setProcessPool:processPool.get()];
>+    auto handler = adoptNS([[PSONScheme alloc] init]);
>+    [handler addMappingFromURLString:@"pson://www.webkit.org/getUserMedia.html" toData:getUserMediaBytes];
>+    [handler addMappingFromURLString:@"pson://www.apple.org/test.html" toData:""];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+
>+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+
>+    auto uiDelegate = adoptNS([[GetUserMediaUIDelegate alloc] init]);
>+    [webView setUIDelegate: uiDelegate.get()];
>+
>+    auto request = adoptNS([NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/getUserMedia.html"]]);
>+    [webView loadRequest:request.get()];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    TestWebKitAPI::Util::run(&isCapturing);
>+
>+    auto pid1 = [webView _webProcessIdentifier];
>+
>+    request = adoptNS([NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.org/test.html"]]);
>+    [webView loadRequest:request.get()];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid2 = [webView _webProcessIdentifier];
>+
>+    EXPECT_FALSE(isCapturing);
>+    EXPECT_FALSE(pid1 == pid2);
>+}
>+
> #endif // WK_API_ENABLED
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>index 67fc93e91622bdd333185c7f6b208a39fee44516..818d441884bf1cc890401823d48985121997772f 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>@@ -216,6 +216,7 @@ interface TestRunner {
>     void setUserMediaPersistentPermissionForOrigin(boolean permission, DOMString origin, DOMString parentOrigin);
>     unsigned long userMediaPermissionRequestCountForOrigin(DOMString origin, DOMString parentOrigin);
>     void resetUserMediaPermissionRequestCountForOrigin(DOMString origin, DOMString parentOrigin);
>+    readonly attribute boolean isDoingMediaCapture;
> 
>     // Audio testing.
>     [PassContext] void setAudioResult(object data);
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>index 79e71cbf437344ea5658ca73c67cd531fdd4d23e..273b48d01f91f7013365476517791b3936514710 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>@@ -1093,6 +1093,15 @@ void TestRunner::resetUserMediaPermission()
>     InjectedBundle::singleton().resetUserMediaPermission();
> }
> 
>+bool TestRunner::isDoingMediaCapture() const
>+{
>+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("IsDoingMediaCapture"));
>+    WKTypeRef returnData = nullptr;
>+    WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr, &returnData);
>+    ASSERT(WKGetTypeID(returnData) == WKBooleanGetTypeID());
>+    return WKBooleanGetValue(adoptWK(static_cast<WKBooleanRef>(returnData)).get());
>+}
>+
> void TestRunner::setUserMediaPersistentPermissionForOrigin(bool permission, JSStringRef origin, JSStringRef parentOrigin)
> {
>     WKRetainPtr<WKStringRef> originWK = toWK(origin);
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>index 2214d77b8c3979a47c5e8edaa5d32712d14341d9..ec769f646f5c50123a19f9a1361a9a1a9432c5b5 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>@@ -318,6 +318,7 @@ public:
>     void setUserMediaPersistentPermissionForOrigin(bool permission, JSStringRef origin, JSStringRef parentOrigin);
>     unsigned userMediaPermissionRequestCountForOrigin(JSStringRef origin, JSStringRef parentOrigin) const;
>     void resetUserMediaPermissionRequestCountForOrigin(JSStringRef origin, JSStringRef parentOrigin);
>+    bool isDoingMediaCapture() const;
> 
>     void setPageVisibility(JSStringRef state);
>     void resetPageVisibility();
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 6d4dffb49656524c64d962e79fe238384c69c0c6..54e39211bf5ce2339bc402f5fdd92920d6feb430 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -3185,6 +3185,12 @@ bool TestController::keyExistsInKeychain(const String&, const String&)
> {
>     return false;
> }
>+
>+bool TestController::isDoingMediaCapture() const
>+{
>+    return false;
>+}
>+
> #endif
> 
> void TestController::sendDisplayConfigurationChangedMessageForTesting()
>diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h
>index 13a39c6905f0d87ba87b9db57f8dad5c17ac71bc..95a058125d2aea0dc41740f2d9fe66a6a1bb76dd 100644
>--- a/Tools/WebKitTestRunner/TestController.h
>+++ b/Tools/WebKitTestRunner/TestController.h
>@@ -278,6 +278,8 @@ public:
>     UIKeyboardInputMode *overriddenKeyboardInputMode() const { return m_overriddenKeyboardInputMode.get(); }
> #endif
> 
>+    bool isDoingMediaCapture() const;
>+
> private:
>     WKRetainPtr<WKPageConfigurationRef> generatePageConfiguration(WKContextConfigurationRef);
>     WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration() const;
>diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
>index 0c6c877ff1dab755ade28e9e234f44e55fd94c27..d1c0441b9e57d4998620f89347aea9e157eee757 100644
>--- a/Tools/WebKitTestRunner/TestInvocation.cpp
>+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
>@@ -1034,6 +1034,10 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
>         WKRetainPtr<WKUInt64Ref> result(AdoptWK, WKUInt64Create(count));
>         return result;
>     }
>+    if (WKStringIsEqualToUTF8CString(messageName, "IsDoingMediaCapture")) {
>+        WKRetainPtr<WKTypeRef> result(AdoptWK, WKBooleanCreate(TestController::singleton().isDoingMediaCapture()));
>+        return result;
>+    }
> 
>     if (WKStringIsEqualToUTF8CString(messageName, "SetStatisticsDebugMode")) {
>         ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
>diff --git a/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm b/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm
>index 11b0e3ae34fbbe7e1ba9e1721a4d118d33026796..4d4d17642e471e853f9a3b83d713ddba20e2d562 100644
>--- a/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm
>+++ b/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm
>@@ -382,4 +382,9 @@ bool TestController::keyExistsInKeychain(const String& attrLabel, const String&
>     return false;
> }
> 
>+bool TestController::isDoingMediaCapture() const
>+{
>+    return m_mainWebView->platformView()._mediaCaptureState != _WKMediaCaptureStateNone;
>+}
>+
> } // namespace WTR
>diff --git a/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping-expected.txt b/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..7ef22e9a431ad0272713b71fdc8794016c8ef12f
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping-expected.txt
>@@ -0,0 +1 @@
>+PASS
>diff --git a/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping.html b/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d50ff22f7a32cd10d40d085a33156251c0bab656
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webrtc/getUserMedia-processSwapping.html
>@@ -0,0 +1,41 @@
>+<!doctype html>
>+<meta charset="utf-8">
>+<title>getUserMedia on process swapping</title>
>+<body>
>+<video id="local"></video>
>+<script src="/common/get-host-info.sub.js"></script>
>+<script>
>+if (window.testRunner) {
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+}
>+
>+async function getUserMediaAndNavigate()
>+{
>+    local.srcObject = await navigator.mediaDevices.getUserMedia({video: true});
>+    await local.play();
>+    await new Promise(resolve => setTimeout(resolve, 1000));
>+    if (window.testRunner && !testRunner.isDoingMediaCapture) {
>+        document.body.innerHTML = "FAIL: no capture state";
>+        testRunner.notifyDone();
>+        return;
>+    }
>+    window.location = get_host_info().HTTPS_ORIGIN + "/WebKit/webrtc/getUserMedia-processSwapping.html";
>+}
>+
>+function validateMediaCaptureStateAfterNavigation()
>+{
>+    if (!window.testRunner) {
>+        document.body.innerHTML = "FAIL: Need testRunner API";
>+        return;
>+    }
>+    document.body.innerHTML = testRunner.isDoingMediaCapture ? "FAIL" : "PASS";
>+    testRunner.notifyDone();
>+}
>+
>+if (window.location.protocol === "http:")
>+    getUserMediaAndNavigate();
>+else
>+    validateMediaCaptureStateAfterNavigation();
>+</script>
>+</body>

Actions: View | Formatted Diff | Diff

Attachments on bug 194122: 360804 | 360874 | 360875 | 360885 | 360891 | 360895 | 360896 | 360906