Title: [274378] trunk/Source/WebKit
Revision
274378
Author
jer.no...@apple.com
Date
2021-03-12 18:29:53 -0800 (Fri, 12 Mar 2021)

Log Message

[Cocoa][WebM] Hang when reloading page before WebM content is loaded
https://bugs.webkit.org/show_bug.cgi?id=223139
<rdar://75351029>

Reviewed by Darin Adler.

No new tests; a truly deterministic test would require a .cgi script to block loading after
the WebM init segment, but a bug in the platform format reader causes URLs not ending in
.webm to fail to load the format reader plugin. Once this issue is fixed, we can write a
test to cover this behavior.

The WebM TrackEntry "enabled" bit is optional, and WebKit previously waited until any media data
was appended to say whether or not the track is enabled in the absense of an explicit signal.
Instead, assume any track that is not explicity disabled is enabled, for the purpose of the
format reader. This means that "enabled" queries will no longer block, which breaks the deadlock
when tearing down the AVAsset backing the WebM file.

* Shared/mac/MediaFormatReader/MediaTrackReader.cpp:
(WebKit::MediaTrackReader::copyProperty):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (274377 => 274378)


--- trunk/Source/WebKit/ChangeLog	2021-03-13 02:26:50 UTC (rev 274377)
+++ trunk/Source/WebKit/ChangeLog	2021-03-13 02:29:53 UTC (rev 274378)
@@ -1,3 +1,25 @@
+2021-03-12  Jer Noble  <jer.no...@apple.com>
+
+        [Cocoa][WebM] Hang when reloading page before WebM content is loaded
+        https://bugs.webkit.org/show_bug.cgi?id=223139
+        <rdar://75351029>
+
+        Reviewed by Darin Adler.
+
+        No new tests; a truly deterministic test would require a .cgi script to block loading after
+        the WebM init segment, but a bug in the platform format reader causes URLs not ending in
+        .webm to fail to load the format reader plugin. Once this issue is fixed, we can write a
+        test to cover this behavior.
+
+        The WebM TrackEntry "enabled" bit is optional, and WebKit previously waited until any media data
+        was appended to say whether or not the track is enabled in the absense of an explicit signal.
+        Instead, assume any track that is not explicity disabled is enabled, for the purpose of the
+        format reader. This means that "enabled" queries will no longer block, which breaks the deadlock
+        when tearing down the AVAsset backing the WebM file.
+
+        * Shared/mac/MediaFormatReader/MediaTrackReader.cpp:
+        (WebKit::MediaTrackReader::copyProperty):
+
 2021-03-12  Chris Fleizach  <cfleiz...@apple.com>
 
         AX: PDF frame conversion routines need to be updated

Modified: trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaTrackReader.cpp (274377 => 274378)


--- trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaTrackReader.cpp	2021-03-13 02:26:50 UTC (rev 274377)
+++ trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaTrackReader.cpp	2021-03-13 02:29:53 UTC (rev 274378)
@@ -157,9 +157,11 @@
 
 OSStatus MediaTrackReader::copyProperty(CFStringRef key, CFAllocatorRef allocator, void* copiedValue)
 {
-    // Don't block waiting for media if the we know the enabled state.
-    if (CFEqual(key, PAL::get_MediaToolbox_kMTPluginTrackReaderProperty_Enabled()) && m_isEnabled != Enabled::Unknown) {
-        *reinterpret_cast<CFBooleanRef*>(copiedValue) = retainPtr(m_isEnabled == Enabled::True ? kCFBooleanTrue : kCFBooleanFalse).leakRef();
+    // Don't block waiting for media to answer requests for the enabled state.
+    if (CFEqual(key, PAL::get_MediaToolbox_kMTPluginTrackReaderProperty_Enabled())) {
+        // Assume that a track without an explicit enabled state is enabled by default,
+        // to avoid blocking waiting for media data to be appended.
+        *reinterpret_cast<CFBooleanRef*>(copiedValue) = m_isEnabled == Enabled::False ? kCFBooleanFalse : kCFBooleanTrue;
         return noErr;
     }
 
@@ -170,17 +172,6 @@
 
     auto& sampleMap = m_sampleStorage->sampleMap;
 
-    if (CFEqual(key, PAL::get_MediaToolbox_kMTPluginTrackReaderProperty_Enabled())) {
-        if (m_isEnabled == Enabled::Unknown) {
-            m_isEnabled = sampleMap.empty() ? Enabled::False : Enabled::True;
-            if (m_isEnabled == Enabled::False)
-                ERROR_LOG(LOGIDENTIFIER, "ignoring empty ", mediaTypeString(), " track");
-        }
-
-        *reinterpret_cast<CFBooleanRef*>(copiedValue) = retainPtr(m_isEnabled == Enabled::True ? kCFBooleanTrue : kCFBooleanFalse).leakRef();
-        return noErr;
-    }
-
     if (sampleMap.empty()) {
         ERROR_LOG(LOGIDENTIFIER, "sample table empty when asked for ", String(key));
         return kCMBaseObjectError_ValueNotAvailable;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to