Changeset 274734 in webkit

Timestamp:

Mar 19, 2021 12:55:02 PM (3 years ago)

Author:

jer.noble@apple.com

Message:

Many media/media-fragments/ tests are crashing due to other tests that run before them.
​https://bugs.webkit.org/show_bug.cgi?id=223277
<rdar://problem/75492360>

Reviewed by Eric Carlson.

Source/WebCore:

Work around the crashing function by pre-conforming the fragment identifier according
to the format required by the crashing function.

• platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:

(WebCore::hasBrokenFragmentSupport):
(WebCore::conformFragmentIdentifierForURL):
(WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):

Source/WebCore/PAL:

• pal/cocoa/AVFoundationSoftLink.h:
• pal/cocoa/AVFoundationSoftLink.mm:

LayoutTests:

• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/PAL/ChangeLog (modified) (1 diff)
• Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.h (modified) (1 diff)
• Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.mm (modified) (1 diff)
• Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-03-19  Jer Noble  <jer.noble@apple.com>
+
+        Many media/media-fragments/ tests are crashing due to other tests that run before them.
+        https://bugs.webkit.org/show_bug.cgi?id=223277
+        <rdar://problem/75492360>
+
+        Reviewed by Eric Carlson.
+
+        * platform/mac/TestExpectations:
+
 2021-03-19  Youenn Fablet  <youenn@apple.com>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -17 +17 @@
 media/mac [ Pass ]
 model-element [ Pass ]
-webkit.org/b/223277 media/media-fragments [ Pass Failure ]
 
 fast/forms/search/search-padding-cancel-results-buttons.html [ Pass ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-19  Jer Noble  <jer.noble@apple.com>
+
+        Many media/media-fragments/ tests are crashing due to other tests that run before them.
+        https://bugs.webkit.org/show_bug.cgi?id=223277
+        <rdar://problem/75492360>
+
+        Reviewed by Eric Carlson.
+
+        Work around the crashing function by pre-conforming the fragment identifier according
+        to the format required by the crashing function.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::hasBrokenFragmentSupport):
+        (WebCore::conformFragmentIdentifierForURL):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):
+
 2021-03-19  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/PAL/ChangeLog

@@ +1 @@
+2021-03-19  Jer Noble  <jer.noble@apple.com>
+
+        Many media/media-fragments/ tests are crashing due to other tests that run before them.
+        https://bugs.webkit.org/show_bug.cgi?id=223277
+        <rdar://problem/75492360>
+
+        Reviewed by Eric Carlson.
+
+        * pal/cocoa/AVFoundationSoftLink.h:
+        * pal/cocoa/AVFoundationSoftLink.mm:
+
 2021-03-17  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.h

@@ -38 +38 @@
 
 SOFT_LINK_CLASS_FOR_HEADER(PAL, AVAssetCache)
+SOFT_LINK_CLASS_FOR_HEADER(PAL, AVAssetCollection)
 SOFT_LINK_CLASS_FOR_HEADER(PAL, AVAssetImageGenerator)
 SOFT_LINK_CLASS_FOR_HEADER(PAL, AVAssetReader)

trunk/Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.mm

@@ -82 +82 @@
 SOFT_LINK_CLASS_FOR_SOURCE_WITH_EXPORT(PAL, AVFoundation, AVPlayerLayer, PAL_EXPORT)
 SOFT_LINK_CLASS_FOR_SOURCE_WITH_EXPORT(PAL, AVFoundation, AVURLAsset, PAL_EXPORT)
+SOFT_LINK_CLASS_FOR_SOURCE_OPTIONAL_WITH_EXPORT(PAL, AVFoundation, AVAssetCollection, PAL_EXPORT)
 SOFT_LINK_CLASS_FOR_SOURCE_OPTIONAL_WITH_EXPORT(PAL, AVFoundation, AVAssetReaderSampleReferenceOutput, PAL_EXPORT)
 SOFT_LINK_CLASS_FOR_SOURCE_OPTIONAL_WITH_EXPORT(PAL, AVFoundation, AVAssetResourceLoadingRequest, PAL_EXPORT)

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

@@ -810 +810 @@
 }
 
+static bool hasBrokenFragmentSupport()
+{
+#if PLATFORM(MAC)
+    // On some versions of macOS, Photos.framework has overriden utility methods from AVFoundation that cause
+    // a exception to be thrown when parsing fragment identifiers from a URL. Their implementation requires
+    // an even number of components when splitting the fragment identifier with separator characters ['&','='].
+    // 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();
+        return iterator == results.end() || ++iterator == results.end() || ++iterator != results.end();
+    };
+
+    StringBuilder replacementFragmentIdentifierBuilder;
+    bool firstParameter = true;
+    bool hasInvalidFragmentIdentifier = false;
+
+    for (auto fragmentParameter : url.fragmentIdentifier().splitAllowingEmptyEntries('&')) {
+        if (hasInvalidNumberOfEqualCharacters(fragmentParameter)) {
+            hasInvalidFragmentIdentifier = true;
+            continue;
+        }
+        if (!firstParameter)
+            replacementFragmentIdentifierBuilder.append('&');
+        else
+            firstParameter = false;
+        replacementFragmentIdentifierBuilder.append(fragmentParameter);
+    }
+
+    if (!hasInvalidFragmentIdentifier)
+        return url;
+
+    URL validURL = url;
+    if (replacementFragmentIdentifierBuilder.isEmpty())
+        validURL.removeFragmentIdentifier();
+    else
+        validURL.setFragmentIdentifier(replacementFragmentIdentifierBuilder.toString());
+
+    return validURL;
+#else
+    ASSERT_NOT_REACHED();
+    return url;
+#endif
+}
+
 void MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL(const URL& url, RetainPtr<NSMutableDictionary> options)
 {
@@ -901 +972 @@
         registerFormatReaderIfNecessary();
 
-    NSURL *cocoaURL = canonicalURL(url);
+    NSURL *cocoaURL = nil;
+    if (hasBrokenFragmentSupport() && url.hasFragmentIdentifier())
+        cocoaURL = canonicalURL(conformFragmentIdentifierForURL(url));
+    else
+        cocoaURL = canonicalURL(url);
+
     m_avAsset = adoptNS([PAL::allocAVURLAssetInstance() initWithURL:cocoaURL options:options.get()]);