- Revision
- 165000
- Author
- [email protected]
- Date
- 2014-03-03 12:47:59 -0800 (Mon, 03 Mar 2014)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (164999 => 165000)
--- trunk/Source/WebCore/ChangeLog 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/ChangeLog 2014-03-03 20:47:59 UTC (rev 165000)
@@ -1,3 +1,37 @@
+2014-03-01 Jer Noble <[email protected]>
+
+ [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):
+
2014-02-28 Jer Noble <[email protected]>
[MSE] YouTube videos fail to play
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h (164999 => 165000)
--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h 2014-03-03 20:47:59 UTC (rev 165000)
@@ -114,7 +114,7 @@
{
}
- Notification(WTF::Function<void ()> function)
+ Notification(std::function<void ()> function)
: m_type(FunctionType)
, m_time(0)
, m_finished(false)
@@ -126,13 +126,13 @@
bool isValid() { return m_type != None; }
double time() { return m_time; }
bool finished() { return m_finished; }
- Function<void ()>& function() { return m_function; }
+ std::function<void ()>& function() { return m_function; }
private:
Type m_type;
double m_time;
bool m_finished;
- Function<void ()> m_function;
+ std::function<void ()> m_function;
};
void scheduleMainThreadNotification(Notification);
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (164999 => 165000)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2014-03-03 20:47:59 UTC (rev 165000)
@@ -117,11 +117,11 @@
void playbackTargetIsWirelessDidChange();
#endif
+ WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
private:
MediaPlayerPrivateAVFoundationObjC(MediaPlayer*);
- WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
-
// engine support
static PassOwnPtr<MediaPlayerPrivateInterface> create(MediaPlayer*);
static void getSupportedTypes(HashSet<String>& types);
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (164999 => 165000)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2014-03-03 20:47:59 UTC (rev 165000)
@@ -2192,7 +2192,14 @@
if (function.isNull())
return;
- m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification(function));
+ auto weakThis = m_callback->createWeakPtr();
+ m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification([weakThis, function]{
+ // weakThis and function both refer to the same MediaPlayerPrivateAVFoundationObjC instance. If the WeakPtr has
+ // been cleared, the underlying object has been destroyed, and it is unsafe to call function().
+ if (!weakThis)
+ return;
+ function();
+ }));
}
#if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP)
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h (164999 => 165000)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h 2014-03-03 20:47:59 UTC (rev 165000)
@@ -77,6 +77,8 @@
void keyNeeded(Uint8Array*);
#endif
+ WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
private:
// MediaPlayerPrivateInterface
virtual void load(const String& url) override;
@@ -153,8 +155,6 @@
void ensureLayer();
void destroyLayer();
- WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
-
// MediaPlayer Factory Methods
static PassOwnPtr<MediaPlayerPrivateInterface> create(MediaPlayer*);
static bool isAvailable();
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm (164999 => 165000)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2014-03-03 20:47:52 UTC (rev 164999)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2014-03-03 20:47:59 UTC (rev 165000)
@@ -119,7 +119,12 @@
static void CMTimebaseEffectiveRateChangedCallback(CMNotificationCenterRef, const void *listener, CFStringRef, const void *, CFTypeRef)
{
MediaPlayerPrivateMediaSourceAVFObjC* player = (MediaPlayerPrivateMediaSourceAVFObjC*)listener;
- callOnMainThread(bind(&MediaPlayerPrivateMediaSourceAVFObjC::effectiveRateChanged, player));
+ auto weakThis = player->createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->effectiveRateChanged();
+ });
}
MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC(MediaPlayer* player)
@@ -297,7 +302,12 @@
void MediaPlayerPrivateMediaSourceAVFObjC::play()
{
- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::playInternal, this));
+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->playInternal();
+ });
}
void MediaPlayerPrivateMediaSourceAVFObjC::playInternal()
@@ -308,7 +318,12 @@
void MediaPlayerPrivateMediaSourceAVFObjC::pause()
{
- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal, this));
+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->pauseInternal();
+ });
}
void MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal()
@@ -388,7 +403,12 @@
void MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance(double time, double negativeThreshold, double positiveThreshold)
{
m_seeking = true;
- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::seekInternal, this, time, negativeThreshold, positiveThreshold));
+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis, time, negativeThreshold, positiveThreshold]{
+ if (!weakThis)
+ return;
+ weakThis.get()->seekInternal(time, negativeThreshold, positiveThreshold);
+ });
}
void MediaPlayerPrivateMediaSourceAVFObjC::seekInternal(double time, double negativeThreshold, double positiveThreshold)
