Title: [280766] trunk
Revision
280766
Author
[email protected]
Date
2021-08-08 14:53:12 -0700 (Sun, 08 Aug 2021)

Log Message

REGRESSION: http/tests/preload/onload_event.html is a flaky timeout on Big Sur wk1 Release
https://bugs.webkit.org/show_bug.cgi?id=227366
<rdar://problem/79733280>

Reviewed by Eric Carlson.

Source/WebCore:

Querying for any state on an AVAsset before its fully loaded will cause AVFoundation to block
on networking to fulfill the request, and in the case where WebKit handles loading, will eventually
dispatch to the main thread (which is blocked) in WebCore loader code, causing a deadlock.

Always guard queries to AVAsset with checks for the loaded status of the property being queried.
To make this easier, add a new safeAVAssetTracksForVisualMedia() method, similar to the existing
safeAVAssetTracksForAudibleMedia(), which checks the "tracks" property status and returns an empty
array if the tracks are not yet loaded.

Guard a few more methods by bailing early if the AVAsset is not fully loaded.

* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::tracksChanged):
(WebCore::MediaPlayerPrivateAVFoundationObjC::updateRotationSession):
(WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):
(WebCore::MediaPlayerPrivateAVFoundationObjC::safeAVAssetTracksForVisualMedia):

LayoutTests:

* platform/mac-wk1/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (280765 => 280766)


--- trunk/LayoutTests/ChangeLog	2021-08-08 20:43:12 UTC (rev 280765)
+++ trunk/LayoutTests/ChangeLog	2021-08-08 21:53:12 UTC (rev 280766)
@@ -1,3 +1,13 @@
+2021-08-08  Jer Noble  <[email protected]>
+
+        REGRESSION: http/tests/preload/onload_event.html is a flaky timeout on Big Sur wk1 Release
+        https://bugs.webkit.org/show_bug.cgi?id=227366
+        <rdar://problem/79733280>
+
+        Reviewed by Eric Carlson.
+
+        * platform/mac-wk1/TestExpectations:
+
 2021-08-07  Wenson Hsieh  <[email protected]>
 
         [macOS] Web process crashes when detaching Document with uncommitted marked text

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (280765 => 280766)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-08-08 20:43:12 UTC (rev 280765)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-08-08 21:53:12 UTC (rev 280766)
@@ -1331,8 +1331,6 @@
 [ BigSur ] imported/w3c/web-platform-tests/css/css-will-change/will-change-stacking-context-filter-1.html [ Pass ImageOnlyFailure ]
 [ BigSur ] imported/w3c/web-platform-tests/css/css-will-change/will-change-stacking-context-opacity-1.html [ Pass ImageOnlyFailure ]
 
-webkit.org/b/227366 [ BigSur ] http/tests/preload/onload_event.html [ Pass Timeout ]
-
 # This test relies on BroadcastChannel and has been flaky on WK1 since we added support for BroadcastChannel.
 webkit.org/b/228089 imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html  [ Pass Failure ]
 

Modified: trunk/Source/WebCore/ChangeLog (280765 => 280766)


--- trunk/Source/WebCore/ChangeLog	2021-08-08 20:43:12 UTC (rev 280765)
+++ trunk/Source/WebCore/ChangeLog	2021-08-08 21:53:12 UTC (rev 280766)
@@ -1,3 +1,29 @@
+2021-08-08  Jer Noble  <[email protected]>
+
+        REGRESSION: http/tests/preload/onload_event.html is a flaky timeout on Big Sur wk1 Release
+        https://bugs.webkit.org/show_bug.cgi?id=227366
+        <rdar://problem/79733280>
+
+        Reviewed by Eric Carlson.
+
+        Querying for any state on an AVAsset before its fully loaded will cause AVFoundation to block
+        on networking to fulfill the request, and in the case where WebKit handles loading, will eventually
+        dispatch to the main thread (which is blocked) in WebCore loader code, causing a deadlock.
+
+        Always guard queries to AVAsset with checks for the loaded status of the property being queried.
+        To make this easier, add a new safeAVAssetTracksForVisualMedia() method, similar to the existing
+        safeAVAssetTracksForAudibleMedia(), which checks the "tracks" property status and returns an empty
+        array if the tracks are not yet loaded.
+
+        Guard a few more methods by bailing early if the AVAsset is not fully loaded.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::tracksChanged):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::updateRotationSession):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::safeAVAssetTracksForVisualMedia):
+
 2021-08-07  David Kilzer  <[email protected]>
 
         UBSan: KeyboardScrollingAnimator.cpp:303:10: runtime error: load of value nnn, which is not a valid value for type 'bool'

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (280765 => 280766)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h	2021-08-08 20:43:12 UTC (rev 280765)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h	2021-08-08 21:53:12 UTC (rev 280766)
@@ -273,6 +273,7 @@
     AVMediaSelectionGroup *safeMediaSelectionGroupForVisualMedia();
 
     NSArray *safeAVAssetTracksForAudibleMedia();
