Changeset 267081 in webkit

Timestamp:

Sep 15, 2020 1:56:47 AM (4 years ago)

Author:

youenn@apple.com

Message:

End of media capture should not be reported before 3 seconds of the start of capture
​https://bugs.webkit.org/show_bug.cgi?id=216415
Source/WebCore:

Reviewed by Eric Carlson.

Add a timer that allows taking a function and a delay as parameter.
Covered by added API test.

• platform/Timer.h:

(WebCore::DeferrableTaskTimer::fired):
(WebCore::DeferrableTaskTimer::doTask):
(WebCore::DeferrableTaskTimer::cancel):

Source/WebKit:

<rdar://problem/68512358>

Reviewed by Eric Carlson.

Add support for delaying of end of capture notification to the application.
This allows to ensure that a capture indicator stays for long enough for the user to notice it.
A capture indicator should be visible to the user for at least three seconds.

A timer is scheduled when starting a capture and there is no ongoing capture.
As long as the timer is active, the capture state cannot be transitioned to no capture.
Other transitions are allowed.
Once the timer kicks in, any capture state update is done synchronously.
Ensure to update the capture state when the timer kicks in.

Note that even navigations will not allow transitioning the capture state sooner.
This is done to ensure a page does not try to capture one frame before navigating to another page.
In practice, very few pages should navigate quickly after starting capture.

• UIProcess/API/C/WKPage.cpp:

(WKPageGetMediaState):
(WKPageSetMediaCaptureReportingDelayForTesting):

• UIProcess/API/C/WKPagePrivate.h:
• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _mediaCaptureState]):

• UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
• UIProcess/API/Cocoa/WKWebViewTesting.mm:

(-[WKWebView _mediaCaptureReportingDelayForTesting]):
(-[WKWebView _setMediaCaptureReportingDelayForTesting:]):

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::updatePlayingMediaDidChange):
(WebKit::WebPageProxy::updateReportedMediaCaptureState):

• UIProcess/WebPageProxy.h:

(WebKit::WebPageProxy::reportedMediaCaptureState const):
(WebKit::WebPageProxy::mediaCaptureReportingDelay const):
(WebKit::WebPageProxy::setMediaCaptureReportingDelay):

Tools:

Reviewed by Eric Carlson.

• TestWebKitAPI/Tests/WebKit/GetUserMedia.mm:

(TestWebKitAPI::TEST):

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

(-[GetUserMediaUIDelegate _webView:mediaCaptureStateDidChange:]):
Add a notCapturing boolean and wait for the notification before going back to the capturing page.
Decrease delay to 1 second to make the test run faster.

• WebKitTestRunner/TestController.cpp:

