Changeset 165000 in webkit


Ignore:
Timestamp:
Mar 3, 2014, 12:47:59 PM (11 years ago)
Author:
jer.noble@apple.com
Message:

[Mac] Crash in MediaPlayer::rateChanged()
​https://bugs.webkit.org/show_bug.cgi?id=129548

Reviewed by Darin Adler.

WTF::bind will automatically ref the parameters added to it. But MediaPlayerPrivate-
AVFoundation and -MediaSOurceAVFObjC are not RefCounted, so by the time the bound
function is called, the underlying objects may have been freed.

Replace or augment callOnMainThread arguments with lambdas and weakPtrs so that
if the argument has been destroyed, its methods will not be called.

Make the MediaPlayerPrivateAVFoundation::Notification function type a std::function:

  • platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:

(WebCore::MediaPlayerPrivateAVFoundation::Notification::Notification):
(WebCore::MediaPlayerPrivateAVFoundation::Notification::function):

Make createWeakPtr() public so that it can be called from non-class methods:

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

(WebCore::MediaPlayerPrivateAVFoundationObjC::createWeakPtr):

  • platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:

(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::createWeakPtr):

Use a weakPtr to abort callOnMainThread() if the object has been destroyed:

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

(-[WebCoreAVFMovieObserver observeValueForKeyPath:ofObject:change:context:]):

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

(WebCore::CMTimebaseEffectiveRateChangedCallback):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance):

Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r164999 r165000  
     12014-03-01  Jer Noble  <jer.noble@apple.com>
     2
     3        [Mac] Crash in MediaPlayer::rateChanged()
     4        https://bugs.webkit.org/show_bug.cgi?id=129548
     5
     6        Reviewed by Darin Adler.
     7
     8        WTF::bind will automatically ref the parameters added to it. But MediaPlayerPrivate-
     9        AVFoundation and -MediaSOurceAVFObjC are not RefCounted, so by the time the bound
     10        function is called, the underlying objects may have been freed.
     11
     12        Replace or augment callOnMainThread arguments with lambdas and weakPtrs so that
     13        if the argument has been destroyed, its methods will not be called.
     14
     15        Make the MediaPlayerPrivateAVFoundation::Notification function type a std::function:
     16        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
     17        (WebCore::MediaPlayerPrivateAVFoundation::Notification::Notification):
     18        (WebCore::MediaPlayerPrivateAVFoundation::Notification::function):
     19
     20        Make createWeakPtr() public so that it can be called from non-class methods:
     21        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
     22        (WebCore::MediaPlayerPrivateAVFoundationObjC::createWeakPtr):
     23        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
     24        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::createWeakPtr):
     25
     26        Use a weakPtr to abort callOnMainThread() if the object has been destroyed:
     27        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
     28        (-[WebCoreAVFMovieObserver observeValueForKeyPath:ofObject:change:context:]):
     29        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
     30        (WebCore::CMTimebaseEffectiveRateChangedCallback):
     31        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
     32        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
     33        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance):
     34
    1352014-02-28  Jer Noble  <jer.noble@apple.com>
    236

  • trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h

    r164661 r165000  
    115115        }
    116116
    117         Notification(WTF::Function<void ()> function)
     117        Notification(std::function<void ()> function)
    118118            : m_type(FunctionType)
    119119            , m_time(0)
    … …  
    127127        double time() { return m_time; }
    128128        bool finished() { return m_finished; }
    129         Function<void ()>& function() { return m_function; }
     129        std::function<void ()>& function() { return m_function; }
    130130       
    131131    private:
    … …  
    133133        double m_time;
    134134        bool m_finished;
    135         Function<void ()> m_function;
     135        std::function<void ()> m_function;
    136136    };
    137137

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h

    r164661 r165000  
    118118#endif
    119119
     120    WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
     121
    120122private:
    121123    MediaPlayerPrivateAVFoundationObjC(MediaPlayer*);
    122 
    123     WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
    124124
    125125    // engine support

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

    r164661 r165000  
    21932193        return;
    21942194
    2195     m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification(function));
     2195    auto weakThis = m_callback->createWeakPtr();
     2196    m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification([weakThis, function]{
     2197        // weakThis and function both refer to the same MediaPlayerPrivateAVFoundationObjC instance. If the WeakPtr has
     2198        // been cleared, the underlying object has been destroyed, and it is unsafe to call function().
     2199        if (!weakThis)
     2200            return;
     2201        function();
     2202    }));
    21962203}
    21972204

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h

    r164529 r165000  
    7878#endif
    7979
     80    WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
     81
    8082private:
    8183    // MediaPlayerPrivateInterface
    … …  
    154156    void destroyLayer();
    155157
    156     WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
    157 
    158158    // MediaPlayer Factory Methods
    159159    static PassOwnPtr<MediaPlayerPrivateInterface> create(MediaPlayer*);

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm

    r164529 r165000  
    120120{
    121121    MediaPlayerPrivateMediaSourceAVFObjC* player = (MediaPlayerPrivateMediaSourceAVFObjC*)listener;
    122     callOnMainThread(bind(&MediaPlayerPrivateMediaSourceAVFObjC::effectiveRateChanged, player));
     122    auto weakThis = player->createWeakPtr();
     123    callOnMainThread([weakThis]{
     124        if (!weakThis)
     125            return;
     126        weakThis.get()->effectiveRateChanged();
     127    });
    123128}
    124129
    … …  
    298303void MediaPlayerPrivateMediaSourceAVFObjC::play()
    299304{
    300     callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::playInternal, this));
     305    auto weakThis = createWeakPtr();
     306    callOnMainThread([weakThis]{
     307        if (!weakThis)
     308            return;
     309        weakThis.get()->playInternal();
     310    });
    301311}
    302312
    … …  
    309319void MediaPlayerPrivateMediaSourceAVFObjC::pause()
    310320{
    311     callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal, this));
     321    auto weakThis = createWeakPtr();
     322    callOnMainThread([weakThis]{
     323        if (!weakThis)
     324            return;
     325        weakThis.get()->pauseInternal();
     326    });
    312327}
    313328
    … …  
    389404{
    390405    m_seeking = true;
    391     callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::seekInternal, this, time, negativeThreshold, positiveThreshold));
     406    auto weakThis = createWeakPtr();
     407    callOnMainThread([weakThis, time, negativeThreshold, positiveThreshold]{
     408        if (!weakThis)
     409            return;
     410        weakThis.get()->seekInternal(time, negativeThreshold, positiveThreshold);
     411    });
    392412}
    393413
Note: See TracChangeset for help on using the changeset viewer.