Don't call -[AVCapture startRunning] when interruption ends
https://bugs.webkit.org/show_bug.cgi?id=209919
<rdar://problem/61090625>

Calling -[AVCaptureSession startRunning] after a VideoNotAllowedInSideBySide interruption
ends triggers a bug in AVCapture that hangs the app for several seconds, but restarting the
capture session isn't necessary because it will restart automatically in that case.

Reviewed by Youenn Fablet.

Tested manually.

* platform/mediastream/mac/AVVideoCaptureSource.h: Remove InterruptionReason enum, convert
m_interruption to m_interrupted bool
* platform/mediastream/mac/AVVideoCaptureSource.mm:
(WebCore::AVVideoCaptureSource::AVVideoCaptureSource): Remove InterruptionReason enum checking.
(WebCore::AVVideoCaptureSource::stopProducingData): m_interruption -> m_interrupted.
(WebCore::AVVideoCaptureSource::interrupted const): Ditto.
(WebCore::AVVideoCaptureSource::captureSessionBeginInterruption): Ditto.
(WebCore::AVVideoCaptureSource::captureSessionEndInterruption): Ditto. Don't restart the session.
(-[WebCoreAVVideoCaptureSourceObserver observeValueForKeyPath:ofObject:change:context:]):
Drive-by fix: always log notifications to help with debugging.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@259430 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0a0526d8..98654ad 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2020-04-02  Eric Carlson  <eric.carlson@apple.com>
+
+        Don't call -[AVCapture startRunning] when interruption ends
+        https://bugs.webkit.org/show_bug.cgi?id=209919
+        <rdar://problem/61090625>
+
+        Calling -[AVCaptureSession startRunning] after a VideoNotAllowedInSideBySide interruption
+        ends triggers a bug in AVCapture that hangs the app for several seconds, but restarting the
+        capture session isn't necessary because it will restart automatically in that case.
+
+        Reviewed by Youenn Fablet.
+
+        Tested manually.
+
+        * platform/mediastream/mac/AVVideoCaptureSource.h: Remove InterruptionReason enum, convert
+        m_interruption to m_interrupted bool
+        * platform/mediastream/mac/AVVideoCaptureSource.mm:
+        (WebCore::AVVideoCaptureSource::AVVideoCaptureSource): Remove InterruptionReason enum checking.
+        (WebCore::AVVideoCaptureSource::stopProducingData): m_interruption -> m_interrupted.
+        (WebCore::AVVideoCaptureSource::interrupted const): Ditto.
+        (WebCore::AVVideoCaptureSource::captureSessionBeginInterruption): Ditto.
+        (WebCore::AVVideoCaptureSource::captureSessionEndInterruption): Ditto. Don't restart the session.
+        (-[WebCoreAVVideoCaptureSourceObserver observeValueForKeyPath:ofObject:change:context:]):
+        Drive-by fix: always log notifications to help with debugging.
+
 2020-04-02  David Kilzer  <ddkilzer@apple.com>
 
         REGRESSION (r258525): Leak of NSMutableAttributedString in -[WebAccessibilityObjectWrapper doAXAttributedStringForTextMarkerRange:spellCheck:]
diff --git a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h
index 2a85ad4..3fb0183 100644
--- a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h
+++ b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h
@@ -57,7 +57,6 @@
 
     WEBCORE_EXPORT static VideoCaptureFactory& factory();
 
-    enum class InterruptionReason { None, VideoNotAllowedInBackground, AudioInUse, VideoInUse, VideoNotAllowedInSideBySide };
     void captureSessionBeginInterruption(RetainPtr<NSNotification>);
     void captureSessionEndInterruption(RetainPtr<NSNotification>);
     void deviceDisconnected(RetainPtr<NSNotification>);
@@ -134,8 +133,8 @@
     RefPtr<AVVideoPreset> m_currentPreset;
     IntSize m_currentSize;
     double m_currentFrameRate;
-    InterruptionReason m_interruption { InterruptionReason::None };
     int m_framesToDropAtStartup { 0 };
+    bool m_interrupted { false };
     bool m_isRunning { false };
 };
 