(WTR::TestController::resetStateToConsistentValues):
Make sure to set media capture state delay to zero.

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/Timer.h (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/C/WKPage.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/API/C/WKPagePrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (4 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit/GetUserMedia.mm (modified) (3 diffs)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (5 diffs)
• Tools/WebKitTestRunner/TestController.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-15  Youenn Fablet  <youenn@apple.com>
+
+        End of media capture should not be reported before 3 seconds of the start of capture
+        https://bugs.webkit.org/show_bug.cgi?id=216415
+
+        Reviewed by Eric Carlson.
+
+        Add a timer that allows taking a function and a delay as parameter.
+        Covered by added API test.
+
+        * platform/Timer.h:
+        (WebCore::DeferrableTaskTimer::fired):
+        (WebCore::DeferrableTaskTimer::doTask):
+        (WebCore::DeferrableTaskTimer::cancel):
+
 2020-09-14  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/WebCore/platform/Timer.h

@@ -196 +196 @@
 };
 
-}
+class DeferrableTaskTimer final : private TimerBase {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    DeferrableTaskTimer() = default;
+
+    void doTask(Function<void()>&&, Seconds);
+    void cancel();
+    bool isActive() const { return TimerBase::isActive(); }
+
+private:
+    void fired() final;
+
+    Function<void()> m_function;
+};
+
+inline void DeferrableTaskTimer::fired()
+{
+    std::exchange(m_function, { })();
+}
+
+inline void DeferrableTaskTimer::doTask(Function<void()>&& function, Seconds delay)
+{
+    ASSERT(!isActive());
+    ASSERT(!m_function);
+    m_function = WTFMove(function);
+    startOneShot(delay);
+}
+
+inline void DeferrableTaskTimer::cancel()
+{
+    std::exchange(m_function, { });
+    stop();
+}
+
+}

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-09-15  Youenn Fablet  <youenn@apple.com>
+
+        End of media capture should not be reported before 3 seconds of the start of capture
+        https://bugs.webkit.org/show_bug.cgi?id=216415
+        <rdar://problem/68512358>
+
+        Reviewed by Eric Carlson.
+
+        Add support for delaying of end of capture notification to the application.
+        This allows to ensure that a capture indicator stays for long enough for the user to notice it.
+        A capture indicator should be visible to the user for at least three seconds.
+
+        A timer is scheduled when starting a capture and there is no ongoing capture.
+        As long as the timer is active, the capture state cannot be transitioned to no capture.
+        Other transitions are allowed.
+        Once the timer kicks in, any capture state update is done synchronously.
+        Ensure to update the capture state when the timer kicks in.
+
+        Note that even navigations will not allow transitioning the capture state sooner.
+        This is done to ensure a page does not try to capture one frame before navigating to another page.
+        In practice, very few pages should navigate quickly after starting capture.
+
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageGetMediaState):
+        (WKPageSetMediaCaptureReportingDelayForTesting):
+        * UIProcess/API/C/WKPagePrivate.h:
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _mediaCaptureState]):
+        * UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
+        * UIProcess/API/Cocoa/WKWebViewTesting.mm:
+        (-[WKWebView _mediaCaptureReportingDelayForTesting]):
+        (-[WKWebView _setMediaCaptureReportingDelayForTesting:]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::updatePlayingMediaDidChange):
+        (WebKit::WebPageProxy::updateReportedMediaCaptureState):
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::reportedMediaCaptureState const):
+        (WebKit::WebPageProxy::mediaCaptureReportingDelay const):
+        (WebKit::WebPageProxy::setMediaCaptureReportingDelay):
+
 2020-09-15  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp

@@ -2863 +2863 @@
 WKMediaState WKPageGetMediaState(WKPageRef page)
 {
-    WebCore::MediaProducer::MediaStateFlags coreState = toImpl(page)->mediaStateFlags();
+    WebCore::MediaProducer::MediaStateFlags coreState = toImpl(page)->reportedMediaCaptureState();
     WKMediaState state = kWKMediaIsNotPlaying;
 
@@ -2998 +2998 @@
 #endif
 }
+
+void WKPageSetMediaCaptureReportingDelayForTesting(WKPageRef page, double delay)
+{
+    toImpl(page)->setMediaCaptureReportingDelay(Seconds(delay));
+}

trunk/Source/WebKit/UIProcess/API/C/WKPagePrivate.h

@@ -199 +199 @@
 WK_EXPORT void WKPageClearLoadedSubresourceDomains(WKPageRef page);
 
+WK_EXPORT void WKPageSetMediaCaptureReportingDelayForTesting(WKPageRef page, double delay);
+
 #ifdef __cplusplus
 }

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -2952 +2952 @@
 - (_WKMediaCaptureState)_mediaCaptureState
 {
-    return WebKit::toWKMediaCaptureState(_page->mediaStateFlags());
+    return WebKit::toWKMediaCaptureState(_page->reportedMediaCaptureState());
 }
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h

@@ -49 +49 @@
 
 - (void)_denyNextUserMediaRequest;
+@property (nonatomic, setter=_setMediaCaptureReportingDelayForTesting:) double _mediaCaptureReportingDelayForTesting WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 - (BOOL)_beginBackSwipeForTesting;

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm

@@ -227 +227 @@
 }
 
