End of media capture should not be reported before 3 seconds of the start of capture
Created attachment 408554 [details] Patch
Created attachment 408684 [details] Patch
<rdar://problem/68512358>
<rdar://problem/68839151>
Comment on attachment 408684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408684&action=review > Source/WebKit/ChangeLog:8 > + Add support for delaying of end of capture. We're not delaying the end of capture, just the state transition. Also, I think it would be helpful to state why this is being changed. Maybe just: A capture indicator should be visible to the user for at least three seconds. > Source/WebKit/ChangeLog:9 > + A timer is scheduled when transitioning from no capture to capture. s/transitioning from no capture to capture/transitioning from capture to no capture/ > Source/WebKit/UIProcess/API/C/WKPage.cpp:3004 > +void WKPageSetMediaCaptureReportingDelayForTesting(WKPageRef page, double delay) > +{ > + toImpl(page)->setMediaCaptureReportingDelay(Seconds(delay)); > +} It seems a shame to add new C API > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:330 > +@property (nonatomic, setter=_setMediaCaptureReportingDelayForTesting:) double _mediaCaptureReportingDelayForTesting WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); Should this go in WKWebViewPrivateForTesting.h instead? > Source/WebKit/UIProcess/WebPageProxy.cpp:9073 > + bool isReportingCapture = m_reportedMediaCaptureState & MediaProducer::MediaCaptureMask; `haveReportedCapture` might be better.
Thanks for the review. Will update the patch according all comments before landing. (In reply to Eric Carlson from comment #5) > Comment on attachment 408684 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408684&action=review > > > Source/WebKit/ChangeLog:8 > > + Add support for delaying of end of capture. > > We're not delaying the end of capture, just the state transition. Also, I > think it would be helpful to state why this is being changed. > > Maybe just: A capture indicator should be visible to the user for at least > three seconds. OK > > Source/WebKit/UIProcess/API/C/WKPage.cpp:3004 > > +void WKPageSetMediaCaptureReportingDelayForTesting(WKPageRef page, double delay) > > +{ > > + toImpl(page)->setMediaCaptureReportingDelay(Seconds(delay)); > > +} > > It seems a shame to add new C API Yes, I initially went ObjC. I added this one so that other ports do not have to add their own versions to continue running some of the existing tests. > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:330 > > +@property (nonatomic, setter=_setMediaCaptureReportingDelayForTesting:) double _mediaCaptureReportingDelayForTesting WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > Should this go in WKWebViewPrivateForTesting.h instead? Did not know about it, yes!
Created attachment 408799 [details] Patch for landing
Committed r267081: <https://trac.webkit.org/changeset/267081> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408799 [details].