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

Source/core/html/track/TextTrackCue.cpp

Index: Source/core/html/track/TextTrackCue.cpp
diff --git a/Source/core/html/track/TextTrackCue.cpp b/Source/core/html/track/TextTrackCue.cpp
index f80d78cbc5f8fc8bff2c3be130ea49516817f4c0..1fb9a58dfa0fa49e7b41f689a0cde065e518e54d 100644
--- a/Source/core/html/track/TextTrackCue.cpp
+++ b/Source/core/html/track/TextTrackCue.cpp
@@ -30,11 +30,11 @@
  */
 
 #include "config.h"
-
 #include "core/html/track/TextTrackCue.h"
 
 #include "CSSPropertyNames.h"
 #include "CSSValueKeywords.h"
+#include "RuntimeEnabledFeatures.h"
 #include "bindings/v8/ExceptionStatePlaceholder.h"
 #include "core/dom/DocumentFragment.h"
 #include "core/events/Event.h"
@@ -107,12 +107,10 @@ TextTrackCue* TextTrackCueBox::getCue() const
 void TextTrackCueBox::applyCSSProperties(const IntSize&)
 {
     // FIXME: Apply all the initial CSS positioning properties. http://wkb.ug/79916
-#if ENABLE(WEBVTT_REGIONS)
-    if (!m_cue->regionId().isEmpty()) {
+    if (RuntimeEnabledFeatures::webVTTRegionsEnabled() && !m_cue->regionId().isEmpty()) {

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

nit: Can the regionId be non-empty if this flag is not set? If not, then drop
the flag check.

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

Done.

         setInlineStyleProperty(CSSPropertyPosition, CSSValueRelative);
         return;
     }
-#endif
 
     // 3.5.1 On the (root) List of WebVTT Node Objects:
 
@@ -539,7 +537,6 @@ bool TextTrackCue::dispatchEvent(PassRefPtr<Event> event)
     return EventTarget::dispatchEvent(event);
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 void TextTrackCue::setRegionId(const String& regionId)
 {
     if (m_regionId == regionId)
@@ -549,7 +546,11 @@ void TextTrackCue::setRegionId(const String& regionId)
     m_regionId = regionId;
     cueDidChange();
 }
-#endif
+
+void TextTrackCue::notifyRegionWhenRemovingDisplayTree(bool notifyRegion)
+{
+    m_notifyRegion = notifyRegion;
+}
 
 bool TextTrackCue::isActive()
 {
@@ -830,12 +831,12 @@ PassRefPtr<TextTrackCueBox> TextTrackCue::getDisplayTree(const IntSize& videoSiz
 
 void TextTrackCue::removeDisplayTree()
 {
-#if ENABLE(WEBVTT_REGIONS)
-    // The region needs to be informed about the cue removal.
-    TextTrackRegion* region = m_track->regions()->getRegionById(m_regionId);
-    if (region)
-        region->willRemoveTextTrackCueBox(m_displayTree.get());
-#endif
+    if (RuntimeEnabledFeatures::webVTTRegionsEnabled() && m_notifyRegion) {

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

nit: Do you really need the flag check here? What is it protecting?

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

Removed, added the proper check that track->regions() isn't NULL.

+        // The region needs to be informed about the cue removal.
+        TextTrackRegion* region = m_track->regions()->getRegionById(m_regionId);
+        if (region)
+            region->willRemoveTextTrackCueBox(m_displayTree.get());
+    }
 
     displayTreeInternal()->remove(ASSERT_NO_EXCEPTION);
 }
@@ -885,9 +886,7 @@ TextTrackCue::CueSetting TextTrackCue::settingName(const String& name)
     DEFINE_STATIC_LOCAL(const String, positionKeyword, ("position"));
     DEFINE_STATIC_LOCAL(const String, sizeKeyword, ("size"));
     DEFINE_STATIC_LOCAL(const String, alignKeyword, ("align"));
-#if ENABLE(WEBVTT_REGIONS)
     DEFINE_STATIC_LOCAL(const String, regionIdKeyword, ("region"));
-#endif
 
     if (name == verticalKeyword)
         return Vertical;
@@ -899,10 +898,8 @@ TextTrackCue::CueSetting TextTrackCue::settingName(const String& name)
         return Size;
     else if (name == alignKeyword)
         return Align;
-#if ENABLE(WEBVTT_REGIONS)
-    else if (name == regionIdKeyword)
+    else if (RuntimeEnabledFeatures::webVTTRegionsEnabled() && name == regionIdKeyword)
         return RegionId;
-#endif
 
     return None;
 }
@@ -1096,11 +1093,11 @@ void TextTrackCue::setCueSettings(const String& input)
                 m_cueAlignment = End;
             }
             break;
-#if ENABLE(WEBVTT_REGIONS)
         case RegionId:
-            m_regionId = WebVTTParser::collectWord(input, &position);
-            break;
-#endif
+            if (RuntimeEnabledFeatures::webVTTRegionsEnabled()) {

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

nit: The check in settingName() should make this check unnecessary and prevent
this code getting executed right? If so please remove this flag check.

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

Done.

+                m_regionId = WebVTTParser::collectWord(input, &position);
+                break;
+            }
         case None:
             break;
         }
@@ -1108,16 +1105,17 @@ void TextTrackCue::setCueSettings(const String& input)
 NextSetting:
         position = endOfSetting;
     }
-#if ENABLE(WEBVTT_REGIONS)
-    // If cue's line position is not auto or cue's size is not 100 or cue's
-    // writing direction is not horizontal, but cue's region identifier is not
-    // the empty string, let cue's region identifier be the empty string.
-    if (m_regionId.isEmpty())
-        return;
 
-    if (m_linePosition != undefinedPosition || m_cueSize != 100 || m_writingDirection != Horizontal)
-        m_regionId = emptyString();
-#endif
+    if (RuntimeEnabledFeatures::webVTTRegionsEnabled()) {

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

nit: ditto. If m_regionId can only become non-empty when this flag is enabled,
then I think the flag check should be removed.

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

Done.

+        // If cue's line position is not auto or cue's size is not 100 or cue's
+        // writing direction is not horizontal, but cue's region identifier is not
+        // the empty string, let cue's region identifier be the empty string.
+        if (m_regionId.isEmpty())
+            return;
+
+        if (m_linePosition != undefinedPosition || m_cueSize != 100 || m_writingDirection != Horizontal)
+            m_regionId = emptyString();
+    }
 }
 
 CSSValueID TextTrackCue::getCSSWritingDirection() const