+- (double)_mediaCaptureReportingDelayForTesting
+{
+    return _page->mediaCaptureReportingDelay().value();
+}
+
+- (void)_setMediaCaptureReportingDelayForTesting:(double)captureReportingDelay
+{
+    _page->setMediaCaptureReportingDelay(Seconds(captureReportingDelay));
+}
+
 - (void)_doAfterProcessingAllPendingMouseEvents:(dispatch_block_t)action
 {

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -9038 +9038 @@
 #if ENABLE(MEDIA_STREAM)
     if (oldMediaCaptureState != newMediaCaptureState) {
-        m_uiClient->mediaCaptureStateDidChange(m_mediaState);
+        updateReportedMediaCaptureState();
+
         ASSERT(m_userMediaPermissionRequestManager);
         if (m_userMediaPermissionRequestManager)
@@ -9055 +9056 @@
 
     m_process->updateAudibleMediaAssertions();
+}
+
+void WebPageProxy::updateReportedMediaCaptureState()
+{
+    if (m_reportedMediaCaptureState == m_mediaState)
+        return;
+
+    bool haveReportedCapture = m_reportedMediaCaptureState & MediaProducer::MediaCaptureMask;
+    bool willReportCapture = m_mediaState & MediaProducer::MediaCaptureMask;
+
+    if (haveReportedCapture && !willReportCapture && m_delayStopCapturingReportingTimer.isActive())
+        return;
+
+    if (!haveReportedCapture && willReportCapture) {
+        m_delayStopCapturingReporting = true;
+        m_delayStopCapturingReportingTimer.doTask([this] {
+            m_delayStopCapturingReporting = false;
+            updateReportedMediaCaptureState();
+        }, m_mediaCaptureReportingDelay);
+    }
+
+    m_reportedMediaCaptureState = m_mediaState;
+    m_uiClient->mediaCaptureStateDidChange(m_mediaState);
 }
 

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1450 +1450 @@
     bool isPlayingAudio() const { return !!(m_mediaState & WebCore::MediaProducer::IsPlayingAudio); }
     void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags, uint64_t);
+    void updateReportedMediaCaptureState();
+
     void updatePlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags);
     bool isCapturingAudio() const { return m_mediaState & WebCore::MediaProducer::AudioCaptureMask; }
@@ -1456 +1458 @@
     bool hasActiveVideoStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveVideoCaptureDevice; }
     WebCore::MediaProducer::MediaStateFlags mediaStateFlags() const { return m_mediaState; }
+    WebCore::MediaProducer::MediaStateFlags reportedMediaCaptureState() const { return m_reportedMediaCaptureState; }
     WebCore::MediaProducer::MutedStateFlags mutedStateFlags() const { return m_mutedState; }
 
@@ -1821 +1824 @@
     void pdfOpenWithPreview(PDFPluginIdentifier);
 #endif
+
+    Seconds mediaCaptureReportingDelay() const { return m_mediaCaptureReportingDelay; }
+    void setMediaCaptureReportingDelay(Seconds captureReportingDelay) { m_mediaCaptureReportingDelay = captureReportingDelay; }
 
 private:
@@ -2757 +2763 @@
     WebCore::MediaProducer::MediaStateFlags m_mediaState { WebCore::MediaProducer::IsNotPlaying };
 
+    // To make sure capture indicators are visible long enough, m_reportedMediaCaptureState is the same as m_mediaState except that we might delay a bit transition from capturing to not-capturing.
+    WebCore::MediaProducer::MediaStateFlags m_reportedMediaCaptureState { WebCore::MediaProducer::IsNotPlaying };
+    WebCore::DeferrableTaskTimer m_delayStopCapturingReportingTimer;
+    bool m_delayStopCapturingReporting { false };
+    static constexpr Seconds DefaultMediaCaptureReportingDelay { 3_s };
+    Seconds m_mediaCaptureReportingDelay { DefaultMediaCaptureReportingDelay };
+
     bool m_hasHadSelectionChangesFromUserInteraction { false };
 

trunk/Tools/ChangeLog

@@ +1 @@
+2020-09-15  Youenn Fablet  <youenn@apple.com>
+
+        End of media capture should not be reported before 3 seconds of the start of capture
+        https://bugs.webkit.org/show_bug.cgi?id=216415
+
+        Reviewed by Eric Carlson.
+
+        * TestWebKitAPI/Tests/WebKit/GetUserMedia.mm:
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        (-[GetUserMediaUIDelegate _webView:mediaCaptureStateDidChange:]):
+        Add a notCapturing boolean and wait for the notification before going back to the capturing page.
+        Decrease delay to 1 second to make the test run faster.
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        Make sure to set media capture state delay to zero.
+
 2020-09-14  Sam Weinig  <weinig@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit/GetUserMedia.mm

