Attachment 424545 Details for Bug 222277 – Patch

[patch] Patch

bug-222277-20210329105121.patch (text/plain), 5.05 KB, created by Jer Noble on 2021-03-29 10:51:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2021-03-29 10:51:22 PDT

Size: 5.05 KB

patch

obsolete

>Subversion Revision: 275103
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0316f95a4b78aa0ccf82ff920c371dabbff2554a..52507d6f82be1e44786d98560611dc77c34bbd7a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2021-03-29  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 NOBODY (OOPS!).
>+
>+        In r274734, we attempted to work around a crash due to an uncaught exception when loading an
>+        intentionally malformed fragment identifier through AVURLAsset. However, the code which causes
>+        the exception to be thrown is dynamically loaded, and the method in question replaced by one
>+        which crashes when parsing that fragment.
>+
>+        r274734 attempted to be efficient, by only checking whether calls to parse a malforgmed fragment
>+        would crash once, and thus when the offending code was loaded later, the check had already been
>+        made & succeeded.  Now, we will wrap the call into AVURLAsset with a @try/@catch block, and if
>+        that call generates an exception, we will reset the check boolean and conform the incoming URL
>+        before re-attempting to create the AVURLAsset.
>+
>+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
>+        (WebCore::hasBrokenFragmentSupportValue):
>+        (WebCore::hasBrokenFragmentSupport):
>+        (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):
>+
> 2021-03-28  Jer Noble  <jer.noble@apple.com>
> 
>         [Cocoa] Exception thrown from -setShouldProvideMediaDataForTrackID:
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>index f5b947085570789e3e577f1d266527630c88db7e..91f86c69c58f482a9c3c3f3c3263b1a7574ac9a2 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>@@ -809,6 +809,12 @@ static bool willUseWebMFormatReaderForType(const String& type)
> #endif
> }
> 
>+static Optional<bool> hasBrokenFragmentSupportValue()
>+{
>+    auto value = makeNeverDestroyed<Optional<bool>>();
>+    return value;
>+}
>+
> static bool hasBrokenFragmentSupport()
> {
> #if PLATFORM(MAC)
>@@ -818,20 +824,20 @@ static bool hasBrokenFragmentSupport()
>     // 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;
>+    if (hasBrokenFragmentSupportValue())
>+        return *hasBrokenFragmentSupportValue();
>+
>+    @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&"]];
>+        hasBrokenFragmentSupportValue() = false;
>+    } @catch (NSException *exception) {
>+        hasBrokenFragmentSupportValue() = true;
>+    }
>+    return *hasBrokenFragmentSupportValue();
> #else
>     return false;
> #endif
>@@ -977,7 +983,18 @@ void MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL(const URL& url, Ret
>     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) {
>+        ERROR_LOG(LOGIDENTIFIER, "-[AVURLAsset initWithURL:options:] threw exception: ", exception.description);
>+        if (!url.hasFragmentIdentifier())
>+            return;
>+
>+        hasBrokenFragmentSupportValue() = true;
>+        cocoaURL = canonicalURL(conformFragmentIdentifierForURL(url));
>+        m_avAsset = adoptNS([PAL::allocAVURLAssetInstance() initWithURL:cocoaURL options:options.get()]);
>+    }
>+
> 
>     AVAssetResourceLoader *resourceLoader = m_avAsset.get().resourceLoader;
>     [resourceLoader setDelegate:m_loaderDelegate.get() queue:globalLoaderDelegateQueue()];

Actions: View | Formatted Diff | Diff

Attachments on bug 222277: 421248 | 421459 | 421479 | 424545 | 424546 | 424551 | 428461 | 428517