+    NSArray *safeAVAssetTracksForVisualMedia();
 
 #if ENABLE(DATACUE_VALUE)
     void processMetadataTrack();

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (280765 => 280766)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2021-08-08 20:43:12 UTC (rev 280765)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2021-08-08 21:53:12 UTC (rev 280766)
@@ -2093,9 +2093,9 @@
     if (!m_avPlayerItem) {
         // We don't have a player item yet, so check with the asset because some assets support inspection
         // prior to becoming ready to play.
-        AVAssetTrack* firstEnabledVideoTrack = firstEnabledTrack([m_avAsset.get() tracksWithMediaCharacteristic:AVMediaCharacteristicVisual]);
+        AVAssetTrack* firstEnabledVideoTrack = firstEnabledTrack(safeAVAssetTracksForVisualMedia());
         setHasVideo(firstEnabledVideoTrack);
-        setHasAudio(firstEnabledTrack([m_avAsset.get() tracksWithMediaCharacteristic:AVMediaCharacteristicAudible]));
+        setHasAudio(firstEnabledTrack(safeAVAssetTracksForAudibleMedia()));
         auto size = firstEnabledVideoTrack ? FloatSize(CGSizeApplyAffineTransform([firstEnabledVideoTrack naturalSize], [firstEnabledVideoTrack preferredTransform])) : FloatSize();
         // For videos with rotation tag set, the transformation above might return a CGSize instance with negative width or height.
         // See https://bugs.webkit.org/show_bug.cgi?id=172648.
@@ -2174,9 +2174,12 @@
 
 void MediaPlayerPrivateAVFoundationObjC::updateRotationSession()
 {
+    if (!m_avAsset || assetStatus() < MediaPlayerAVAssetStatusLoaded)
+        return;
+
     AffineTransform finalTransform = m_avAsset.get().preferredTransform;
     FloatSize naturalSize;
-    if (auto* firstEnabledVideoTrack = firstEnabledTrack([m_avAsset.get() tracksWithMediaCharacteristic:AVMediaCharacteristicVisual])) {
+    if (auto* firstEnabledVideoTrack = firstEnabledTrack(safeAVAssetTracksForVisualMedia())) {
         naturalSize = FloatSize(firstEnabledVideoTrack.naturalSize);
         finalTransform *= firstEnabledVideoTrack.preferredTransform;
     }
@@ -2513,7 +2516,7 @@
     if (!m_lastImage)
         return;
 
-    AVAssetTrack* firstEnabledVideoTrack = firstEnabledTrack([m_avAsset.get() tracksWithMediaCharacteristic:AVMediaCharacteristicVisual]);
+    AVAssetTrack* firstEnabledVideoTrack = firstEnabledTrack(safeAVAssetTracksForVisualMedia());
     if (!firstEnabledVideoTrack)
         return;
 
@@ -2699,6 +2702,17 @@
     return [m_avAsset tracksWithMediaCharacteristic:AVMediaCharacteristicAudible];
 }
 
+NSArray* MediaPlayerPrivateAVFoundationObjC::safeAVAssetTracksForVisualMedia()
+{
+    if (!m_avAsset)
+        return nil;
+
+    if ([m_avAsset.get() statusOfValueForKey:@"tracks" error:NULL] != AVKeyValueStatusLoaded)
+        return nil;
+
+    return [m_avAsset tracksWithMediaCharacteristic:AVMediaCharacteristicVisual];
+}
+
 bool MediaPlayerPrivateAVFoundationObjC::hasLoadedMediaSelectionGroups()
 {
     if (!m_avAsset)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to