Log Message
Merge r168974.
Modified Paths
- branches/safari-538.34-branch/Source/WebCore/ChangeLog
- branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
- branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.h
- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm
- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h
- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
Diff
Modified: branches/safari-538.34-branch/Source/WebCore/ChangeLog (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/ChangeLog 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/ChangeLog 2014-05-21 00:03:56 UTC (rev 169140)
@@ -1,5 +1,55 @@
2014-05-20 Matthew Hanson <[email protected]>
+ Merge r168974.
+
+ 2014-05-16 Jer Noble <[email protected]>
+
+ [MSE] Crash at WebCore::SourceBuffer::~SourceBuffer + 110
+ https://bugs.webkit.org/show_bug.cgi?id=132973
+
+ Reviewed by Eric Carlson.
+
+ Change SourceBuffer::m_private into a Ref<>, and add an assertion to
+ SourceBufferPrivateAVFObjC's destructor if its client has not been cleared.
+
+ Eliminate unnecessary churn in MediaSourcePrivateAVFObjC by having the predicate
+ functor take bare pointers, rather than a PassRefPtr.
+
+ The underlying problem seems to be in WebAVStreamDataParserListener. RefPtrs were
+ being created off the main thread to a non-thread safe ref counted class. In some
+ situations, this would result in double decrementing the ref, which would cause an
+ early destruction of the underlying object. Instead replace these RefPtr strong
+ pointers with explicit weak ones. Ensure the parser and its delegate are not freed
+ before the append operation completes by passing strong pointers into the async
+ append operation lambda.
+
+ There were a few places where we weren't null checking m_mediaSource before using it,
+ and at least one place where we weren't clearing m_mediaSource.
+
+ * Modules/mediasource/SourceBuffer.cpp:
+ (WebCore::SourceBuffer::SourceBuffer): Use Ref instead of RefPtr.
+ (WebCore::SourceBuffer::appendBufferTimerFired): Ditto.
+ * Modules/mediasource/SourceBuffer.h:
+ * platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
+ (WebCore::MediaSourcePrivateAVFObjCHasAudio): Take a bare pointer, instead of a PassRefPtr.
+ (WebCore::MediaSourcePrivateAVFObjCHasVideo): Ditto.
+ * platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
+ (WebCore::MediaSourcePrivateAVFObjC::removeSourceBuffer): Clear the back pointer when removing a buffer.
+ * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
+ * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+ (-[WebAVStreamDataParserListener initWithParser:parent:WebCore::]): Use WeakPtr instead of RefPtr.
+ (-[WebAVStreamDataParserListener invalidate]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didParseStreamDataAsAsset:]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didParseStreamDataAsAsset:withDiscontinuity:]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didFailToParseStreamDataWithError:]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didProvideMediaData:forTrackID:mediaType:flags:]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didReachEndOfTrackWithTrackID:mediaType:]): Ditto.
+ (-[WebAVStreamDataParserListener streamDataParser:didProvideContentKeyRequestInitializationData:forTrackID:]): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::~SourceBufferPrivateAVFObjC):
+ (WebCore::SourceBufferPrivateAVFObjC::append): Ditto.
+
+2014-05-20 Matthew Hanson <[email protected]>
+
Merge r168599.
2014-05-09 Myles C. Maxfield <[email protected]>
Modified: branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2014-05-21 00:03:56 UTC (rev 169140)
@@ -107,7 +107,6 @@
, m_pendingRemoveEnd(MediaTime::invalidTime())
, m_removeTimer(this, &SourceBuffer::removeTimerFired)
{
- ASSERT(m_private);
ASSERT(m_source);
m_private->setClient(this);
@@ -474,7 +473,7 @@
// 1. Loop Top: If the input buffer is empty, then jump to the need more data step below.
if (!m_pendingAppendData.size()) {
- sourceBufferPrivateAppendComplete(m_private.get(), AppendSucceeded);
+ sourceBufferPrivateAppendComplete(&m_private.get(), AppendSucceeded);
return;
}
Modified: branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.h (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.h 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/Modules/mediasource/SourceBuffer.h 2014-05-21 00:03:56 UTC (rev 169140)
@@ -156,7 +156,7 @@
void removeTimerFired(Timer<SourceBuffer>*);
void removeCodedFrames(const MediaTime& start, const MediaTime& end);
- RefPtr<SourceBufferPrivate> m_private;
+ Ref<SourceBufferPrivate> m_private;
MediaSource* m_source;
GenericEventQueue m_asyncEventQueue;
Modified: branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm 2014-05-21 00:03:56 UTC (rev 169140)
@@ -85,6 +85,7 @@
m_activeSourceBuffers.remove(pos);
pos = m_sourceBuffers.find(buffer);
+ m_sourceBuffers[pos]->clearMediaSource();
m_sourceBuffers.remove(pos);
}
@@ -157,9 +158,8 @@
}
#endif
-static bool MediaSourcePrivateAVFObjCHasAudio(PassRefPtr<SourceBufferPrivateAVFObjC> prpSourceBuffer)
+static bool MediaSourcePrivateAVFObjCHasAudio(SourceBufferPrivateAVFObjC* sourceBuffer)
{
- RefPtr<SourceBufferPrivateAVFObjC> sourceBuffer = prpSourceBuffer;
return sourceBuffer->hasAudio();
}
@@ -168,9 +168,8 @@
return std::any_of(m_activeSourceBuffers.begin(), m_activeSourceBuffers.end(), MediaSourcePrivateAVFObjCHasAudio);
}
-static bool MediaSourcePrivateAVFObjCHasVideo(PassRefPtr<SourceBufferPrivateAVFObjC> prpSourceBuffer)
+static bool MediaSourcePrivateAVFObjCHasVideo(SourceBufferPrivateAVFObjC* sourceBuffer)
{
- RefPtr<SourceBufferPrivateAVFObjC> sourceBuffer = prpSourceBuffer;
return sourceBuffer->hasVideo();
}
Modified: branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h 2014-05-21 00:03:56 UTC (rev 169140)
@@ -36,6 +36,7 @@
#include <wtf/RefPtr.h>
#include <wtf/RetainPtr.h>
#include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
#include <wtf/text/AtomicString.h>
OBJC_CLASS AVAsset;
@@ -113,9 +114,13 @@
void destroyParser();
void destroyRenderers();
+ WeakPtr<SourceBufferPrivateAVFObjC> createWeakPtr() { return m_weakFactory.createWeakPtr(); }
+
Vector<RefPtr<VideoTrackPrivateMediaSourceAVFObjC>> m_videoTracks;
Vector<RefPtr<AudioTrackPrivateMediaSourceAVFObjC>> m_audioTracks;
+ WeakPtrFactory<SourceBufferPrivateAVFObjC> m_weakFactory;
+
RetainPtr<AVStreamDataParser> m_parser;
RetainPtr<AVAsset> m_asset;
RetainPtr<AVSampleBufferDisplayLayer> m_displayLayer;
Modified: branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm (169139 => 169140)
--- branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2014-05-20 22:55:49 UTC (rev 169139)
+++ branches/safari-538.34-branch/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2014-05-21 00:03:56 UTC (rev 169140)
@@ -153,14 +153,14 @@
#pragma mark WebAVStreamDataParserListener
@interface WebAVStreamDataParserListener : NSObject {
- WebCore::SourceBufferPrivateAVFObjC* _parent;
+ WeakPtr<WebCore::SourceBufferPrivateAVFObjC> _parent;
AVStreamDataParser* _parser;
}
-- (id)initWithParser:(AVStreamDataParser*)parser parent:(WebCore::SourceBufferPrivateAVFObjC*)parent;
+- (id)initWithParser:(AVStreamDataParser*)parser parent:(WeakPtr<WebCore::SourceBufferPrivateAVFObjC>)parent;
@end
@implementation WebAVStreamDataParserListener
-- (id)initWithParser:(AVStreamDataParser*)parser parent:(WebCore::SourceBufferPrivateAVFObjC*)parent
+- (id)initWithParser:(AVStreamDataParser*)parser parent:(WeakPtr<WebCore::SourceBufferPrivateAVFObjC>)parent
{
self = [super init];
if (!self)
@@ -182,7 +182,6 @@
- (void)invalidate
{
[_parser setDelegate:nil];
- _parent = nullptr;
_parser = nullptr;
}
@@ -192,13 +191,12 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
RetainPtr<AVAsset*> strongAsset = asset;
- callOnMainThread([strongParent, strongAsset] {
- strongParent->didParseStreamDataAsAsset(strongAsset.get());
+ callOnMainThread([strongSelf, strongAsset] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didParseStreamDataAsAsset(strongAsset.get());
});
}
@@ -209,13 +207,12 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
RetainPtr<AVAsset*> strongAsset = asset;
- callOnMainThread([strongParent, strongAsset] {
- strongParent->didParseStreamDataAsAsset(strongAsset.get());
+ callOnMainThread([strongSelf, strongAsset] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didParseStreamDataAsAsset(strongAsset.get());
});
}
@@ -225,13 +222,12 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
RetainPtr<NSError> strongError = error;
- callOnMainThread([strongParent, strongError] {
- strongParent->didFailToParseStreamDataWithError(strongError.get());
+ callOnMainThread([strongSelf, strongError] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didFailToParseStreamDataWithError(strongError.get());
});
}
@@ -241,14 +237,13 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
RetainPtr<CMSampleBufferRef> strongSample = sample;
String mediaType = nsMediaType;
- callOnMainThread([strongParent, strongSample, trackID, mediaType, flags] {
- strongParent->didProvideMediaDataForTrackID(trackID, strongSample.get(), mediaType, flags);
+ callOnMainThread([strongSelf, strongSample, trackID, mediaType, flags] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didProvideMediaDataForTrackID(trackID, strongSample.get(), mediaType, flags);
});
}
@@ -258,13 +253,12 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
String mediaType = nsMediaType;
- callOnMainThread([strongParent, trackID, mediaType] {
- strongParent->didReachEndOfTrackWithTrackID(trackID, mediaType);
+ callOnMainThread([strongSelf, trackID, mediaType] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didReachEndOfTrackWithTrackID(trackID, mediaType);
});
}
@@ -274,13 +268,12 @@
UNUSED_PARAM(streamDataParser);
#endif
ASSERT(streamDataParser == _parser);
- RefPtr<WebCore::SourceBufferPrivateAVFObjC> strongParent = _parent;
- if (!strongParent)
- return;
+ RetainPtr<WebAVStreamDataParserListener> strongSelf = self;
RetainPtr<NSData> strongData = initData;
- callOnMainThread([strongParent, strongData, trackID] {
- strongParent->didProvideContentKeyRequestInitializationDataForTrackID(strongData.get(), trackID);
+ callOnMainThread([strongSelf, strongData, trackID] {
+ if (strongSelf->_parent)
+ strongSelf->_parent->didProvideContentKeyRequestInitializationDataForTrackID(strongData.get(), trackID);
});
}
@end
@@ -386,8 +379,9 @@
}
SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC(MediaSourcePrivateAVFObjC* parent)
- : m_parser(adoptNS([[getAVStreamDataParserClass() alloc] init]))
- , m_delegate(adoptNS([[WebAVStreamDataParserListener alloc] initWithParser:m_parser.get() parent:this]))
+ : m_weakFactory(this)
+ , m_parser(adoptNS([[getAVStreamDataParserClass() alloc] init]))
+ , m_delegate(adoptNS([[WebAVStreamDataParserListener alloc] initWithParser:m_parser.get() parent:createWeakPtr()]))
, m_mediaSource(parent)
, m_client(0)
, m_parsingSucceeded(true)
@@ -398,6 +392,7 @@
SourceBufferPrivateAVFObjC::~SourceBufferPrivateAVFObjC()
{
+ ASSERT(!m_client);
destroyParser();
destroyRenderers();
}
@@ -469,7 +464,8 @@
if (formatSize != m_cachedSize) {
LOG(Media, "SourceBufferPrivateAVFObjC::processCodedFrame(%p) - size change detected: {width=%lf, height=%lf", formatSize.width(), formatSize.height());
m_cachedSize = formatSize;
- m_mediaSource->player()->sizeChanged();
+ if (m_mediaSource)
+ m_mediaSource->player()->sizeChanged();
}
}
if (m_client)
@@ -487,6 +483,9 @@
void SourceBufferPrivateAVFObjC::didProvideContentKeyRequestInitializationDataForTrackID(NSData* initData, int trackID)
{
+ if (!m_mediaSource)
+ return;
+
UNUSED_PARAM(trackID);
#if ENABLE(ENCRYPTED_MEDIA_V2)
LOG(Media, "SourceBufferPrivateAVFObjC::didProvideContentKeyRequestInitializationDataForTrackID(%p) - track:%d", this, trackID);
@@ -519,15 +518,19 @@
LOG(Media, "SourceBufferPrivateAVFObjC::append(%p) - data:%p, length:%d", this, data, length);
RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:data length:length]);
- RefPtr<SourceBufferPrivateAVFObjC> strongThis = this;
+ WeakPtr<SourceBufferPrivateAVFObjC> weakThis = createWeakPtr();
+ RetainPtr<AVStreamDataParser> parser = m_parser;
+ RetainPtr<WebAVStreamDataParserListener> delegate = m_delegate;
m_parsingSucceeded = true;
- dispatch_async(globalDataParserQueue(), [nsData, strongThis] {
- [strongThis->m_parser appendStreamData:nsData.get()];
+ dispatch_async(globalDataParserQueue(), [nsData, weakThis, parser, delegate] {
- callOnMainThread([strongThis] {
- strongThis->appendCompleted();
+ [parser appendStreamData:nsData.get()];
+
+ callOnMainThread([weakThis] {
+ if (weakThis)
+ weakThis->appendCompleted();
});
});
}
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
