Fix MSE GC, make it less aggressive, more spec-compliant (Blink CL)

Source/modules/mediasource/SourceBuffer.cpp

Index: Source/modules/mediasource/SourceBuffer.cpp
diff --git a/Source/modules/mediasource/SourceBuffer.cpp b/Source/modules/mediasource/SourceBuffer.cpp
index f7138e6fec0b2c28be83f30fc040cbbea116a3ff..c29a8b4714c3a701ea5be2ba97eb2dce9f03d962 100644
--- a/Source/modules/mediasource/SourceBuffer.cpp
+++ b/Source/modules/mediasource/SourceBuffer.cpp
@@ -40,6 +40,8 @@
 #include "core/events/Event.h"
 #include "core/events/GenericEventQueue.h"
 #include "core/fileapi/FileReaderLoader.h"
+#include "core/html/HTMLMediaElement.h"
+#include "core/html/MediaError.h"

[philipj_slow, 2015/07/08 12:47:44]

Is this include needed, or is it enough to forward declare MediaError, given
that it's not actually used?

[servolk, 2015/07/08 19:03:48]

This is needed, we can't get away with just a forward declaration here, because
HTMLMediaElement::error returns a smart pointer to MediaError, which will try to
destroy MediaError object if it's not null, and for the object to be properly
destroyed, we need to know it's size and full definition.

[philipj_slow, 2015/07/09 09:49:23]

Acknowledged.

 #include "core/html/TimeRanges.h"
 #include "core/streams/Stream.h"
 #include "modules/mediasource/MediaSource.h"
@@ -513,24 +515,64 @@ void SourceBuffer::scheduleEvent(const AtomicString& eventName)
     m_asyncEventQueue->enqueueEvent(event.release());
 }
 
