<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[165000] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/165000">165000</a></dd>
<dt>Author</dt> <dd>jer.noble@apple.com</dd>
<dt>Date</dt> <dd>2014-03-03 12:47:59 -0800 (Mon, 03 Mar 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationh">trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCh">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCmm">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCh">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCmm">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2014-03-01 Jer Noble <jer.noble@apple.com>
+
+ [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):
+
</ins><span class="cx"> 2014-02-28 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> [MSE] YouTube videos fail to play
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -114,7 +114,7 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>- Notification(WTF::Function<void ()> function)
</del><ins>+ Notification(std::function<void ()> function)
</ins><span class="cx"> : m_type(FunctionType)
</span><span class="cx"> , m_time(0)
</span><span class="cx"> , m_finished(false)
</span><span class="lines">@@ -126,13 +126,13 @@
</span><span class="cx"> bool isValid() { return m_type != None; }
</span><span class="cx"> double time() { return m_time; }
</span><span class="cx"> bool finished() { return m_finished; }
</span><del>- Function<void ()>& function() { return m_function; }
</del><ins>+ std::function<void ()>& function() { return m_function; }
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> Type m_type;
</span><span class="cx"> double m_time;
</span><span class="cx"> bool m_finished;
</span><del>- Function<void ()> m_function;
</del><ins>+ std::function<void ()> m_function;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> void scheduleMainThreadNotification(Notification);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -117,11 +117,11 @@
</span><span class="cx"> void playbackTargetIsWirelessDidChange();
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
</ins><span class="cx"> private:
</span><span class="cx"> MediaPlayerPrivateAVFoundationObjC(MediaPlayer*);
</span><span class="cx">
</span><del>- WeakPtr<MediaPlayerPrivateAVFoundationObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
-
</del><span class="cx"> // engine support
</span><span class="cx"> static PassOwnPtr<MediaPlayerPrivateInterface> create(MediaPlayer*);
</span><span class="cx"> static void getSupportedTypes(HashSet<String>& types);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2192,7 +2192,14 @@
</span><span class="cx"> if (function.isNull())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification(function));
</del><ins>+ 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();
+ }));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP)
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -77,6 +77,8 @@
</span><span class="cx"> void keyNeeded(Uint8Array*);
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
</ins><span class="cx"> private:
</span><span class="cx"> // MediaPlayerPrivateInterface
</span><span class="cx"> virtual void load(const String& url) override;
</span><span class="lines">@@ -153,8 +155,6 @@
</span><span class="cx"> void ensureLayer();
</span><span class="cx"> void destroyLayer();
</span><span class="cx">
</span><del>- WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
-
</del><span class="cx"> // MediaPlayer Factory Methods
</span><span class="cx"> static PassOwnPtr<MediaPlayerPrivateInterface> create(MediaPlayer*);
</span><span class="cx"> static bool isAvailable();
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm (164999 => 165000)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -119,7 +119,12 @@
</span><span class="cx"> static void CMTimebaseEffectiveRateChangedCallback(CMNotificationCenterRef, const void *listener, CFStringRef, const void *, CFTypeRef)
</span><span class="cx"> {
</span><span class="cx"> MediaPlayerPrivateMediaSourceAVFObjC* player = (MediaPlayerPrivateMediaSourceAVFObjC*)listener;
</span><del>- callOnMainThread(bind(&MediaPlayerPrivateMediaSourceAVFObjC::effectiveRateChanged, player));
</del><ins>+ auto weakThis = player->createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->effectiveRateChanged();
+ });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC(MediaPlayer* player)
</span><span class="lines">@@ -297,7 +302,12 @@
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::play()
</span><span class="cx"> {
</span><del>- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::playInternal, this));
</del><ins>+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->playInternal();
+ });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::playInternal()
</span><span class="lines">@@ -308,7 +318,12 @@
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::pause()
</span><span class="cx"> {
</span><del>- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal, this));
</del><ins>+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis]{
+ if (!weakThis)
+ return;
+ weakThis.get()->pauseInternal();
+ });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal()
</span><span class="lines">@@ -388,7 +403,12 @@
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance(double time, double negativeThreshold, double positiveThreshold)
</span><span class="cx"> {
</span><span class="cx"> m_seeking = true;
</span><del>- callOnMainThread(WTF::bind(&MediaPlayerPrivateMediaSourceAVFObjC::seekInternal, this, time, negativeThreshold, positiveThreshold));
</del><ins>+ auto weakThis = createWeakPtr();
+ callOnMainThread([weakThis, time, negativeThreshold, positiveThreshold]{
+ if (!weakThis)
+ return;
+ weakThis.get()->seekInternal(time, negativeThreshold, positiveThreshold);
+ });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateMediaSourceAVFObjC::seekInternal(double time, double negativeThreshold, double positiveThreshold)
</span></span></pre>
</div>
</div>
</body>
</html>