Implement DataCue interface.

Source/core/html/track/DataCue.cpp

 /*
- * Copyright (c) 2014, Google Inc. All rights reserved.
+ * Copyright (c) 2014, CableLabs Inc. All rights reserved.

[acolwell GONE FROM CHROMIUM, 2014/04/04 23:25:27]

nit: This should use the Chromium header in the Blink style guide.

  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  * 3. Neither the name of Opera Software ASA nor the names of its
  *    contributors may be used to endorse or promote products derived
  *    from this software without specific prior written permission.
  *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
  * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
  * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
  * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
  * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
  * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
  * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
  * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
  * OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
-#include "CSSTestHelper.h"
-
-#include "core/css/CSSRuleList.h"
-#include "core/css/CSSStyleSheet.h"
-#include "core/css/RuleSet.h"
-#include "core/css/StyleSheetContents.h"
-#include "core/dom/Document.h"
-#include "wtf/text/WTFString.h"
-
-#include <gtest/gtest.h>
+#include "core/html/track/DataCue.h"
 
 namespace WebCore {
 
-CSSTestHelper::~CSSTestHelper()
+DataCue::DataCue(ExecutionContext* context, double startTime, double endTime, ArrayBuffer* data, ExceptionState& exceptionState)
+    : TextTrackCue(startTime, endTime)

[acolwell GONE FROM CHROMIUM, 2014/04/04 23:25:27]

I'm surprised that there doesn't appear to be any logic to validate that
startTime <= endTime. Are these really supposed to be allowed by the spec?

[Brendan Long, 2014/05/28 16:31:48]

I don't see anything about that:

http://www.w3.org/html/wg/drafts/html/CR/embedded-content-0.html#dom-texttrac...

[philipj_slow, 2014/06/02 07:50:52]

Right, there is no such restriction. Negative times are also allowed, although
Blink doesn't support it yet ("TODO(93143): Add spec-compliant behavior for
negative time values.")

+    , m_executionContext(context)

[acolwell GONE FROM CHROMIUM, 2014/04/04 23:25:27]

DataCue should probably from ContextLifecycleObserver if it wants to safely hold
onto tihs context pointer.

+{
+    ScriptWrappable::init(this);
+    setData(data, exceptionState);
+}
+
+DataCue::~DataCue()
 {
 }
 
-CSSTestHelper::CSSTestHelper()
+#ifndef NDEBUG
+String DataCue::toString() const
 {
-    m_document = Document::create();
-    TextPosition position;
-    m_styleSheet = CSSStyleSheet::createInline(m_document.get(), KURL(), position, "UTF-8");
+    return String::format("%p id=%s interval=%f-->%f)", this, id().utf8().data(), startTime(), endTime());
+}
+#endif
+
+PassRefPtr<ArrayBuffer> DataCue::data() const
+{
+    ASSERT(m_data);
+    return ArrayBuffer::create(m_data.get());
 }
 
-RuleSet& CSSTestHelper::ruleSet()
+void DataCue::setData(ArrayBuffer* data, ExceptionState& exceptionState)
 {
-    RuleSet& ruleSet = m_styleSheet->contents()->ensureRuleSet(MediaQueryEvaluator(), RuleHasNoSpecialState);
-    ruleSet.compactRulesIfNeeded();
-    return ruleSet;
+    if (!data)
+        exceptionState.throwDOMException(InvalidNodeTypeError, "DataCue.data must be a non-null ArrayBuffer");

[philipj_slow, 2014/04/04 14:54:33]

[StrictTypeChecking] in the IDL should take care of this.

[Brendan Long, 2014/04/04 15:07:22]

I tried that and it still accepted null and strings (but correctly refused to
use numbers).

[fs, 2014/04/04 17:10:35]

Sounds like an issue with the bindings generator - I uploaded
https://codereview.chromium.org/226073003 which might help (the case with string
still sounds odd though.)

[Brendan Long, 2014/04/04 17:43:06]

That looks like it would fix the null case. For strings, I think the issue is
that the parameter version of [StrictTypeChecking] is different:
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-Strict...

If I use this WebIDL:

[
    Constructor(double startTime, double endTime, [StrictTypeChecking]
ArrayBuffer data),
    ConstructorCallWith=ExecutionContext,
] interface DataCue : TextTrackCue {
    [StrictTypeChecking] attribute ArrayBuffer data;
    readonly attribute DOMString? text;
};

Then I get exceptions if I try to set .data to anything except null or an
ArrayBuffer, but pretty much anything works for for the constructor (my layout
test tries passing null, a string, and an array, and all three pass; I also
tried a number and that also works without an exception).

[acolwell GONE FROM CHROMIUM, 2014/04/04 23:25:27]

Should this be exceptionState.throwTypeError() instead?

+    else
+        m_data = ArrayBuffer::create(data);
 }
 
-void CSSTestHelper::addCSSRules(const char* cssText)
+String DataCue::text(bool& isNull) const
 {
-    TextPosition position;
-    unsigned sheetLength = m_styleSheet->length();
-    ASSERT_TRUE(m_styleSheet->contents()->parseStringAtPosition(cssText, position, true));
-    ASSERT_TRUE(m_styleSheet->length() > sheetLength);
+    isNull = true;
+    return String();

[philipj_slow, 2014/04/04 14:54:33]

"The text attribute, on getting, must return UTF-16 text converted from data of
the text track cue that the TextTrackCue object represents. If a conversion of
the content in data is not possible, e.g. because the UA is unable to identify
the encoding, it must return null."

I think it doesn't make sense to expose the property at all if it always returns
null. (I argued when Silvia was spec'ing this that the property itself doesn't
make sense and is just a convenience we could do without.)

[Brendan Long, 2014/04/04 15:07:22]

My problem is, how do we identify the encoding? As far as I can tell, the String
constructors expect us to already know the encoding before we use them. I guess
there's probably a function somewhere that does that for HTML documents, but I
suspect cues are short enough that we'd get a lot of false positives.

It seems like the .text attribute could be useful in cases where the UA is
creating DataCues, and knows that the content is text, but Chromium doesn't
(won't?) do that.

[acolwell GONE FROM CHROMIUM, 2014/04/04 23:25:27]

I agree with Philip here. It seems odd that we would have to always return null.
At a minimum, I'd expect some sort of encoding string to signal whether it was
valid to convert data to text or not. As is this attribute doesn't seem very
useful.

[Brendan Long, 2014/05/28 16:31:48]

There was some discussion about removing .text, but I can't find a W3C bug for
it :\

[philipj_slow, 2014/06/02 07:50:52]

It's been removed now:
https://github.com/w3c/html/commit/d5ec35198fea01e56b0ec38eedce58a57a013138

+}
+
+ExecutionContext* DataCue::executionContext() const
+{
+    return m_executionContext;
 }
 
 } // namespace WebCore