diff --git a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm
index 804f1c3..13edc35 100644
--- a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm
+++ b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm
@@ -130,13 +130,6 @@
     , m_objcObserver(adoptNS([[WebCoreAVVideoCaptureSourceObserver alloc] initWithCallback:this]))
     , m_device(device)
 {
-#if PLATFORM(IOS_FAMILY)
-    static_assert(static_cast<int>(InterruptionReason::VideoNotAllowedInBackground) == static_cast<int>(AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableInBackground), "InterruptionReason::VideoNotAllowedInBackground is not AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableInBackground as expected");
-    static_assert(static_cast<int>(InterruptionReason::VideoNotAllowedInSideBySide) == AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableWithMultipleForegroundApps, "InterruptionReason::VideoNotAllowedInSideBySide is not AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableWithMultipleForegroundApps as expected");
-    static_assert(static_cast<int>(InterruptionReason::VideoInUse) == AVCaptureSessionInterruptionReasonVideoDeviceInUseByAnotherClient, "InterruptionReason::VideoInUse is not AVCaptureSessionInterruptionReasonVideoDeviceInUseByAnotherClient as expected");
-    static_assert(static_cast<int>(InterruptionReason::AudioInUse) == AVCaptureSessionInterruptionReasonAudioDeviceInUseByAnotherClient, "InterruptionReason::AudioInUse is not AVCaptureSessionInterruptionReasonAudioDeviceInUseByAnotherClient as expected");
-#endif
-
     [m_device.get() addObserver:m_objcObserver.get() forKeyPath:@"suspended" options:NSKeyValueObservingOptionNew context:(void *)nil];
 }
 
@@ -191,7 +184,8 @@
     if ([m_session isRunning])
         [m_session stopRunning];
 
-    m_interruption = InterruptionReason::None;
+    m_interrupted = false;
+
 #if PLATFORM(IOS_FAMILY)
     clearSession();
 #endif
@@ -572,7 +566,7 @@
 
 bool AVVideoCaptureSource::interrupted() const
 {
-    if (m_interruption != InterruptionReason::None)
+    if (m_interrupted)
         return true;
 
     return RealtimeMediaSource::interrupted();
@@ -617,21 +611,13 @@
 void AVVideoCaptureSource::captureSessionBeginInterruption(RetainPtr<NSNotification> notification)
 {
     ALWAYS_LOG_IF(loggerPtr(), LOGIDENTIFIER, [notification.get().userInfo[AVCaptureSessionInterruptionReasonKey] integerValue]);
-    m_interruption = static_cast<AVVideoCaptureSource::InterruptionReason>([notification.get().userInfo[AVCaptureSessionInterruptionReasonKey] integerValue]);
+    m_interrupted = true;
 }
 
 void AVVideoCaptureSource::captureSessionEndInterruption(RetainPtr<NSNotification>)
 {
     ALWAYS_LOG_IF(loggerPtr(), LOGIDENTIFIER);
-
-    InterruptionReason reason = m_interruption;
-
-    m_interruption = InterruptionReason::None;
-    if (reason != InterruptionReason::VideoNotAllowedInSideBySide || m_isRunning || !m_session)
-        return;
-
-    [m_session startRunning];
-    m_isRunning = [m_session isRunning];
+    m_interrupted = false;
 }
 #endif
 
@@ -702,21 +688,22 @@
         return;
 
     id newValue = [change valueForKey:NSKeyValueChangeNewKey];
-
     bool willChange = [[change valueForKey:NSKeyValueChangeNotificationIsPriorKey] boolValue];
 
 #if !RELEASE_LOG_DISABLED
-    if (m_callback->loggerPtr() && m_callback->logger().willLog(m_callback->logChannel(), WTFLogLevel::Debug)) {
+    if (m_callback->loggerPtr()) {
         auto identifier = Logger::LogSiteIdentifier("AVVideoCaptureSource", "observeValueForKeyPath", m_callback->logIdentifier());
-
         RetainPtr<NSString> valueString = adoptNS([[NSString alloc] initWithFormat:@"%@", newValue]);
-        m_callback->logger().debug(m_callback->logChannel(), identifier, "did change '", [keyPath UTF8String], "' to ", [valueString.get() UTF8String]);
+        m_callback->logger().logAlways(m_callback->logChannel(), identifier, willChange ? "will" : "did", " change '", [keyPath UTF8String], "' to ", [valueString.get() UTF8String]);
     }
 #endif
 
-    if (!willChange && [keyPath isEqualToString:@"running"])
+    if (willChange)
+        return;
+
+    if ([keyPath isEqualToString:@"running"])
         m_callback->captureSessionIsRunningDidChange([newValue boolValue]);
-    if (!willChange && [keyPath isEqualToString:@"suspended"])
+    if ([keyPath isEqualToString:@"suspended"])
         m_callback->captureDeviceSuspendedDidChange();
 }