- Revision
- 113343
- Author
- [email protected]
- Date
- 2012-04-05 11:32:22 -0700 (Thu, 05 Apr 2012)
Log Message
media/track/track-webvtt-tc004-magic-header.html flakily times out
https://bugs.webkit.org/show_bug.cgi?id=72279
Source/WebCore:
Patch by Zan Dobersek <[email protected]> on 2012-04-05
Reviewed by Eric Carlson.
Move all the checking for the WebVTT file identifier from TextTrackLoader
to WebVTTParser, notifying the parser clients of parsing failures through
a new method. Remove the WebVTTParser::fileIdentifierMaximumLength method
and make WebVTTParser::hasRequiredFileIdentifier private. The latter now
operates on a vector holding the identifier data to which data is added
until there's enough of it to possibly hold the optional BOM character
and the required WEBVTT character sequence.
No new tests. Fixes an existing test.
* html/track/WebVTTParser.cpp: Fix a typo throughout the file:
fileIdentiferLength -> fileIdentifierLength
(WebCore):
(WebCore::WebVTTParser::parseBytes): Only continue with searcing for
the WEBVTT identifier if there's enough data to possibly contain the
identifier. Report that the file failed to parse if the identifier
was then not found.
(WebCore::WebVTTParser::hasRequiredFileIdentifier): Refactor to operate
on the vector containing identifier data and merge with the
hasLongWebVTTIdentifier function.
* html/track/WebVTTParser.h:
(WebVTTParserClient): Add a new method.
(WebVTTParser):
* loader/TextTrackLoader.cpp:
(WebCore::TextTrackLoader::processNewCueData): Parser is now created
immediately regardless of the mime type the response holds.
(WebCore::TextTrackLoader::fileFailedToParse): Log the error, set the
state to failed, start the cue load timer and cancel the load if the file
was not parsed successfully.
(WebCore):
* loader/TextTrackLoader.h:
(TextTrackLoader):
LayoutTests:
Remove the test from test expectations file as it should now pass.
Patch by Zan Dobersek <[email protected]> on 2012-04-05
Reviewed by Eric Carlson.
* platform/chromium/test_expectations.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (113342 => 113343)
--- trunk/LayoutTests/ChangeLog 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/LayoutTests/ChangeLog 2012-04-05 18:32:22 UTC (rev 113343)
@@ -1,3 +1,14 @@
+2012-04-05 Zan Dobersek <[email protected]>
+
+ media/track/track-webvtt-tc004-magic-header.html flakily times out
+ https://bugs.webkit.org/show_bug.cgi?id=72279
+
+ Remove the test from test expectations file as it should now pass.
+
+ Reviewed by Eric Carlson.
+
+ * platform/chromium/test_expectations.txt:
+
2012-04-05 Dimitri Glazkov <[email protected]>
Marked storage/indexeddb/basics-workers.html as flaky across all platforms in DEBUG
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (113342 => 113343)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-04-05 18:32:22 UTC (rev 113343)
@@ -3270,8 +3270,6 @@
BUGWK72271 MAC DEBUG : fast/loader/_javascript_-url-in-embed.html = PASS CRASH
BUGWK72271 SNOWLEOPARD DEBUG : fast/dom/node-iterator-reference-node-moved-crash.html = PASS CRASH
-BUGWK72279 : media/track/track-webvtt-tc004-magic-header.html = TIMEOUT TEXT PASS
-
BUGWK72376 WIN : fast/forms/file/input-file-label.html = TEXT PASS
// Animations tests use an inherently-flaky timer-based design.
Modified: trunk/Source/WebCore/ChangeLog (113342 => 113343)
--- trunk/Source/WebCore/ChangeLog 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/Source/WebCore/ChangeLog 2012-04-05 18:32:22 UTC (rev 113343)
@@ -1,3 +1,43 @@
+2012-04-05 Zan Dobersek <[email protected]>
+
+ media/track/track-webvtt-tc004-magic-header.html flakily times out
+ https://bugs.webkit.org/show_bug.cgi?id=72279
+
+ Reviewed by Eric Carlson.
+
+ Move all the checking for the WebVTT file identifier from TextTrackLoader
+ to WebVTTParser, notifying the parser clients of parsing failures through
+ a new method. Remove the WebVTTParser::fileIdentifierMaximumLength method
+ and make WebVTTParser::hasRequiredFileIdentifier private. The latter now
+ operates on a vector holding the identifier data to which data is added
+ until there's enough of it to possibly hold the optional BOM character
+ and the required WEBVTT character sequence.
+
+ No new tests. Fixes an existing test.
+
+ * html/track/WebVTTParser.cpp: Fix a typo throughout the file:
+ fileIdentiferLength -> fileIdentifierLength
+ (WebCore):
+ (WebCore::WebVTTParser::parseBytes): Only continue with searcing for
+ the WEBVTT identifier if there's enough data to possibly contain the
+ identifier. Report that the file failed to parse if the identifier
+ was then not found.
+ (WebCore::WebVTTParser::hasRequiredFileIdentifier): Refactor to operate
+ on the vector containing identifier data and merge with the
+ hasLongWebVTTIdentifier function.
+ * html/track/WebVTTParser.h:
+ (WebVTTParserClient): Add a new method.
+ (WebVTTParser):
+ * loader/TextTrackLoader.cpp:
+ (WebCore::TextTrackLoader::processNewCueData): Parser is now created
+ immediately regardless of the mime type the response holds.
+ (WebCore::TextTrackLoader::fileFailedToParse): Log the error, set the
+ state to failed, start the cue load timer and cancel the load if the file
+ was not parsed successfully.
+ (WebCore):
+ * loader/TextTrackLoader.h:
+ (TextTrackLoader):
+
2012-04-05 Shawn Singh <[email protected]>
[chromium] Need to clip to homogeneous w=0 plane when applying transforms.
Modified: trunk/Source/WebCore/html/track/WebVTTParser.cpp (113342 => 113343)
--- trunk/Source/WebCore/html/track/WebVTTParser.cpp 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/Source/WebCore/html/track/WebVTTParser.cpp 2012-04-05 18:32:22 UTC (rev 113343)
@@ -46,50 +46,8 @@
const int secondsPerMinute = 60;
const double malformedTime = -1;
const unsigned bomLength = 3;
-const unsigned fileIdentiferLength = 6;
-
-unsigned WebVTTParser::fileIdentifierMaximumLength()
-{
- return bomLength + fileIdentiferLength;
-}
+const unsigned fileIdentifierLength = 6;
-inline bool hasLongWebVTTIdentifier(String line)
-{
- // If line is more than six characters ...
- if (line.length() < fileIdentiferLength)
- return false;
-
- // but the first six characters do not exactly equal "WEBVTT" ...
- if (line.substring(0, fileIdentiferLength) != "WEBVTT")
- return false;
-
- // or the seventh character is neither a space nor a tab character, then abort.
- if (line.length() > fileIdentiferLength && line[fileIdentiferLength] != ' ' && line[fileIdentiferLength] != '\t')
- return false;
-
- return true;
-}
-
-bool WebVTTParser::hasRequiredFileIdentifier(const char* data, unsigned length)
-{
- // A WebVTT file identifier consists of an optional BOM character,
- // the string "WEBVTT" followed by an optional space or tab character,
- // and any number of characters that are not line terminators ...
- unsigned position = 0;
- if (length >= bomLength && data[0] == '\xEF' && data[1] == '\xBB' && data[2] == '\xBF')
- position += bomLength;
- String line = collectNextLine(data, length, &position);
-
- if (line.length() < fileIdentiferLength)
- return false;
- if (line.length() == fileIdentiferLength && line != "WEBVTT")
- return false;
- if (!hasLongWebVTTIdentifier(line))
- return false;
-
- return true;
-}
-
String WebVTTParser::collectDigits(const String& input, unsigned* position)
{
StringBuilder digits;
@@ -133,10 +91,20 @@
switch (m_state) {
case Initial:
+ // Buffer up at least 9 bytes before proceeding with checking for the file identifier.
+ m_identifierData.append(data, length);
+ if (m_identifierData.size() < bomLength + fileIdentifierLength)
+ return;
+
// 4-12 - Collect the first line and check for "WEBVTT".
- if (!hasRequiredFileIdentifier(data, length))
+ if (!hasRequiredFileIdentifier()) {
+ if (m_client)
+ m_client->fileFailedToParse();
return;
+ }
+
m_state = Header;
+ m_identifierData.clear();
break;
case Header:
@@ -173,6 +141,26 @@
}
}
+bool WebVTTParser::hasRequiredFileIdentifier()
+{
+ // A WebVTT file identifier consists of an optional BOM character,
+ // the string "WEBVTT" followed by an optional space or tab character,
+ // and any number of characters that are not line terminators ...
+ unsigned position = 0;
+ if (m_identifierData.size() >= bomLength && m_identifierData[0] == '\xEF' && m_identifierData[1] == '\xBB' && m_identifierData[2] == '\xBF')
+ position += bomLength;
+ String line = collectNextLine(m_identifierData.data(), m_identifierData.size(), &position);
+
+ if (line.length() < fileIdentifierLength)
+ return false;
+ if (line.substring(0, fileIdentifierLength) != "WEBVTT")
+ return false;
+ if (line.length() > fileIdentifierLength && line[fileIdentifierLength] != ' ' && line[fileIdentifierLength] != '\t')
+ return false;
+
+ return true;
+}
+
WebVTTParser::ParseState WebVTTParser::collectCueId(const String& line)
{
if (line.contains("-->"))
Modified: trunk/Source/WebCore/html/track/WebVTTParser.h (113342 => 113343)
--- trunk/Source/WebCore/html/track/WebVTTParser.h 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/Source/WebCore/html/track/WebVTTParser.h 2012-04-05 18:32:22 UTC (rev 113343)
@@ -50,6 +50,7 @@
virtual ~WebVTTParserClient() { }
virtual void newCuesParsed() = 0;
+ virtual void fileFailedToParse() = 0;
};
class WebVTTParser {
@@ -63,9 +64,6 @@
return adoptPtr(new WebVTTParser(client, context));
}
- static unsigned fileIdentifierMaximumLength();
- static bool hasRequiredFileIdentifier(const char* data, unsigned length);
-
static inline bool isRecognizedTag(const AtomicString& tagName)
{
return tagName == iTag
@@ -104,6 +102,7 @@
ParseState m_state;
private:
+ bool hasRequiredFileIdentifier();
ParseState collectCueId(const String&);
ParseState collectTimingsAndSettings(const String&);
ParseState collectCueText(const String&, unsigned length, unsigned);
@@ -118,6 +117,7 @@
void constructTreeFromToken(Document*);
+ Vector<char> m_identifierData;
String m_currentId;
double m_currentStartTime;
double m_currentEndTime;
Modified: trunk/Source/WebCore/loader/TextTrackLoader.cpp (113342 => 113343)
--- trunk/Source/WebCore/loader/TextTrackLoader.cpp 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/Source/WebCore/loader/TextTrackLoader.cpp 2012-04-05 18:32:22 UTC (rev 113343)
@@ -89,46 +89,16 @@
if (m_parseOffset == buffer->size())
return;
+ if (!m_cueParser)
+ m_cueParser = WebVTTParser::create(this, m_scriptExecutionContext);
+
const char* data;
unsigned length;
-
- if (!m_cueParser) {
- if (resource->response().mimeType() == "text/vtt")
- m_cueParser = WebVTTParser::create(this, m_scriptExecutionContext);
- else {
- // Don't proceed until we have enough data to check for the WebVTT magic identifier.
- unsigned identifierLength = WebVTTParser::fileIdentifierMaximumLength();
- if (buffer->size() < identifierLength)
- return;
-
- Vector<char> identifier;
- unsigned offset = 0;
- while (offset < identifierLength && (length = buffer->getSomeData(data, offset))) {
- if (length > identifierLength)
- length = identifierLength;
- identifier.append(data, length);
- offset += length;
- }
-
- if (!WebVTTParser::hasRequiredFileIdentifier(identifier.data(), identifier.size())) {
- LOG(Media, "TextTrackLoader::didReceiveData - file \"%s\" does not have WebVTT magic header",
- resource->response().url().string().utf8().data());
- m_state = Failed;
- m_cueLoadTimer.startOneShot(0);
- return;
- }
-
- m_cueParser = WebVTTParser::create(this, m_scriptExecutionContext);
- }
- }
-
- ASSERT(m_cueParser);
-
+
while ((length = buffer->getSomeData(data, m_parseOffset))) {
m_cueParser->parseBytes(data, length);
m_parseOffset += length;
}
-
}
void TextTrackLoader::didReceiveData(CachedResource* resource)
@@ -215,6 +185,18 @@
m_cueLoadTimer.startOneShot(0);
}
+void TextTrackLoader::fileFailedToParse()
+{
+ LOG(Media, "TextTrackLoader::fileFailedToParse");
+
+ m_state = Failed;
+
+ if (!m_cueLoadTimer.isActive())
+ m_cueLoadTimer.startOneShot(0);
+
+ cancelLoad();
+}
+
void TextTrackLoader::getNewCues(Vector<RefPtr<TextTrackCue> >& outputCues)
{
ASSERT(m_cueParser);
Modified: trunk/Source/WebCore/loader/TextTrackLoader.h (113342 => 113343)
--- trunk/Source/WebCore/loader/TextTrackLoader.h 2012-04-05 18:28:05 UTC (rev 113342)
+++ trunk/Source/WebCore/loader/TextTrackLoader.h 2012-04-05 18:32:22 UTC (rev 113343)
@@ -73,6 +73,7 @@
// WebVTTParserClient
virtual void newCuesParsed();
+ virtual void fileFailedToParse();
TextTrackLoader(TextTrackLoaderClient*, ScriptExecutionContext*);