Title: [159698] branches/safari-537.60-branch/Source/WebCore
Revision
159698
Author
[email protected]
Date
2013-11-22 11:14:17 -0800 (Fri, 22 Nov 2013)

Log Message

Merge r159691

    2013-11-21  Brent Fulgham  <[email protected]>

    [Win] Avoid deadlock when interacting with some AVFoundationCF content
    <rdar://problem/15482977> and https://bugs.webkit.org/show_bug.cgi?id=124752

    Prevent deadlock caused by conflict over the "mapLock" mutex. Notification handling in the file,
    which modify assets and make other changes, are required to happen on the main thread. This
    patch enforces this requirement.

    Reviewed by Eric Carlson.

    * platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:
    (WebCore::NotificationCallbackData::NotificationCallbackData): Added
    (WebCore::AVFWrapper::processNotification): Moved logic from 'notificationCallback', which was
    sometimes happening on a background thread.
    (WebCore::AVFWrapper::notificationCallback): Dispatch calls to main thread.

Modified Paths

Diff

Modified: branches/safari-537.60-branch/Source/WebCore/ChangeLog (159697 => 159698)


--- branches/safari-537.60-branch/Source/WebCore/ChangeLog	2013-11-22 18:24:04 UTC (rev 159697)
+++ branches/safari-537.60-branch/Source/WebCore/ChangeLog	2013-11-22 19:14:17 UTC (rev 159698)
@@ -1,3 +1,24 @@
+2013-11-22  Brent Fulgham  <[email protected]>
+
+        Merge r159691
+
+    2013-11-21  Brent Fulgham  <[email protected]>
+
+            [Win] Avoid deadlock when interacting with some AVFoundationCF content
+            <rdar://problem/15482977> and https://bugs.webkit.org/show_bug.cgi?id=124752
+
+            Prevent deadlock caused by conflict over the "mapLock" mutex. Notification handling in the file,
+            which modify assets and make other changes, are required to happen on the main thread. This
+            patch enforces this requirement.
+
+            Reviewed by Eric Carlson.
+
+            * platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:
+            (WebCore::NotificationCallbackData::NotificationCallbackData): Added
+            (WebCore::AVFWrapper::processNotification): Moved logic from 'notificationCallback', which was
+            sometimes happening on a background thread.
+            (WebCore::AVFWrapper::notificationCallback): Dispatch calls to main thread.
+
 2013-11-04  Lucas Forschler  <[email protected]>
 
         Merge r154180

Modified: branches/safari-537.60-branch/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp (159697 => 159698)


--- branches/safari-537.60-branch/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp	2013-11-22 18:24:04 UTC (rev 159697)
+++ branches/safari-537.60-branch/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp	2013-11-22 19:14:17 UTC (rev 159698)
@@ -115,6 +115,7 @@
     static void periodicTimeObserverCallback(AVCFPlayerRef, CMTime, void*);
     static void seekCompletedCallback(AVCFPlayerItemRef, Boolean, void*);
     static void notificationCallback(CFNotificationCenterRef, void*, CFStringRef, const void*, CFDictionaryRef);
+    static void processNotification(void* context);
 
     inline AVCFPlayerLayerRef videoLayer() const { return (AVCFPlayerLayerRef)m_avCFVideoLayer.get(); }
     inline AVCFPlayerRef avPlayer() const { return (AVCFPlayerRef)m_avPlayer.get(); }
@@ -1392,21 +1393,34 @@
     self->m_owner->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::PlayerTimeChanged, time);
 }
 
-void AVFWrapper::notificationCallback(CFNotificationCenterRef, void* observer, CFStringRef propertyName, const void* object, CFDictionaryRef)
+struct NotificationCallbackData {
+    RetainPtr<CFStringRef> m_propertyName;
+    void* m_context;
+
+    NotificationCallbackData(CFStringRef propertyName, void* context)
+        : m_propertyName(propertyName), m_context(context)
+    {
+    }
+};
+
+void AVFWrapper::processNotification(void* context)
 {
+    ASSERT(dispatch_get_main_queue() == dispatch_get_current_queue());
+    ASSERT(context);
+
+    if (!context)
+        return;
+
+    OwnPtr<NotificationCallbackData> notificationData = adoptPtr(reinterpret_cast<NotificationCallbackData*>(context));
+
     MutexLocker locker(mapLock());
-    AVFWrapper* self = avfWrapperForCallbackContext(observer);
-    
+    AVFWrapper* self = avfWrapperForCallbackContext(notificationData->m_context);
     if (!self) {
-        LOG(Media, "AVFWrapper::notificationCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(observer));
+        LOG(Media, "AVFWrapper::processNotification invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
         return;
     }
 
-#if !LOG_DISABLED
-    char notificationName[256];
-    CFStringGetCString(propertyName, notificationName, sizeof(notificationName), kCFStringEncodingASCII);
-    LOG(Media, "AVFWrapper::notificationCallback(%p) %s", self, notificationName);
-#endif
+    CFStringRef propertyName = notificationData->m_propertyName.get();
 
     if (CFEqual(propertyName, AVCFPlayerItemDidPlayToEndTimeNotification))
         self->m_owner->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::ItemDidPlayToEndTime);
@@ -1437,6 +1451,19 @@
         ASSERT_NOT_REACHED();
 }
 
+void AVFWrapper::notificationCallback(CFNotificationCenterRef, void* observer, CFStringRef propertyName, const void* object, CFDictionaryRef)
+{
+#if !LOG_DISABLED
+    char notificationName[256];
+    CFStringGetCString(propertyName, notificationName, sizeof(notificationName), kCFStringEncodingASCII);
+    LOG(Media, "AVFWrapper::notificationCallback(if=%d) %s", reinterpret_cast<uintptr_t>(observer), notificationName);
+#endif
+
+    OwnPtr<NotificationCallbackData> notificationData = adoptPtr(new NotificationCallbackData(propertyName, observer));
+
+    dispatch_async_f(dispatch_get_main_queue(), notificationData.leakPtr(), processNotification);
+}
+
 void AVFWrapper::loadPlayableCompletionCallback(AVCFAssetRef, void* context)
 {
     MutexLocker locker(mapLock());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to