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

Source/core/html/track/TextTrackCue.cpp

 /*
  * Copyright (C) 2011 Google Inc.  All rights reserved.
  * Copyright (C) 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 12 matching lines @@
  * OWNER 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 "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"
 #include "core/dom/NodeTraversal.h"
 #include "core/html/HTMLDivElement.h"
 #include "core/html/track/TextTrack.h"
 #include "core/html/track/TextTrackCueList.h"
 #include "core/html/track/TextTrackRegionList.h"
 #include "core/html/track/WebVTTElement.h"
 #include "core/html/track/WebVTTParser.h"
@@ skipping 52 matching lines @@
 }
 
 TextTrackCue* TextTrackCueBox::getCue() const
 {
     return m_cue;
 }
 
 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:
 
     // the 'position' property must be set to 'absolute'
     setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
 
     //  the 'unicode-bidi' property must be set to 'plaintext'
     setInlineStyleProperty(CSSPropertyUnicodeBidi, CSSValueWebkitPlaintext);
 
     // the 'direction' property must be set to direction
@@ skipping 406 matching lines @@
 
 bool TextTrackCue::dispatchEvent(PassRefPtr<Event> event)
 {
     // When a TextTrack's mode is disabled: no cues are active, no events fired.
     if (!track() || track()->mode() == TextTrack::disabledKeyword())
         return false;
 
     return EventTarget::dispatchEvent(event);
 }
 
-#if ENABLE(WEBVTT_REGIONS)
 void TextTrackCue::setRegionId(const String& regionId)
 {
     if (m_regionId == regionId)
         return;
 
     cueWillChange();
     m_regionId = regionId;
     cueDidChange();
 }
-#endif
+
+void TextTrackCue::notifyRegionWhenRemovingDisplayTree(bool notifyRegion)
+{
+    m_notifyRegion = notifyRegion;
+}
 
 bool TextTrackCue::isActive()
 {
     return m_isActive && track() && track()->mode() != TextTrack::disabledKeyword();
 }
 
 void TextTrackCue::setIsActive(bool active)
 {
     m_isActive = active;
 
@@ skipping 260 matching lines @@
 
     m_displayTreeShouldChange = false;
 
     // 10.15. Let cue's text track cue display state have the CSS boxes in
     // boxes.
     return displayTree;
 }
 
 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);
 }
 
 std::pair<double, double> TextTrackCue::getPositionCoordinates() const
 {
     // This method is used for setting x and y when snap to lines is not set.
     std::pair<double, double> coordinates;
 
     if (m_writingDirection == Horizontal && m_displayDirection == CSSValueLtr) {
@@ skipping 29 matching lines @@
     return coordinates;
 }
 
 TextTrackCue::CueSetting TextTrackCue::settingName(const String& name)
 {
     DEFINE_STATIC_LOCAL(const String, verticalKeyword, ("vertical"));
     DEFINE_STATIC_LOCAL(const String, lineKeyword, ("line"));
     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;
     else if (name == lineKeyword)
         return Line;
     else if (name == positionKeyword)
         return Position;
     else if (name == sizeKeyword)
         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;
 }
 
 void TextTrackCue::setCueSettings(const String& input)
 {
     m_settings = input;
     unsigned position = 0;
 
     while (position < input.length()) {
@@ skipping 173 matching lines @@
 
             // 2. If value is a case-sensitive match for the string "middle", then let cue's text track cue alignment be middle alignment.
             else if (cueAlignment == middleKeyword())
                 m_cueAlignment = Middle;
 
             // 3. If value is a case-sensitive match for the string "end", then let cue's text track cue alignment be end alignment.
             else if (cueAlignment == endKeyword())
                 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;
         }
 
 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
 {
     return m_displayDirection;
 }
 
 CSSValueID TextTrackCue::getCSSWritingMode() const
 {
     return m_displayWritingMode;
@@ skipping 53 matching lines @@
         return false;
     if (m_cueSize != cue.size())
         return false;
     if (align() != cue.align())
         return false;
 
     return true;
 }
 
 } // namespace WebCore