+bool SourceBuffer::prepareAppend(ExceptionState& exceptionState)
+{
+    TRACE_EVENT_ASYNC_BEGIN0("media", "SourceBuffer::prepareAppend", this);
+    // http://w3c.github.io/media-source/#sourcebuffer-prepare-append
+    // 3.5.4 Prepare Append Algorithm
+    // 1. If the SourceBuffer has been removed from the sourceBuffers attribute of the parent media source then throw an InvalidStateError exception and abort these steps.
+    // 2. If the updating attribute equals true, then throw an InvalidStateError exception and abort these steps.
+    if (throwExceptionIfRemovedOrUpdating(isRemoved(), m_updating, exceptionState)) {
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::prepareAppend", this);
+        return false;
+    }
+
+    // 3. If the HTMLMediaElement.error attribute is not null, then throw an InvalidStateError exception and abort these steps.
+    ASSERT(m_source);
+    ASSERT(m_source->mediaElement());
+    if (m_source->mediaElement()->error()) {
+        exceptionState.throwDOMException(InvalidStateError, "The HTMLMediaElement.error attribute is not null.");
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::prepareAppend", this);
+        return false;
+    }
+
+    // 4. If the readyState attribute of the parent media source is in the "ended" state then run the following steps:
+    //    1. Set the readyState attribute of the parent media source to "open"
+    //    2. Queue a task to fire a simple event named sourceopen at the parent media source.
+    m_source->openIfInEndedState();
+
+    // 5. Run the coded frame eviction algorithm.
+    if (!evictCodedFrames()) {
+        // 6. If the buffer full flag equals true, then throw a QUOTA_EXCEEDED_ERR exception and abort these steps.
+        exceptionState.throwDOMException(QuotaExceededError, "The SourceBuffer is full, and cannot free space to append additional buffers.");
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::prepareAppend", this);
+        return false;
+    }
+
+    TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::prepareAppend", this);
+    return true;
+}
+
+bool SourceBuffer::evictCodedFrames()
+{
+    ASSERT(m_source);
+    ASSERT(m_source->mediaElement());
+    double currentTime = m_source->mediaElement()->currentTime();
+    return m_webSourceBuffer->evictCodedFrames(currentTime);
+}
+
 void SourceBuffer::appendBufferInternal(const unsigned char* data, unsigned size, ExceptionState& exceptionState)
 {
+    TRACE_EVENT_ASYNC_BEGIN1("media", "SourceBuffer::appendBuffer", this, "size", size);
     // Section 3.2 appendBuffer()
     // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#widl-SourceBuffer-appendBuffer-void-ArrayBufferView-data
 
     // 1. Run the prepare append algorithm.
-    // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-prepare-append
-    //  1. If this object has been removed from the sourceBuffers attribute of the parent media source then throw an InvalidStateError exception and abort these steps.
-    //  2. If the updating attribute equals true, then throw an InvalidStateError exception and abort these steps.
-    if (throwExceptionIfRemovedOrUpdating(isRemoved(), m_updating, exceptionState))
+    if (!prepareAppend(exceptionState)) {
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::appendBuffer", this);
         return;
-
-    TRACE_EVENT_ASYNC_BEGIN1("media", "SourceBuffer::appendBuffer", this, "size", size);
-
-    //  3. If the readyState attribute of the parent media source is in the "ended" state then run the following steps: ...
-    m_source->openIfInEndedState();
-
-    //  Steps 4-5 - end "prepare append" algorithm.
+    }
+    TRACE_EVENT_ASYNC_STEP_INTO0("media", "SourceBuffer::appendBuffer", this, "prepareAppend");
 
     // 2. Add data to the end of the input buffer.
     ASSERT(data || size == 0);
@@ -630,28 +672,22 @@ void SourceBuffer::removeAsyncPart()
 
 void SourceBuffer::appendStreamInternal(Stream* stream, ExceptionState& exceptionState)
 {
+    TRACE_EVENT_ASYNC_BEGIN0("media", "SourceBuffer::appendStream", this);
+
     // Section 3.2 appendStream()
-    // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#widl-SourceBuffer-appendStream-void-Stream-stream-unsigned-long-long-maxSize
+    // http://w3c.github.io/media-source/#widl-SourceBuffer-appendStream-void-ReadableStream-stream-unsigned-long-long-maxSize
     // (0. If the stream has been neutered, then throw an InvalidAccessError exception and abort these steps.)
     if (stream->isNeutered()) {
         exceptionState.throwDOMException(InvalidAccessError, "The stream provided has been neutered.");
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::appendStream", this);
         return;
     }
 
     // 1. Run the prepare append algorithm.
-    //  Section 3.5.4 Prepare Append Algorithm.
-    //  https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-prepare-append
-    //  1. If this object has been removed from the sourceBuffers attribute of the parent media source then throw an InvalidStateError exception and abort these steps.
-    //  2. If the updating attribute equals true, then throw an InvalidStateError exception and abort these steps.
-    if (throwExceptionIfRemovedOrUpdating(isRemoved(), m_updating, exceptionState))
+    if (!prepareAppend(exceptionState)) {
+        TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::appendStream", this);
         return;
-
-    TRACE_EVENT_ASYNC_BEGIN0("media", "SourceBuffer::appendStream", this);
-
-    //  3. If the readyState attribute of the parent media source is in the "ended" state then run the following steps: ...
-    m_source->openIfInEndedState();
-
-    // Steps 4-5 of the prepare append algorithm are handled by m_webSourceBuffer.
+    }
 
     // 2. Set the updating attribute to true.
     m_updating = true;
@@ -660,7 +696,6 @@ void SourceBuffer::appendStreamInternal(Stream* stream, ExceptionState& exceptio
     scheduleEvent(EventTypeNames::updatestart);
 
     // 4. Asynchronously run the stream append loop algorithm with stream and maxSize.
-
     stream->neuter();
     m_loader = FileReaderLoader::create(FileReaderLoader::ReadByClient, this);
     m_stream = stream;
@@ -672,9 +707,10 @@ void SourceBuffer::appendStreamAsyncPart()
     ASSERT(m_updating);
     ASSERT(m_loader);
     ASSERT(m_stream);
+    TRACE_EVENT_ASYNC_STEP_INTO0("media", "SourceBuffer::appendStream", this, "appendStreamAsyncPart");
 
     // Section 3.5.6 Stream Append Loop
-    // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-stream-append-loop
+    // http://w3c.github.io/media-source/#sourcebuffer-stream-append-loop
 
     // 1. If maxSize is set, then let bytesLeft equal maxSize.
     // 2. Loop Top: If maxSize is set and bytesLeft equals 0, then jump to the loop done step below.
@@ -697,24 +733,14 @@ void SourceBuffer::appendStreamDone(bool success)
     clearAppendStreamState();
 
     if (!success) {
-        // Section 3.5.3 Append Error Algorithm
-        // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-append-error
-        //
-        // 1. Run the reset parser state algorithm. (Handled by caller)
-        // 2. Set the updating attribute to false.
-        m_updating = false;
-
-        // 3. Queue a task to fire a simple event named error at this SourceBuffer object.
-        scheduleEvent(EventTypeNames::error);
-
-        // 4. Queue a task to fire a simple event named updateend at this SourceBuffer object.
-        scheduleEvent(EventTypeNames::updateend);
+        appendError(false);
         TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::appendStream", this);
         return;
     }
 
     // Section 3.5.6 Stream Append Loop
     // Steps 1-11 are handled by appendStreamAsyncPart(), |m_loader|, and |m_webSourceBuffer|.
+
     // 12. Loop Done: Set the updating attribute to false.
     m_updating = false;
 
@@ -734,6 +760,29 @@ void SourceBuffer::clearAppendStreamState()
     m_stream = nullptr;
 }
 
+void SourceBuffer::appendError(bool decodeError)
+{
+    // Section 3.5.3 Append Error Algorithm
+    // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-append-error
+    //

[philipj_slow, 2015/07/08 12:47:44]

Blank line instead?

[servolk, 2015/07/08 19:03:47]

Done.

+    // 1. Run the reset parser state algorithm.
+    m_webSourceBuffer->abort();

[philipj_slow, 2015/07/08 12:47:44]

The web-facing SourceBuffer.abort() does more than just run the reset parser
state algorithm, per spec at least. Might this have other side effects, or is
the naming just a bit confusing?

[servolk, 2015/07/08 19:03:47]

Yes, this is a bit confusing. Note that I'm not calling SourceBuffer.abort(),
I'm calling WebSourceBuffer::abort(), which is one of the steps of
SourceBuffer::abort (see step #4 there and note the comment). I've looked at
WebSourceBufferImpl::abort and ChunkDemuxer::Abort + associated code path and I
believe this is the right thing to do here. Keep in mind that we are doing this
from appendError, i.e. we are already in error state here, calling
WebSourceBuffer::abort should ensure we fully flush/reset parser state (as a
side effect a pending seek operation might complete, but I think that should be
ok since we are about to report error, I don't think anybody expects us to
remain in pending seek state after an error is reported).

[philipj_slow, 2015/07/09 09:49:23]

Understood. Do you think resetParserState() would be a more appropriate name
than abort() on WebSourceBuffer?

[servolk, 2015/07/10 01:11:28]

Yes, I think it might a good idea to rename the current WebSourceBuffer::abort
into WebSourceBuffer::resetParserState, but I'd like also wolenetz@ to chime in
on this.
I'm not sure if completing a pending seek operation in ChunkDemuxer can be
decoupled from the parser reset algorithm.

[wolenetz, 2015/08/12 22:42:03]

Rename to resetParserState of WebSourceBuffer::abort really sgtm. It's
definitely not SourceBuffer::abort(), and the names being the same otherwise is
really confusing. Renaming ChunkDemuxer::Abort() also sgtm.

Resetting the parser state may indeed legally complete a pending seek, IIUC: it
includes emitting any parsed but not-yet-processed coded frames, which may
indeed allow seek to complete.

[servolk, 2015/08/21 02:51:25]

Ok, I think renaming WebSourceBuffer::abort into resetParserState makes sense,
but I'll do it as a separate CL after this one lands.

+
+    // 2. Set the updating attribute to false.
+    m_updating = false;
+
+    // 3. Queue a task to fire a simple event named error at this SourceBuffer object.
+    scheduleEvent(EventTypeNames::error);
+
+    // 4. Queue a task to fire a simple event named updateend at this SourceBuffer object.
+    scheduleEvent(EventTypeNames::updateend);
+
+    // 5. If decode error is true, then run the end of stream algorithm with the
+    // error parameter set to "decode".
+    if (decodeError)
+        m_source->endOfStream("decode", ASSERT_NO_EXCEPTION);
+}
+
 void SourceBuffer::didStartLoading()
 {
     WTF_LOG(Media, "SourceBuffer::didStartLoading() %p", this);
@@ -750,7 +799,15 @@ void SourceBuffer::didReceiveDataForClient(const char* data, unsigned dataLength
 void SourceBuffer::didFinishLoading()
 {
     WTF_LOG(Media, "SourceBuffer::didFinishLoading() %p", this);
-    appendStreamDone(true);
+    // Section 3.5.6 Stream Append Loop
+    // Steps 1-9 are handled by appendStreamAsyncPart(), |m_loader|, and |m_webSourceBuffer|.
+
+    // 10. Run the coded frame eviction algorithm.
+    bool evictCodedFramesResult = evictCodedFrames();
+
+    // (will be done inside appendStreamDone)
+    // 11. If the buffer full flag equals true, then run the append error algorithm with the decode error parameter set to false and abort this algorithm.
+    appendStreamDone(evictCodedFramesResult);
 }
 
 void SourceBuffer::didFail(FileError::ErrorCode errorCode)