Enable WebVTT regions for runtime testing, updated tests and minor fixes

Source/core/html/track/WebVTTParser.cpp

Index: Source/core/html/track/WebVTTParser.cpp
diff --git a/Source/core/html/track/WebVTTParser.cpp b/Source/core/html/track/WebVTTParser.cpp
index e249da4cfd635e9f2b6dad3ce2f666bd4341e952..45ca0d807eda90e8475cfb4a7e5a323d3842ec3a 100644
--- a/Source/core/html/track/WebVTTParser.cpp
+++ b/Source/core/html/track/WebVTTParser.cpp
@@ -62,7 +62,6 @@ String WebVTTParser::collectWord(const String& input, unsigned* position)
     return string.toString();
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 float WebVTTParser::parseFloatPercentageValue(const String& value, bool& isValidSetting)
 {
     // '%' must be present and at the end of the setting value.
@@ -110,7 +109,6 @@ FloatPoint WebVTTParser::parseFloatPercentageValuePair(const String& value, char
     isValidSetting = isFirstValueValid && isSecondValueValid;
     return FloatPoint(firstCoord, secondCoord);
 }
-#endif
 
 WebVTTParser::WebVTTParser(WebVTTParserClient* client, ScriptExecutionContext* context)
     : m_scriptExecutionContext(context)
@@ -128,13 +126,11 @@ void WebVTTParser::getNewCues(Vector<RefPtr<TextTrackCue> >& outputCues)
     m_cuelist.clear();
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 void WebVTTParser::getNewRegions(Vector<RefPtr<TextTrackRegion> >& outputRegions)
 {
     outputRegions = m_regionList;
     m_regionList.clear();
 }
-#endif
 
 void WebVTTParser::parseBytes(const char* data, unsigned length)
 {
@@ -165,20 +161,19 @@ void WebVTTParser::parseBytes(const char* data, unsigned length)
 
         case Header:
             // 13-18 - Allow a header (comment area) under the WEBVTT line.
-#if ENABLE(WEBVTT_REGIONS)
             if (line.isEmpty()) {
-                if (m_client && m_regionList.size())
+                if (RuntimeEnabledFeatures::webVTTRegionsEnabled() && m_client && m_regionList.size())

[acolwell GONE FROM CHROMIUM, 2013/10/04 19:48:20]

nit: Can m_regionList.size() be > 0 if the flag is not set? If not then please
remove flag check.

[vcarbune.chromium, 2013/10/08 18:07:38]

Done.

                     m_client->newRegionsParsed();
 
                 m_state = Id;
                 break;
             }
-            collectHeader(line);
+            if (RuntimeEnabledFeatures::webVTTRegionsEnabled())

[acolwell GONE FROM CHROMIUM, 2013/10/04 19:48:20]

nit: Do you really need a flag check here? It seems like you should transition
to Metadata instead of Header if the flag isn't set instead of doing multiple
checks here. In fact I don't see any code that causes m_state transition to
Metadata.

[vcarbune.chromium, 2013/10/08 18:07:38]

I moved the flag check and inside the function and renamed it to
collectMetadataHeader to match the spec ("Metadata header loop"). I removed the
Metadata state, because it's not needed.

On the other hand, I think these steps need a spec re-sync (filed
http://crbug.com/305317)

+                collectHeader(line);
 
             break;
 
         case Metadata:
-#endif
             if (line.isEmpty())
                 m_state = Id;
             break;
@@ -231,7 +226,6 @@ bool WebVTTParser::hasRequiredFileIdentifier()
     return true;
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 void WebVTTParser::collectHeader(const String& line)
 {
     // 4.1 Extension of WebVTT header parsing (11 - 15)
@@ -253,7 +247,6 @@ void WebVTTParser::collectHeader(const String& line)
         createNewRegion();
     }
 }
-#endif
 
 WebVTTParser::ParseState WebVTTParser::collectCueId(const String& line)
 {
@@ -374,7 +367,6 @@ void WebVTTParser::resetCueValues()
     m_currentContent.clear();
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 void WebVTTParser::createNewRegion()
 {
     if (!m_currentHeaderValue.length())
@@ -393,7 +385,6 @@ void WebVTTParser::createNewRegion()
 
     m_regionList.append(region);
 }
-#endif
 
 double WebVTTParser::collectTimeStamp(const String& line, unsigned* position)
 {