Oilpan: have the media element safely close its MediaSource.

Source/modules/mediasource/MediaSource.cpp

Index: Source/modules/mediasource/MediaSource.cpp
diff --git a/Source/modules/mediasource/MediaSource.cpp b/Source/modules/mediasource/MediaSource.cpp
index aa0a295c25867c84b0cc201706fd769ca934c6a0..04d76fb5b9d89b888ab0674bba68050c4ac8eaff 100644
--- a/Source/modules/mediasource/MediaSource.cpp
+++ b/Source/modules/mediasource/MediaSource.cpp
@@ -98,7 +98,7 @@ MediaSource::MediaSource(ExecutionContext* context)
     : ActiveDOMObject(context)
     , m_readyState(closedKeyword())
     , m_asyncEventQueue(GenericEventQueue::create(this))
-    , m_attachedElement(0)
+    , m_attachedElement(nullptr)
     , m_sourceBuffers(SourceBufferList::create(executionContext(), m_asyncEventQueue.get()))
     , m_activeSourceBuffers(SourceBufferList::create(executionContext(), m_asyncEventQueue.get()))
 {
@@ -109,7 +109,9 @@ MediaSource::MediaSource(ExecutionContext* context)
 MediaSource::~MediaSource()
 {
     WTF_LOG(Media, "MediaSource::~MediaSource %p", this);
+#if !ENABLE(OILPAN)
     ASSERT(isClosed());
+#endif
 }
 
 SourceBuffer* MediaSource::addSourceBuffer(const String& type, ExceptionState& exceptionState)
@@ -257,6 +259,17 @@ ExecutionContext* MediaSource::executionContext() const
     return ActiveDOMObject::executionContext();
 }
 
+void MediaSource::clearWeakMembers(Visitor* visitor)
+{
+#if ENABLE(OILPAN)
+    // Oilpan: If the MediaSource survived, but its attached media
+    // element did not, signal the element that it can safely
+    // notify its MediaSource during finalization by calling close().
+    if (m_attachedElement && !visitor->isAlive(m_attachedElement))
+        m_attachedElement->setCloseMediaSourceWhenFinalizing();

[haraken, 2014/07/03 03:25:37]

Don't you need to clear m_attachedElement here?

[haraken, 2014/07/03 03:25:37]

By the way, wouldn't it be possible to call closeMediaSource() here instead of
setting the flag and let the HTMLMediaElement's destructor call
closeMediaSource()?

[sof, 2014/07/03 06:22:24]

The close() will do that, so I didn't duplicate it here.

[sof, 2014/07/03 06:22:24]

That will bring about the allocation of an Event, which we're not allowed inside
weak callbacks, right?

[haraken, 2014/07/03 06:40:31]

ah, got it; I'd prefer explicitly clearing out m_attachedElement here as well
though.

[haraken, 2014/07/03 06:40:31]

Makes sense.

+#endif
+}
+
 void MediaSource::trace(Visitor* visitor)
 {
 #if ENABLE(OILPAN)
@@ -264,6 +277,7 @@ void MediaSource::trace(Visitor* visitor)
 #endif
     visitor->trace(m_sourceBuffers);
     visitor->trace(m_activeSourceBuffers);
+    visitor->registerWeakMembers<MediaSource, &MediaSource::clearWeakMembers>(this);
     EventTargetWithInlineData::trace(visitor);
 }
 
@@ -403,7 +417,7 @@ void MediaSource::setReadyState(const AtomicString& state)
 
     if (state == closedKeyword()) {
         m_webMediaSource.clear();
-        m_attachedElement = 0;
+        m_attachedElement = nullptr;
     }
 
     if (oldState == state)