Changeset 277442 in webkit

Timestamp:

May 13, 2021, 11:23:49 AM (5 years ago)

Author:

jer.noble@apple.com

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-05-13  Jer Noble  <jer.noble@apple.com>
+
+        [ 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  <jenner@apple.com>
 

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -1344 +1344 @@
 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 ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-13  Jer Noble  <jer.noble@apple.com>
+
+        [ 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  <aboya@igalia.com>
 

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

@@ -814 +814 @@
 }
 
-static bool hasBrokenFragmentSupport()
+static URL conformFragmentIdentifierForURL(const URL& url)
 {
 #if PLATFORM(MAC)
@@ -823 +823 @@
     // 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('=');
@@ -976 +952 @@
         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;