Title: [277442] trunk
Revision
277442
Author
[email protected]
Date
2021-05-13 11:23:49 -0700 (Thu, 13 May 2021)

Log Message

[ macOS Wk2 ] media/media-fragments/TC0051.html is flakey crashing
https://bugs.webkit.org/show_bug.cgi?id=222277
<rdar://problem/74600790>

Reviewed by Eric Carlson.

Source/WebCore:

In r274734, a workaround was added to detect a bug in the Photos.framework that would cause
an exception to be thrown if the URL provided to AVURLAsset had a malformed fragment identifier.
However, in an effort to reduce the runtime cost of this check, it was only done once, the first
time an AVURLAsset is created. This failed to account for the Photos.framework being dynamically
loaded and changing the behavior of AVURLAsset after the check had been performed.

Instead, fall back to a much simpler check: wrap the creation of the AVURLAsset in a @try/@catch
block, and if an exception was thrown, re-attempt to create the AVURLAsset with a manually
conformed URL.

* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::conformFragmentIdentifierForURL):
(WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):
(WebCore::hasBrokenFragmentSupport): Deleted.

LayoutTests:

* platform/mac-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (277441 => 277442)


--- trunk/LayoutTests/ChangeLog	2021-05-13 18:16:14 UTC (rev 277441)
+++ trunk/LayoutTests/ChangeLog	2021-05-13 18:23:49 UTC (rev 277442)
@@ -1,3 +1,13 @@
+2021-05-13  Jer Noble  <[email protected]>
+
+        [ macOS Wk2 ] media/media-fragments/TC0051.html is flakey crashing
+        https://bugs.webkit.org/show_bug.cgi?id=222277
+        <rdar://problem/74600790>
+
+        Reviewed by Eric Carlson.
+
+        * platform/mac-wk2/TestExpectations:
+
 2021-05-13  Robert Jenner  <[email protected]>
 
         [ Catalina+ wk2 ] http/tests/webAPIStatistics/screen-functions-accessed-data-collection.html (layout-test) is a flaky timeout

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (277441 => 277442)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2021-05-13 18:16:14 UTC (rev 277441)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2021-05-13 18:23:49 UTC (rev 277442)
@@ -1343,8 +1343,6 @@
 
 webkit.org/b/224032 [ arm64 ] tiled-drawing/top-content-inset-fixed-attachment-cover-local.html [ Pass ImageOnlyFailure ]
 
-webkit.org/b/222277 media/media-fragments [ Skip ]
-
 webkit.org/b/224135 media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-drag-is-prevented-over-button.html [ Pass Timeout ]
 
 webkit.org/b/224294 [ Debug ] inspector/indexeddb/requestDatabaseNames.html [ Pass Failure ]

Modified: trunk/Source/WebCore/ChangeLog (277441 => 277442)


--- trunk/Source/WebCore/ChangeLog	2021-05-13 18:16:14 UTC (rev 277441)
+++ trunk/Source/WebCore/ChangeLog	2021-05-13 18:23:49 UTC (rev 277442)
@@ -1,3 +1,26 @@
+2021-05-13  Jer Noble  <[email protected]>
+
+        [ macOS Wk2 ] media/media-fragments/TC0051.html is flakey crashing
+        https://bugs.webkit.org/show_bug.cgi?id=222277
+        <rdar://problem/74600790>
+
+        Reviewed by Eric Carlson.
+
+        In r274734, a workaround was added to detect a bug in the Photos.framework that would cause
+        an exception to be thrown if the URL provided to AVURLAsset had a malformed fragment identifier.
+        However, in an effort to reduce the runtime cost of this check, it was only done once, the first
+        time an AVURLAsset is created. This failed to account for the Photos.framework being dynamically
+        loaded and changing the behavior of AVURLAsset after the check had been performed.
+
+        Instead, fall back to a much simpler check: wrap the creation of the AVURLAsset in a @try/@catch
+        block, and if an exception was thrown, re-attempt to create the AVURLAsset with a manually
+        conformed URL.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::conformFragmentIdentifierForURL):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):
+        (WebCore::hasBrokenFragmentSupport): Deleted.
+
 2021-05-13  Alicia Boya GarcĂ­a  <[email protected]>
 
         [WTF] Add holdLock() overload for WTF::DataMutex

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


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2021-05-13 18:16:14 UTC (rev 277441)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2021-05-13 18:23:49 UTC (rev 277442)
@@ -813,7 +813,7 @@
 #endif
 }
 
