- Revision
- 160281
- Author
- [email protected]
- Date
- 2013-12-07 22:39:39 -0800 (Sat, 07 Dec 2013)
Log Message
[MSE][Mac] Crash when removing MediaSource from HTMLMediaElement.
https://bugs.webkit.org/show_bug.cgi?id=125269
Reviewed by Sam Weinig.
Fixes the media/media-source/media-source-fastseek.html test when run with MallocScribble enabled.
It's possible for a SourceBufferPrivateAVFObjC to outlive its MediaSourcePrivateAVFObjC, so
make sure to clear the pointer from the former to the latter when the latter is destroyed.
That means we now have to check to see if the pointer to the latter is still valid at every
call site.
As a drive-by fix, rename m_parent to m_mediaSource to more accurately reflect what the pointer
points to.
* platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::~MediaSourcePrivateAVFObjC): Clear the SourceBuffer's backpointer.
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
(WebCore::SourceBufferPrivateAVFObjC::clearMediaSource):
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC): Rename m_parent -> m_mediaSource.
(WebCore::SourceBufferPrivateAVFObjC::append): Check m_mediaSource before calling.
(WebCore::SourceBufferPrivateAVFObjC::removedFromMediaSource): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::readyState): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::setReadyState): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::trackDidChangeEnabled): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::flushAndEnqueueNonDisplayingSamples): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::enqueueSample): Ditto.
(WebCore::SourceBufferPrivateAVFObjC::setActive): Ditto.
* platform/mock/mediasource/MockMediaSourcePrivate.cpp:
(WebCore::MockMediaSourcePrivate::~MockMediaSourcePrivate): Clear the SourceBuffer's backpointer.
* platform/mock/mediasource/MockSourceBufferPrivate.cpp:
(WebCore::MockSourceBufferPrivate::MockSourceBufferPrivate): Rename m_parent -> m_mediaSource.
(WebCore::MockSourceBufferPrivate::removedFromMediaSource): Check m_mediaSource before calling.
(WebCore::MockSourceBufferPrivate::readyState): Ditto.
(WebCore::MockSourceBufferPrivate::setReadyState): Ditto.
(WebCore::MockSourceBufferPrivate::setActive): Ditto.
* platform/mock/mediasource/MockSourceBufferPrivate.h:
(WebCore::MockSourceBufferPrivate::clearMediaSource):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (160280 => 160281)
--- trunk/Source/WebCore/ChangeLog 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/ChangeLog 2013-12-08 06:39:39 UTC (rev 160281)
@@ -1,3 +1,45 @@
+2013-12-04 Jer Noble <[email protected]>
+
+ [MSE][Mac] Crash when removing MediaSource from HTMLMediaElement.
+ https://bugs.webkit.org/show_bug.cgi?id=125269
+
+ Reviewed by Sam Weinig.
+
+ Fixes the media/media-source/media-source-fastseek.html test when run with MallocScribble enabled.
+
+ It's possible for a SourceBufferPrivateAVFObjC to outlive its MediaSourcePrivateAVFObjC, so
+ make sure to clear the pointer from the former to the latter when the latter is destroyed.
+ That means we now have to check to see if the pointer to the latter is still valid at every
+ call site.
+
+ As a drive-by fix, rename m_parent to m_mediaSource to more accurately reflect what the pointer
+ points to.
+
+ * platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
+ (WebCore::MediaSourcePrivateAVFObjC::~MediaSourcePrivateAVFObjC): Clear the SourceBuffer's backpointer.
+ * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
+ (WebCore::SourceBufferPrivateAVFObjC::clearMediaSource):
+ * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+ (WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC): Rename m_parent -> m_mediaSource.
+ (WebCore::SourceBufferPrivateAVFObjC::append): Check m_mediaSource before calling.
+ (WebCore::SourceBufferPrivateAVFObjC::removedFromMediaSource): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::readyState): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::setReadyState): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::trackDidChangeEnabled): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::flushAndEnqueueNonDisplayingSamples): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::enqueueSample): Ditto.
+ (WebCore::SourceBufferPrivateAVFObjC::setActive): Ditto.
+ * platform/mock/mediasource/MockMediaSourcePrivate.cpp:
+ (WebCore::MockMediaSourcePrivate::~MockMediaSourcePrivate): Clear the SourceBuffer's backpointer.
+ * platform/mock/mediasource/MockSourceBufferPrivate.cpp:
+ (WebCore::MockSourceBufferPrivate::MockSourceBufferPrivate): Rename m_parent -> m_mediaSource.
+ (WebCore::MockSourceBufferPrivate::removedFromMediaSource): Check m_mediaSource before calling.
+ (WebCore::MockSourceBufferPrivate::readyState): Ditto.
+ (WebCore::MockSourceBufferPrivate::setReadyState): Ditto.
+ (WebCore::MockSourceBufferPrivate::setActive): Ditto.
+ * platform/mock/mediasource/MockSourceBufferPrivate.h:
+ (WebCore::MockSourceBufferPrivate::clearMediaSource):
+
2013-12-07 Zoltan Horvath <[email protected]>
Remove statusWithDirection static function from RenderBlockLineLayout
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm (160280 => 160281)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm 2013-12-08 06:39:39 UTC (rev 160281)
@@ -57,6 +57,8 @@
MediaSourcePrivateAVFObjC::~MediaSourcePrivateAVFObjC()
{
+ for (auto it = m_sourceBuffers.begin(), end = m_sourceBuffers.end(); it != end; ++it)
+ (*it)->clearMediaSource();
}
MediaSourcePrivate::AddStatus MediaSourcePrivateAVFObjC::addSourceBuffer(const ContentType& contentType, RefPtr<SourceBufferPrivate>& outPrivate)
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h (160280 => 160281)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h 2013-12-08 06:39:39 UTC (rev 160281)
@@ -59,6 +59,8 @@
static RefPtr<SourceBufferPrivateAVFObjC> create(MediaSourcePrivateAVFObjC*);
virtual ~SourceBufferPrivateAVFObjC();
+ void clearMediaSource() { m_mediaSource = nullptr; }
+
// AVStreamDataParser delegate methods
void didParseStreamDataAsAsset(AVAsset*);
void didFailToParseStreamDataWithError(NSError*);
@@ -101,7 +103,7 @@
RetainPtr<AVSampleBufferDisplayLayer> m_displayLayer;
RetainPtr<NSObject> m_delegate;
- MediaSourcePrivateAVFObjC* m_parent;
+ MediaSourcePrivateAVFObjC* m_mediaSource;
SourceBufferPrivateClient* m_client;
bool m_parsingSucceeded;
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm (160280 => 160281)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2013-12-08 06:39:39 UTC (rev 160281)
@@ -284,7 +284,7 @@
SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC(MediaSourcePrivateAVFObjC* parent)
: m_parser(adoptNS([[getAVStreamDataParserClass() alloc] init]))
, m_delegate(adoptNS([[WebAVStreamDataParserListener alloc] initWithParser:m_parser.get() parent:this]))
- , m_parent(parent)
+ , m_mediaSource(parent)
, m_client(0)
, m_parsingSucceeded(true)
{
@@ -293,7 +293,8 @@
SourceBufferPrivateAVFObjC::~SourceBufferPrivateAVFObjC()
{
if (m_displayLayer) {
- m_parent->player()->removeDisplayLayer(m_displayLayer.get());
+ if (m_mediaSource)
+ m_mediaSource->player()->removeDisplayLayer(m_displayLayer.get());
[m_displayLayer flushAndRemoveImage];
[m_displayLayer stopRequestingMediaData];
m_displayLayer = nullptr;
@@ -386,8 +387,8 @@
LOG(Media, "SourceBufferPrivateAVFObjC::append(%p) - data:%p, length:%d", this, data, length);
[m_parser appendStreamData:[NSData dataWithBytes:data length:length]];
- if (m_parsingSucceeded)
- m_parent->player()->setLoadingProgresssed(true);
+ if (m_parsingSucceeded && m_mediaSource)
+ m_mediaSource->player()->setLoadingProgresssed(true);
return m_parsingSucceeded ? AppendSucceeded : ParsingFailed;
}
@@ -400,23 +401,26 @@
void SourceBufferPrivateAVFObjC::removedFromMediaSource()
{
if (m_displayLayer) {
- m_parent->player()->removeDisplayLayer(m_displayLayer.get());
+ if (m_mediaSource)
+ m_mediaSource->player()->removeDisplayLayer(m_displayLayer.get());
[m_displayLayer flush];
[m_displayLayer stopRequestingMediaData];
m_displayLayer = nullptr;
}
- m_parent->removeSourceBuffer(this);
+ if (m_mediaSource)
+ m_mediaSource->removeSourceBuffer(this);
}
MediaPlayer::ReadyState SourceBufferPrivateAVFObjC::readyState() const
{
- return m_parent->player()->readyState();
+ return m_mediaSource ? m_mediaSource->player()->readyState() : MediaPlayer::HaveNothing;
}
void SourceBufferPrivateAVFObjC::setReadyState(MediaPlayer::ReadyState readyState)
{
- m_parent->player()->setReadyState(readyState);
+ if (m_mediaSource)
+ m_mediaSource->player()->setReadyState(readyState);
}
void SourceBufferPrivateAVFObjC::evictCodedFrames()
@@ -453,6 +457,8 @@
if (!track->selected() && m_enabledVideoTrackID == trackID) {
m_enabledVideoTrackID = -1;
[m_parser setShouldProvideMediaData:NO forTrackID:trackID];
+ if (m_mediaSource)
+ m_mediaSource->player()->removeDisplayLayer(m_displayLayer.get());
} else if (track->selected()) {
m_enabledVideoTrackID = trackID;
[m_parser setShouldProvideMediaData:YES forTrackID:trackID];
@@ -462,7 +468,8 @@
if (m_client)
m_client->sourceBufferPrivateDidBecomeReadyForMoreSamples(this);
}];
- m_parent->player()->addDisplayLayer(m_displayLayer.get());
+ if (m_mediaSource)
+ m_mediaSource->player()->addDisplayLayer(m_displayLayer.get());
}
}
}
@@ -508,7 +515,8 @@
[m_displayLayer enqueueSampleBuffer:sampleBuffer.get()];
}
- m_parent->player()->setHasAvailableVideoFrame(false);
+ if (m_mediaSource)
+ m_mediaSource->player()->setHasAvailableVideoFrame(false);
}
void SourceBufferPrivateAVFObjC::enqueueSample(PassRefPtr<MediaSample> prpMediaSample, AtomicString trackID)
@@ -523,7 +531,8 @@
return;
[m_displayLayer enqueueSampleBuffer:platformSample.sample.cmSampleBuffer];
- m_parent->player()->setHasAvailableVideoFrame(true);
+ if (m_mediaSource)
+ m_mediaSource->player()->setHasAvailableVideoFrame(true);
}
bool SourceBufferPrivateAVFObjC::isReadyForMoreSamples()
@@ -533,7 +542,8 @@
void SourceBufferPrivateAVFObjC::setActive(bool isActive)
{
- m_parent->sourceBufferPrivateDidChangeActiveState(this, isActive);
+ if (m_mediaSource)
+ m_mediaSource->sourceBufferPrivateDidChangeActiveState(this, isActive);
}
MediaTime SourceBufferPrivateAVFObjC::fastSeekTimeForMediaTime(MediaTime time, MediaTime negativeThreshold, MediaTime positiveThreshold)
Modified: trunk/Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp (160280 => 160281)
--- trunk/Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp 2013-12-08 06:39:39 UTC (rev 160281)
@@ -49,6 +49,8 @@
MockMediaSourcePrivate::~MockMediaSourcePrivate()
{
+ for (auto it = m_sourceBuffers.begin(), end = m_sourceBuffers.end(); it != end; ++it)
+ (*it)->clearMediaSource();
}
MediaSourcePrivate::AddStatus MockMediaSourcePrivate::addSourceBuffer(const ContentType& contentType, RefPtr<SourceBufferPrivate>& outPrivate)
Modified: trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp (160280 => 160281)
--- trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp 2013-12-08 06:39:39 UTC (rev 160281)
@@ -100,7 +100,7 @@
}
MockSourceBufferPrivate::MockSourceBufferPrivate(MockMediaSourcePrivate* parent)
- : m_parent(parent)
+ : m_mediaSource(parent)
, m_client(0)
{
}
@@ -187,17 +187,19 @@
void MockSourceBufferPrivate::removedFromMediaSource()
{
- m_parent->removeSourceBuffer(this);
+ if (m_mediaSource)
+ m_mediaSource->removeSourceBuffer(this);
}
MediaPlayer::ReadyState MockSourceBufferPrivate::readyState() const
{
- return m_parent->player()->readyState();
+ return m_mediaSource ? m_mediaSource->player()->readyState() : MediaPlayer::HaveNothing;
}
void MockSourceBufferPrivate::setReadyState(MediaPlayer::ReadyState readyState)
{
- m_parent->player()->setReadyState(readyState);
+ if (m_mediaSource)
+ m_mediaSource->player()->setReadyState(readyState);
}
void MockSourceBufferPrivate::evictCodedFrames()
@@ -212,7 +214,8 @@
void MockSourceBufferPrivate::setActive(bool isActive)
{
- m_parent->sourceBufferPrivateDidChangeActiveState(this, isActive);
+ if (m_mediaSource)
+ m_mediaSource->sourceBufferPrivateDidChangeActiveState(this, isActive);
}
bool MockSourceBufferPrivate::hasVideo() const
Modified: trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.h (160280 => 160281)
--- trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.h 2013-12-08 06:17:18 UTC (rev 160280)
+++ trunk/Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.h 2013-12-08 06:39:39 UTC (rev 160281)
@@ -50,6 +50,8 @@
static RefPtr<MockSourceBufferPrivate> create(MockMediaSourcePrivate*);
virtual ~MockSourceBufferPrivate();
+ void clearMediaSource() { m_mediaSource = nullptr; }
+
bool hasVideo() const;
bool hasAudio() const;
@@ -77,7 +79,7 @@
void didReceiveInitializationSegment(const MockInitializationBox&);
void didReceiveSample(const MockSampleBox&);
- MockMediaSourcePrivate* m_parent;
+ MockMediaSourcePrivate* m_mediaSource;
SourceBufferPrivateClient* m_client;
Vector<char> m_inputBuffer;