Implement DataCue interface.

Source/core/html/track/DataCue.cpp

Index: Source/core/html/track/DataCue.cpp
diff --git a/Source/core/css/CSSTestHelper.cpp b/Source/core/html/track/DataCue.cpp
similarity index 59%
copy from Source/core/css/CSSTestHelper.cpp
copy to Source/core/html/track/DataCue.cpp
index 3b24d9c75e44f8dcf59fabefacdb7c7815c7587b..d74506c0041089782a8fdca698e42f466ffa640a 100644
--- a/Source/core/css/CSSTestHelper.cpp
+++ b/Source/core/html/track/DataCue.cpp
@@ -1,5 +1,5 @@
 /*
- * 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
@@ -28,43 +28,52 @@
  */
 
 #include "config.h"
-#include "CSSTestHelper.h"
+#include "core/html/track/DataCue.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"
+namespace WebCore {
 
-#include <gtest/gtest.h>
+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);
+}
 
-namespace WebCore {
+DataCue::~DataCue()
+{
+}
+
+#ifndef NDEBUG
+String DataCue::toString() const
+{
+    return String::format("%p id=%s interval=%f-->%f)", this, id().utf8().data(), startTime(), endTime());
+}
+#endif
 
-CSSTestHelper::~CSSTestHelper()
+PassRefPtr<ArrayBuffer> DataCue::data() const
 {
+    ASSERT(m_data);
+    return ArrayBuffer::create(m_data.get());
 }
 
-CSSTestHelper::CSSTestHelper()
+void DataCue::setData(ArrayBuffer* data, ExceptionState& exceptionState)
 {
-    m_document = Document::create();
-    TextPosition position;
-    m_styleSheet = CSSStyleSheet::createInline(m_document.get(), KURL(), position, "UTF-8");
+    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);
 }
 
-RuleSet& CSSTestHelper::ruleSet()
+String DataCue::text(bool& isNull) const
 {
-    RuleSet& ruleSet = m_styleSheet->contents()->ensureRuleSet(MediaQueryEvaluator(), RuleHasNoSpecialState);
-    ruleSet.compactRulesIfNeeded();
-    return ruleSet;
+    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

 }
 
-void CSSTestHelper::addCSSRules(const char* cssText)
+ExecutionContext* DataCue::executionContext() const
 {
-    TextPosition position;
-    unsigned sheetLength = m_styleSheet->length();
-    ASSERT_TRUE(m_styleSheet->contents()->parseStringAtPosition(cssText, position, true));
-    ASSERT_TRUE(m_styleSheet->length() > sheetLength);
+    return m_executionContext;
 }
 
 } // namespace WebCore