MSE: Throw TypeError instead of InvalidAccessError per spec update

third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 146 matching lines @@
     DEFINE_STATIC_LOCAL(const AtomicString, segments, ("segments"));
     return segments;
 }
 
 const AtomicString& SourceBuffer::sequenceKeyword()
 {
     DEFINE_STATIC_LOCAL(const AtomicString, sequence, ("sequence"));
     return sequence;
 }
 
 void SourceBuffer::setMode(const AtomicString& newMode, ExceptionState& exceptionState)

[chcunningham, 2016/09/07 18:36:06]

Not covered: If generate timestamps flag equals true and new mode equals
"segments", then throw a TypeError exception and abort these steps.

Also, not tested?

[wolenetz, 2016/09/07 21:09:26]

This is expected. Fixing this in Chrome is tracked separately by
https://bugs.chromium.org/p/chromium/issues/detail?id=607372

Test of this is already in upstream w3c mse tests: 

Base cases:
*
https://github.com/w3c/web-platform-tests/blob/master/media-source/mediasourc...
(Chrome, even with this CL, fails 1/2 of the subtests: "addSourceBuffer() sets
SourceBuffer.mode to 'sequence' when the generate timestamps flag is true")
*
https://github.com/w3c/web-platform-tests/blob/master/media-source/mediasourc...
(Chrome, with this CL (and I think without it) passes all these subtests)

Specific coverage for the spec item you referenced:
*
https://github.com/w3c/web-platform-tests/blob/master/media-source/mediasourc...
(Chrome, even with this CL, fails 2/2 of the subtests).

 {
     BLINK_SBLOG << __func__ << " this=" << this << " newMode=" << newMode;
     // Section 3.1 On setting mode attribute steps.
     // 1. Let new mode equal the new value being assigned to this attribute.
     // 2. If this object has been removed from the sourceBuffers attribute of the parent media source, then throw
     //    an INVALID_STATE_ERR exception and abort these steps.
     // 3. If the updating attribute equals true, then throw an INVALID_STATE_ERR exception and abort these steps.
     if (throwExceptionIfRemovedOrUpdating(isRemoved(), m_updating, exceptionState))
         return;
 
@@ skipping 77 matching lines @@
 
 double SourceBuffer::appendWindowStart() const
 {
     return m_appendWindowStart;
 }
 
 void SourceBuffer::setAppendWindowStart(double start, ExceptionState& exceptionState)
 {
     BLINK_SBLOG << __func__ << " this=" << this << " start=" << start;
     // Section 3.1 appendWindowStart attribute setter steps.
-    // https://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#widl-SourceBuffer-appendWindowStart
+    // https://www.w3.org/TR/media-source/#widl-SourceBuffer-appendWindowStart
     // 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))
         return;
 
-    // 3. If the new value is less than 0 or greater than or equal to appendWindowEnd then throw an InvalidAccessError
+    // 3. If the new value is less than 0 or greater than or equal to appendWindowEnd then throw a TypeError
     //    exception and abort these steps.
     if (start < 0 || start >= m_appendWindowEnd) {

[chcunningham, 2016/09/07 18:36:05]

Weird that we don't check NaN for start like we do end? Spec doesn't seem to
mention it.

[wolenetz, 2016/09/07 21:09:27]

We do and spec does. That's enforced by IDL bindings (it's a double, not an
unrestricted double like end.) IDL is:
 [RaisesException=Setter] attribute double appendWindowStart;

vs end:
 [RaisesException=Setter] attribute unrestricted double appendWindowEnd;

And we have test coverage for this in
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test...

-        MediaSource::logAndThrowDOMException(exceptionState, InvalidAccessError, ExceptionMessages::indexOutsideRange("value", start, 0.0, ExceptionMessages::ExclusiveBound, m_appendWindowEnd, ExceptionMessages::InclusiveBound));
+        MediaSource::logAndThrowTypeError(exceptionState, ExceptionMessages::indexOutsideRange("value", start, 0.0, ExceptionMessages::ExclusiveBound, m_appendWindowEnd, ExceptionMessages::InclusiveBound));
         return;
     }
 
     m_webSourceBuffer->setAppendWindowStart(start);
 
     // 4. Update the attribute to the new value.
     m_appendWindowStart = start;
 }
 
 double SourceBuffer::appendWindowEnd() const
 {
     return m_appendWindowEnd;
 }
 
 void SourceBuffer::setAppendWindowEnd(double end, ExceptionState& exceptionState)
 {
     BLINK_SBLOG << __func__ << " this=" << this << " end=" << end;
     // Section 3.1 appendWindowEnd attribute setter steps.
-    // https://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#widl-SourceBuffer-appendWindowEnd
+    // https://www.w3.org/TR/media-source/#widl-SourceBuffer-appendWindowEnd
     // 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))
         return;
 
-    // 3. If the new value equals NaN, then throw an InvalidAccessError and abort these steps.
+    // 3. If the new value equals NaN, then throw a TypeError and abort these steps.
     if (std::isnan(end)) {
-        MediaSource::logAndThrowDOMException(exceptionState, InvalidAccessError, ExceptionMessages::notAFiniteNumber(end));
+        MediaSource::logAndThrowTypeError(exceptionState, ExceptionMessages::notAFiniteNumber(end));
         return;
     }
-    // 4. If the new value is less than or equal to appendWindowStart then throw an InvalidAccessError
+    // 4. If the new value is less than or equal to appendWindowStart then throw a TypeError
     //    exception and abort these steps.
     if (end <= m_appendWindowStart) {
-        MediaSource::logAndThrowDOMException(exceptionState, InvalidAccessError, ExceptionMessages::indexExceedsMinimumBound("value", end, m_appendWindowStart));
+        MediaSource::logAndThrowTypeError(exceptionState, ExceptionMessages::indexExceedsMinimumBound("value", end, m_appendWindowStart));
         return;
     }
 
     m_webSourceBuffer->setAppendWindowEnd(end);
 
     // 5. Update the attribute to the new value.
     m_appendWindowEnd = end;
 }
 
 void SourceBuffer::appendBuffer(DOMArrayBuffer* data, ExceptionState& exceptionState)
@@ skipping 71 matching lines @@
 
     // 7. Set appendWindowEnd to positive Infinity.
     setAppendWindowEnd(std::numeric_limits<double>::infinity(), exceptionState);
 }
 
 void SourceBuffer::remove(double start, double end, ExceptionState& exceptionState)
 {
     BLINK_SBLOG << __func__ << " this=" << this << " start=" << start << " end=" << end;
 
     // Section 3.2 remove() method steps.
-    // 1. If duration equals NaN, then throw an InvalidAccessError exception and abort these steps.
-    // 2. If start is negative or greater than duration, then throw an InvalidAccessError exception and abort these steps.
+    // https://www.w3.org/TR/media-source/#widl-SourceBuffer-remove-void-double-start-unrestricted-double-end
+    // 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))
+        return;
 
-    if (start < 0 || (m_source && (std::isnan(m_source->duration()) || start > m_source->duration()))) {
-        MediaSource::logAndThrowDOMException(exceptionState, InvalidAccessError, ExceptionMessages::indexOutsideRange("start", start, 0.0, ExceptionMessages::ExclusiveBound, !m_source || std::isnan(m_source->duration()) ? 0 : m_source->duration(), ExceptionMessages::ExclusiveBound));
+    // 3. If duration equals NaN, then throw a TypeError exception and abort these steps.
+    // 4. If start is negative or greater than duration, then throw a TypeError exception and abort these steps.
+    if (start < 0 || std::isnan(m_source->duration()) || start > m_source->duration()) {
+        MediaSource::logAndThrowTypeError(exceptionState, ExceptionMessages::indexOutsideRange("start", start, 0.0, ExceptionMessages::ExclusiveBound, std::isnan(m_source->duration()) ? 0 : m_source->duration(), ExceptionMessages::ExclusiveBound));
         return;
     }
 
-    // 3. If end is less than or equal to start or end equals NaN, then throw an InvalidAccessError exception and abort these steps.
+    // 5. If end is less than or equal to start or end equals NaN, then throw a TypeError exception and abort these steps.
     if (end <= start || std::isnan(end)) {
-        MediaSource::logAndThrowDOMException(exceptionState, InvalidAccessError, "The end value provided (" + String::number(end) + ") must be greater than the start value provided (" + String::number(start) + ").");
+        MediaSource::logAndThrowTypeError(exceptionState, "The end value provided (" + String::number(end) + ") must be greater than the start value provided (" + String::number(start) + ").");
         return;
     }
 
-    // 4. If this object has been removed from the sourceBuffers attribute of the parent media source then throw an
-    //    InvalidStateError exception and abort these steps.
-    // 5. If the updating attribute equals true, then throw an InvalidStateError exception and abort these steps.
-    if (throwExceptionIfRemovedOrUpdating(isRemoved(), m_updating, exceptionState))
-        return;
-
     TRACE_EVENT_ASYNC_BEGIN0("media", "SourceBuffer::remove", this);
 
     // 6. If the readyState attribute of the parent media source is in the "ended" state then run the following steps:
     // 6.1. Set the readyState attribute of the parent media source to "open"
     // 6.2. Queue a task to fire a simple event named sourceopen at the parent media source .
     m_source->openIfInEndedState();
 
     // 7. Run the range removal algorithm with start and end as the start and end of the removal range.
     // 7.3. Set the updating attribute to true.
     m_updating = true;
@@ skipping 827 matching lines @@
     visitor->trace(m_removeAsyncPartRunner);
     visitor->trace(m_appendStreamAsyncPartRunner);
     visitor->trace(m_stream);
     visitor->trace(m_audioTracks);
     visitor->trace(m_videoTracks);
     EventTargetWithInlineData::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink