MSE: Implement TrackDefaultList object

Source/modules/mediasource/TrackDefaultList.cpp

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+
+#include "modules/mediasource/TrackDefaultList.h"
+
+#include "bindings/core/v8/ExceptionState.h"
+#include "core/dom/ExceptionCode.h"
+
+namespace blink {
+
+TrackDefault* TrackDefaultList::item(unsigned index) const
+{
+    // Per 18 Nov 2014 Editor's Draft
+    // https://w3c.github.io/media-source/#trackdefaultlist
+    // When this method is invoked, the user agent must run the following steps:
+    // 1. If |index| is greater than or equal to the length attribute then
+    //    return undefined and abort these steps.
+    if (index >= m_trackDefaults.size())
+        return 0;
+
+    // 2. Return the |index|'th TrackDefault object in the list.
+    return m_trackDefaults[index].get();
+}
+
+TrackDefaultList::TrackDefaultList(const HeapVector<Member<TrackDefault>>& trackDefaults, ExceptionState& exceptionState)
+{
+    // Per 18 Nov 2014 Editor's Draft

[sof, 2014/12/03 19:15:40]

I think it would be tidier if the static factory method (create()) performed the
argument validation & raised an exception, if needed.

It looks odd (but it won't break anything much) to be doing all this validation
in the ctor - the local style for modules/ is (afaict) to prefer that pattern,
but I notice that TrackDefault also raises exceptions from within its ctor.

[philipj_slow, 2014/12/04 15:29:14]

I agree, m_typeAndIDToTrackDefaultMap is only used in the constructor to check
if an exception should be thrown, so if "If trackDefaults contains two or more
TrackDefault objects with the same type and the same byteStreamTrackID, then
throw an InvalidAccessError and abort these steps." is implemented in
TrackDefaultList::create() some simplification should follow.

m_trackDefaults is never modified after the constructor, so if you like you
could make it const to make that clear.

[wolenetz, 2014/12/11 23:52:39]

Thanks for pointing out these issues.
I'll address TrackDefault::create vs TrackDefault::ctor logic in a separate CL.

I've moved the logic to TrackDefaultList::create from its ctor.

Note, I definitely expect to be using (soon) TrackDefaultList's
|m_typeAndIDToTrackDefaultMap| to support the MSE initialization segment
received algorithm's default{kinds,language,label} algorithm, so I've kept that
constructed map as a member of TrackDefaultList (though it's now passed as a
const-ref arg to the ctor).

I've also made m_trackDefaults and m_typeAndIDToTrackDefaultMap const.

+    // https://w3c.github.io/media-source/#trackdefaultlist
+    // When this method is invoked, the user agent must run the following steps:
+    // 1. If |trackDefaults| contains two or more TrackDefault objects with the
+    //    same type and the same byteStreamTrackID, then throw an
+    //    InvalidAccessError and abort these steps.
+    //    Note: This also applies when byteStreamTrackID contains an empty
+    //    string and ensures that there is only one "byteStreamTrackID
+    //    independent" default for each TrackDefaultType value.
+    for (const auto& trackDefault : trackDefaults) {
+        TypeAndID key = TypeAndID(trackDefault->type(), trackDefault->byteStreamTrackID());
+        if (!m_typeAndIDToTrackDefaultMap.add(key, trackDefault).isNewEntry) {
+            exceptionState.throwDOMException(InvalidAccessError, "Duplicate TrackDefault type (" + key.first + ") and byteStreamTrackID (" + key.second + ")");
+            return;
+        }
+    }
+
+    // 2. Store a copy of |trackDefaults| in this new object so the values can

[haraken, 2014/12/04 11:29:51]

This is a shadow copy, is it OK? I guess this needs to be a deep copy.

[philipj_slow, 2014/12/04 15:29:14]

The spec says "Store a copy of trackDefaults in this new object so the values
can be returned by the accessor methods." which isn't explicit about it. Since
TrackDefault has only readonly members it's not possible to violate the
uniqueness invariant, but it's still very much observable whether a copy is made
or not. I'd lean towards not making a deep copy and having the spec changed to
require that the same objects are returned as were passed in.

[wolenetz, 2014/12/11 23:52:39]

Thanks to discussion on this CL, a w3c bug
(https://www.w3.org/Bugs/Public/show_bug.cgi?id=27559) was filed and fixed, and
the spec now explicitly requires shallow copy here.

+    //    be returned by the accessor methods.
+    m_trackDefaults = trackDefaults;
+}
+
+void TrackDefaultList::trace(Visitor* visitor)
+{
+    visitor->trace(m_trackDefaults);
+    visitor->trace(m_typeAndIDToTrackDefaultMap);
+}
+
+} // namespace blink