Separate VTTCue from VTTCueBox and LayoutVTTCue

Source/core/html/track/vtt/VTTCue.cpp

 /*
  * Copyright (c) 2013, Opera Software ASA. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 127 matching lines @@
 }
 
 // Sets inline CSS properties on passed in element if value is not an empty string
 static void setInlineStylePropertyIfNotEmpty(Element& element,
     CSSPropertyID propertyID, const String& value)
 {
     if (!value.isEmpty())
         element.setInlineStyleProperty(propertyID, value);
 }
 
-VTTCueBox::VTTCueBox(Document& document, VTTCue* cue)
+VTTCueBox::VTTCueBox(Document& document)
     : HTMLDivElement(document)
-    , m_cue(cue)
+    , m_snapToLinesPosition(std::numeric_limits<float>::quiet_NaN())
 {
     setShadowPseudoId(AtomicString("-webkit-media-text-track-display", AtomicString::ConstructFromLiteral));
 }
 
 void VTTCueBox::applyCSSProperties(const VTTDisplayParameters& displayParameters)
 {
-    // FIXME: Apply all the initial CSS positioning properties. http://wkb.ug/79916
-    if (!m_cue->regionId().isEmpty()) {
-        setInlineStyleProperty(CSSPropertyPosition, CSSValueRelative);
-        return;
-    }
+    // http://dev.w3.org/html5/webvtt/#applying-css-properties-to-webvtt-node-objects
 
-    // 3.5.1 On the (root) List of WebVTT Node Objects:
+    // Initialize the (root) list of WebVTT Node Objects with the following CSS settings:
 
     // 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
     setInlineStyleProperty(CSSPropertyDirection, displayParameters.direction);
 
     // the 'writing-mode' property must be set to writing-mode
     setInlineStyleProperty(CSSPropertyWebkitWritingMode, displayParameters.writingMode);
 
     const FloatPoint& position = displayParameters.position;
 
     // the 'top' property must be set to top,
     setInlineStyleProperty(CSSPropertyTop, position.y(), CSSPrimitiveValue::CSS_PERCENTAGE);
 
     // the 'left' property must be set to left
     setInlineStyleProperty(CSSPropertyLeft, position.x(), CSSPrimitiveValue::CSS_PERCENTAGE);
 
     // the 'width' property must be set to width, and the 'height' property  must be set to height
-    if (m_cue->vertical() == horizontalKeyword()) {
+    if (displayParameters.writingMode == CSSValueHorizontalTb) {
         setInlineStyleProperty(CSSPropertyWidth, displayParameters.size, CSSPrimitiveValue::CSS_PERCENTAGE);
         setInlineStyleProperty(CSSPropertyHeight, CSSValueAuto);
     } else {
         setInlineStyleProperty(CSSPropertyWidth, CSSValueAuto);
         setInlineStyleProperty(CSSPropertyHeight, displayParameters.size,  CSSPrimitiveValue::CSS_PERCENTAGE);
     }
 
     // The 'text-align' property on the (root) List of WebVTT Node Objects must
     // be set to the value in the second cell of the row of the table below
     // whose first cell is the value of the corresponding cue's text track cue
     // alignment:
-    setInlineStyleProperty(CSSPropertyTextAlign, displayAlignmentMap[m_cue->cueAlignment()]);
+    setInlineStyleProperty(CSSPropertyTextAlign, displayParameters.textAlign);
 
-    if (!m_cue->snapToLines()) {
+    // TODO(philipj): The position adjustment for non-snap-to-lines cues has
+    // been removed from the spec:
+    // https://www.w3.org/Bugs/Public/show_bug.cgi?id=19178
+    if (std::isnan(displayParameters.snapToLinesPosition)) {
         // 10.13.1 Set up x and y:
         // Note: x and y are set through the CSS left and top above.
 
         // 10.13.2 Position the boxes in boxes such that the point x% along the
         // width of the bounding box of the boxes in boxes is x% of the way
         // across the width of the video's rendering area, and the point y%
         // along the height of the bounding box of the boxes in boxes is y%
         // of the way across the height of the video's rendering area, while
         // maintaining the relative positions of the boxes in boxes to each
         // other.
         setInlineStyleProperty(CSSPropertyTransform,
             String::format("translate(-%.2f%%, -%.2f%%)", position.x(), position.y()));
 
         setInlineStyleProperty(CSSPropertyWhiteSpace, CSSValuePre);
     }
+
+    // The snap-to-lines position is propagated to LayoutVTTCue.
+    m_snapToLinesPosition = displayParameters.snapToLinesPosition;
 }
 
-LayoutObject* VTTCueBox::createLayoutObject(const ComputedStyle&)
+LayoutObject* VTTCueBox::createLayoutObject(const ComputedStyle& style)
 {
-    return new LayoutVTTCue(this);
-}
+    // If WebVTT Regions are used, the regular WebVTT layout algorithm is no
+    // longer necessary, since cues having the region parameter set do not have
+    // any positioning parameters. Also, in this case, the regions themselves
+    // have positioning information.
+    if (style.position() == RelativePosition)
+        return HTMLDivElement::createLayoutObject(style);
 
-DEFINE_TRACE(VTTCueBox)
-{
-    visitor->trace(m_cue);
-    HTMLDivElement::trace(visitor);
+    return new LayoutVTTCue(this, m_snapToLinesPosition);
 }
 
 VTTCue::VTTCue(Document& document, double startTime, double endTime, const String& text)
     : TextTrackCue(startTime, endTime)
     , m_text(text)
     , m_linePosition(std::numeric_limits<float>::quiet_NaN())
     , m_textPosition(std::numeric_limits<float>::quiet_NaN())
     , m_cueSize(100)
     , m_writingDirection(Horizontal)
     , m_cueAlignment(Middle)
     , m_vttNodeTree(nullptr)
     , m_cueBackgroundBox(HTMLDivElement::create(document))
     , m_snapToLines(true)
     , m_displayTreeShouldChange(true)
 {
     UseCounter::count(document, UseCounter::VTTCue);
     m_cueBackgroundBox->setShadowPseudoId(cueShadowPseudoId());
 }
 
-VTTCue::~VTTCue()

[sof, 2015/07/20 08:15:39]

It would be preferable to keep an empty destructor, to avoid synthesizing &
inlining a larger one.

[philipj_slow, 2015/07/20 08:34:16]

Done.

-{
-    // Using oilpan, if m_displayTree is in the document it will strongly keep
-    // the cue alive. Thus, if the cue is dead, either m_displayTree is not in
-    // the document or the entire document is dead too.
-#if !ENABLE(OILPAN)
-    // FIXME: This is scary, we should make the life cycle smarter so the destructor
-    // doesn't need to do DOM mutations.
-    if (m_displayTree)
-        m_displayTree->remove(ASSERT_NO_EXCEPTION);
-#endif
-}
-
 #ifndef NDEBUG
 String VTTCue::toString() const
 {
     return String::format("%p id=%s interval=%f-->%f cue=%s)", this, id().utf8().data(), startTime(), endTime(), text().utf8().data());
 }
 #endif
 
 void VTTCue::cueDidChange()
 {
     TextTrackCue::cueDidChange();
@@ skipping 369 matching lines @@
     case VTTCue::Right:
         return VTTCue::End;
     default:
         return m_cueAlignment;
     }
 }
 
 VTTDisplayParameters::VTTDisplayParameters()
     : size(std::numeric_limits<float>::quiet_NaN())
     , direction(CSSValueNone)
-    , writingMode(CSSValueNone) { }
+    , textAlign(CSSValueNone)
+    , writingMode(CSSValueNone)
+    , snapToLinesPosition(std::numeric_limits<float>::quiet_NaN()) { }
 
 VTTDisplayParameters VTTCue::calculateDisplayParameters() const
 {
     // http://dev.w3.org/html5/webvtt/#dfn-apply-webvtt-cue-settings
 
     VTTDisplayParameters displayParameters;
+
     // Steps 1 and 2.
     displayParameters.direction = determineTextDirection(m_vttNodeTree.get());
 
     if (displayParameters.direction == CSSValueRtl)
         UseCounter::count(document(), UseCounter::VTTCueRenderRtl);
 
+    // Note: The 'text-align' property is also determined here so that
+    // VTTCueBox::applyCSSProperties need not have access to a VTTCue.
+    displayParameters.textAlign = displayAlignmentMap[cueAlignment()];
+
     // 3. If the text track cue writing direction is horizontal, then let
     // block-flow be 'tb'. Otherwise, if the text track cue writing direction is
     // vertical growing left, then let block-flow be 'lr'. Otherwise, the text
     // track cue writing direction is vertical growing right; let block-flow be
     // 'rl'.
     displayParameters.writingMode = displayWritingModeMap[m_writingDirection];
 
     // Resolve the cue alignment to one of the values {start, end, middle}.
     CueAlignment computedCueAlignment = calculateComputedCueAlignment();
 
@@ skipping 69 matching lines @@
             displayParameters.position.setX(computedLinePosition);
     } else {
         if (m_writingDirection == Horizontal)
             displayParameters.position.setY(0);
         else
             displayParameters.position.setX(0);
     }
 
     // Step 9 not implemented (margin == 0).
 
+    // The snap-to-lines position is propagated to LayoutVTTCue.
+    displayParameters.snapToLinesPosition = m_snapToLines
+        ? computedLinePosition
+        : std::numeric_limits<float>::quiet_NaN();
+
     ASSERT(std::isfinite(displayParameters.size));
     ASSERT(displayParameters.direction != CSSValueNone);
     ASSERT(displayParameters.writingMode != CSSValueNone);
     return displayParameters;
 }
 
 void VTTCue::updatePastAndFutureNodes(double movieTime)
 {
     DEFINE_STATIC_LOCAL(const String, timestampTag, ("timestamp"));
 
@@ skipping 33 matching lines @@
                 toElement(child).setIdAttribute(id());
         }
     }
 }
 
 PassRefPtrWillBeRawPtr<VTTCueBox> VTTCue::getDisplayTree()
 {
     ASSERT(track() && track()->isRendered() && isActive());
 
     if (!m_displayTree) {
-        m_displayTree = VTTCueBox::create(document(), this);
+        m_displayTree = VTTCueBox::create(document());
         m_displayTree->appendChild(m_cueBackgroundBox);
     }
 
     ASSERT(m_displayTree->firstChild() == m_cueBackgroundBox);
 
     if (!m_displayTreeShouldChange) {
         // Apply updated user style overrides for text tracks when display tree doesn't change.
         // This ensures that the track settings are refreshed when the video is
         // replayed or when the user slides back to an already rendered track.
         applyUserOverrideCSSProperties();
         return m_displayTree;
     }
 
     createVTTNodeTree();
 
     m_cueBackgroundBox->removeChildren();
     m_vttNodeTree->cloneChildNodes(m_cueBackgroundBox.get());
 
-    VTTDisplayParameters displayParameters = calculateDisplayParameters();
-    m_displayTree->applyCSSProperties(displayParameters);
+    // TODO(philipj): The region identifier may be non-empty without there being
+    // a corresponding region, in which case this VTTCueBox will be added
+    // directly to the text track container in updateDisplay().
+    if (regionId().isEmpty()) {

[fs, 2015/07/17 14:31:48]

Maybe you could add a "VTTRegion* region() { return track() &&
track()->regions() ? ... : nullptr; }" and use that (here and all the other
places that accesses the region. IIRC that's what the spec looks like now too
right?). Anyway, can be done later, since is equivalent to what's there already
IIUC.

[philipj_slow, 2015/07/20 07:33:30]

Yes, I changed the spec such that it should be possible to have an id->region
mapping while parsing and then after that simply let a VTTCue point to a
VTTRegion. I didn't want to change any behavior here, though.

+        VTTDisplayParameters displayParameters = calculateDisplayParameters();
+        m_displayTree->applyCSSProperties(displayParameters);
+    } else {
+        m_displayTree->setInlineStyleProperty(CSSPropertyPosition, CSSValueRelative);
+    }
 
     // Apply user override settings for text tracks
     applyUserOverrideCSSProperties();
 
     m_displayTreeShouldChange = false;
 
     return m_displayTree;
 }
 
 void VTTCue::removeDisplayTree(RemovalNotification removalNotification)
@@ skipping 299 matching lines @@
 
 DEFINE_TRACE(VTTCue)
 {
     visitor->trace(m_vttNodeTree);
     visitor->trace(m_cueBackgroundBox);
     visitor->trace(m_displayTree);
     TextTrackCue::trace(visitor);
 }
 
 } // namespace blink