@@ -37 +37 @@
 #import <WebKit/WKWebView.h>
 #import <WebKit/WKWebViewConfiguration.h>
+#import <WebKit/WKWebViewPrivateForTesting.h>
 #import <WebKit/_WKProcessPoolConfiguration.h>
 #import <wtf/text/StringBuilder.h>
@@ -125 +126 @@
     webView.UIDelegate = delegate.get();
 
+    webView._mediaCaptureReportingDelayForTesting = 0;
+
     [webView loadTestPageNamed:@"getUserMedia"];
     EXPECT_TRUE(waitUntilCaptureState(webView, _WKMediaCaptureStateActiveCamera));
@@ -202 +205 @@
 }
 
+TEST(WebKit2, CaptureIndicatorDelay)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto processPoolConfig = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    auto preferences = [configuration preferences];
+    preferences._mediaCaptureRequiresSecureConnection = NO;
+    preferences._mediaDevicesEnabled = YES;
+    preferences._mockCaptureDevicesEnabled = YES;
+
+    auto messageHandler = adoptNS([[GUMMessageHandler alloc] init]);
+    [[configuration.get() userContentController] addScriptMessageHandler:messageHandler.get() name:@"gum"];
+
+    auto webView = [[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get() processPoolConfiguration:processPoolConfig.get()];
+    webView._mediaCaptureReportingDelayForTesting = 2;
+
+    auto delegate = adoptNS([[GetUserMediaCaptureUIDelegate alloc] init]);
+    webView.UIDelegate = delegate.get();
+
+    wasPrompted = false;
+
+    [webView loadTestPageNamed:@"getUserMedia"];
+    EXPECT_TRUE(waitUntilCaptureState(webView, _WKMediaCaptureStateActiveCamera));
+
+    TestWebKitAPI::Util::run(&wasPrompted);
+    wasPrompted = false;
+
+    [webView stringByEvaluatingJavaScript:@"stop()"];
+
+    // We wait 1 second, we should still see camera be reported.
+    sleep(1_s);
+    EXPECT_EQ([webView _mediaCaptureState], _WKMediaCaptureStateActiveCamera);
+
+    // One additional second should allow us to go back to no capture being reported.
+    EXPECT_TRUE(waitUntilCaptureState(webView, _WKMediaCaptureStateNone));
+
+}
+
 TEST(WebKit2, GetCapabilities)
 {

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

@@ -6397 +6397 @@
 
 static bool isCapturing = false;
+static bool isNotCapturing = false;
 @interface GetUserMediaUIDelegate : NSObject<WKUIDelegate>
 - (void)_webView:(WKWebView *)webView requestUserMediaAuthorizationForDevices:(_WKCaptureDevices)devices url:(NSURL *)url mainFrameURL:(NSURL *)mainFrameURL decisionHandler:(void (^)(BOOL authorized))decisionHandler;
@@ -6417 +6418 @@
 {
     isCapturing = state == _WKMediaCaptureStateActiveCamera;
+    isNotCapturing = !state;
 }
 @end
@@ -6449 +6451 @@
 
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    webView.get()._mediaCaptureReportingDelayForTesting = 1;
 
     auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
@@ -6473 +6476 @@
 
     auto pid2 = [webView _webProcessIdentifier];
+    TestWebKitAPI::Util::run(&isNotCapturing);
 
     EXPECT_FALSE(isCapturing);
@@ -6479 +6483 @@
     isCapturing = false;
     [webView goBack];
+
     TestWebKitAPI::Util::run(&isCapturing);
     isCapturing = false;
+    isNotCapturing = true;
 }
 

trunk/Tools/WebKitTestRunner/TestController.cpp

@@ -1075 +1075 @@
     WKPageSetMockCameraOrientation(m_mainWebView->page(), 0);
     resetMockMediaDevices();
+    WKPageSetMediaCaptureReportingDelayForTesting(m_mainWebView->page(), 0);
 
     // FIXME: This function should also ensure that there is only one page open.