Bug 121562 – [MSE] Implement support for SourceBuffer.remove()

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[MSE] Implement support for SourceBuffer.remove()

[Jer Noble]

Created attachment 212007 [details]
Patch

[Eric Carlson]

Comment on attachment 212007 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212007&action=review

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:112
> +    // FIXME: Add step 6 text when mode attribute is implemented.

Nit: bug number?

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:170
> +void SourceBuffer::remove(double start, double end, ExceptionState& es)

I think this merge was a bit too automatic ;-). Don't you mean:

void SourceBuffer::remove(double start, double end, ExceptionCode& ec)

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:176
> +        es.throwDOMException(InvalidAccessError);

ec = INVALID_ACCESS_ERR;

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:184
> +        es.throwDOMException(InvalidStateError);

ec = INVALID_STATE_ERR;

[Radar WebKit Bug Importer]

<rdar://problem/16410855>

[Jon Lee]

ping?

[Jer Noble]

Created attachment 228072 [details]
Patch

Clearing r+ flag; new patch is significantly different enough from previous patch to warrant re-review.

[Eric Carlson]

Comment on attachment 228072 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228072&action=review

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:246
> +    m_source->openIfInEndedState();

You NULL check m_source above, do you need to so so here as well?

[Jer Noble]

Comment on attachment 228072 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228072&action=review

>> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:246
>> +    if (isRemoved() || m_updating) {
>> +        ec = INVALID_ACCESS_ERR;
>> +        return;
>> +    }
>> +
>> +    // 5. If the readyState attribute of the parent media source is in the "ended" state then run the following steps:
>> +    // 5.1. Set the readyState attribute of the parent media source to "open"
>> +    // 5.2. Queue a task to fire a simple event named sourceopen at the parent media source .
>> +    m_source->openIfInEndedState();
> 
> You NULL check m_source above, do you need to so so here as well?

No, the isRemoved() statement above is effectively a NULL check.

[WebKit Commit Bot]

Comment on attachment 228072 [details]
Patch

Clearing flags on attachment: 228072

Committed r166423: <http://trac.webkit.org/changeset/166423>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.