-static bool hasBrokenFragmentSupport()
+static URL conformFragmentIdentifierForURL(const URL& url)
 {
 #if PLATFORM(MAC)
     // On some versions of macOS, Photos.framework has overriden utility methods from AVFoundation that cause
@@ -822,30 +822,6 @@
     // Work around this broken implementation by pre-parsing the fragment and ensuring that it meets their
     // criteria. Problematic strings from the TC0051.html test include "t=3&", and this problem generally is
     // with subtrings between the '&' character not including an equal sign.
-    static bool hasBrokenFragmentSupport = false;
-    static dispatch_once_t onceToken;
-    dispatch_once(&onceToken, ^{
-        @try {
-            auto selector = NSSelectorFromString(@"isURLForAssetInCollection:");
-            auto theClass = PAL::getAVAssetCollectionClass();
-            if (![theClass respondsToSelector:selector])
-                return;
-            [theClass performSelector:selector withObject:[NSURL URLWithString:@"file:///invalid-file.mp4#t=3&"]];
-        } @catch (NSException *exception) {
-            hasBrokenFragmentSupport = true;
-        }
-    });
-    return hasBrokenFragmentSupport;
-#else
-    return false;
-#endif
-}
-
-static URL conformFragmentIdentifierForURL(const URL& url)
-{
-#if PLATFORM(MAC)
-    ASSERT(hasBrokenFragmentSupport());
-
     auto hasInvalidNumberOfEqualCharacters = [](const StringView& fragmentParameter) {
         auto results = fragmentParameter.splitAllowingEmptyEntries('=');
         auto iterator = results.begin();
@@ -975,13 +951,23 @@
     if (willUseWebMFormatReader)
         registerFormatReaderIfNecessary();
 
-    NSURL *cocoaURL = nil;
-    if (hasBrokenFragmentSupport() && url.hasFragmentIdentifier())
+    NSURL *cocoaURL = canonicalURL(url);
+
+    @try {
+        m_avAsset = adoptNS([PAL::allocAVURLAssetInstance() initWithURL:cocoaURL options:options.get()]);
+    } @catch(NSException *exception) {
+        ERROR_LOG(LOGIDENTIFIER, "-[AVURLAssetInstance initWithURL:cocoaURL options:] threw an exception: ", [[exception name] UTF8String], ", reason : ", [[exception reason] UTF8String]);
         cocoaURL = canonicalURL(conformFragmentIdentifierForURL(url));
-    else
-        cocoaURL = canonicalURL(url);
 
-    m_avAsset = adoptNS([PAL::allocAVURLAssetInstance() initWithURL:cocoaURL options:options.get()]);
+        @try {
+            m_avAsset = adoptNS([PAL::allocAVURLAssetInstance() initWithURL:cocoaURL options:options.get()]);
+        } @catch(NSException *exception) {
+            ASSERT_NOT_REACHED();
+            ERROR_LOG(LOGIDENTIFIER, "-[AVURLAssetInstance initWithURL:cocoaURL options:] threw a second exception, bailing: ", [[exception name] UTF8String], ", reason : ", [[exception reason] UTF8String]);
+            setNetworkState(MediaPlayer::NetworkState::FormatError);
+            return;
+        }
+    }
 
     AVAssetResourceLoader *resourceLoader = m_avAsset.get().resourceLoader;
     [resourceLoader setDelegate:m_loaderDelegate.get() queue:globalLoaderDelegateQueue()];
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to