Use WeakPtr to refer to VTTCue in VTTCueBox

https://bugs.webkit.org/show_bug.cgi?id=192575

Reviewed by Eric Carlson.

Address the FIXME in VTTCue::~VTTCue by clearing VTTCueBox::m_cue when VTTCue goes away.
This is implemented by simply using WeakPtr.

No new tests since there shoul be no behaivoral change.

* html/track/TextTrackCueGeneric.cpp:
(WebCore::TextTrackCueGenericBoxElement::applyCSSProperties):
* html/track/VTTCue.cpp:
(WebCore::VTTCueBox::VTTCueBox):
(WebCore::VTTCueBox::getCue const):
(WebCore::VTTCueBox::applyCSSProperties):
(WebCore::VTTCue::~VTTCue):
* html/track/VTTCue.h:
(WebCore::VTTCueBox::fontSizeFromCaptionUserPrefs const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239063 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 7d19692..ce61833 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2018-12-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Use WeakPtr to refer to VTTCue in VTTCueBox
+        https://bugs.webkit.org/show_bug.cgi?id=192575
+
+        Reviewed by Eric Carlson.
+
+        Address the FIXME in VTTCue::~VTTCue by clearing VTTCueBox::m_cue when VTTCue goes away.
+        This is implemented by simply using WeakPtr.
+
+        No new tests since there shoul be no behaivoral change.
+
+        * html/track/TextTrackCueGeneric.cpp:
+        (WebCore::TextTrackCueGenericBoxElement::applyCSSProperties):
+        * html/track/VTTCue.cpp:
+        (WebCore::VTTCueBox::VTTCueBox):
+        (WebCore::VTTCueBox::getCue const):
+        (WebCore::VTTCueBox::applyCSSProperties):
+        (WebCore::VTTCue::~VTTCue):
+        * html/track/VTTCue.h:
+        (WebCore::VTTCueBox::fontSizeFromCaptionUserPrefs const):
+
 2018-12-10  Mark Lam  <mark.lam@apple.com>
 
         PropertyAttribute needs a CustomValue bit.

diff --git a/Source/WebCore/html/track/TextTrackCueGeneric.cpp b/Source/WebCore/html/track/TextTrackCueGeneric.cpp
index 14203db..972e1d0 100644 (file)
--- a/Source/WebCore/html/track/TextTrackCueGeneric.cpp
+++ b/Source/WebCore/html/track/TextTrackCueGeneric.cpp
@@ -68,10 +68,13 @@ TextTrackCueGenericBoxElement::TextTrackCueGenericBoxElement(Document& document,
 
 void TextTrackCueGenericBoxElement::applyCSSProperties(const IntSize& videoSize)
 {
+    RefPtr<TextTrackCueGeneric> cue = static_cast<TextTrackCueGeneric*>(getCue());
+    if (!cue)
+        return;
+
     setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
     setInlineStyleProperty(CSSPropertyUnicodeBidi, CSSValuePlaintext);
-    
-    RefPtr<TextTrackCueGeneric> cue = static_cast<TextTrackCueGeneric*>(getCue());
+
     Ref<HTMLSpanElement> cueElement = cue->element();
 
     double textPosition = cue->calculateComputedTextPosition();
@@ -92,16 +95,16 @@ void TextTrackCueGenericBoxElement::applyCSSProperties(const IntSize& videoSize)
         if (cue->fontSizeMultiplier())
             authorFontSize *= cue->fontSizeMultiplier() / 100;
 
-        double multiplier = m_fontSizeFromCaptionUserPrefs / authorFontSize;
+        double multiplier = fontSizeFromCaptionUserPrefs() / authorFontSize;
         double newCueSize = std::min(size * multiplier, 100.0);
         if (cue->getWritingDirection() == VTTCue::Horizontal) {
             setInlineStyleProperty(CSSPropertyWidth, newCueSize, CSSPrimitiveValue::CSS_PERCENTAGE);
             if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
-                setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(textPosition - (newCueSize - m_cue.getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
+                setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(textPosition - (newCueSize - cue->getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
         } else {
             setInlineStyleProperty(CSSPropertyHeight, newCueSize,  CSSPrimitiveValue::CSS_PERCENTAGE);
             if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
-                setInlineStyleProperty(CSSPropertyTop, static_cast<double>(cue->line() - (newCueSize - m_cue.getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
+                setInlineStyleProperty(CSSPropertyTop, static_cast<double>(cue->line() - (newCueSize - cue->getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
         }
     }
 

diff --git a/Source/WebCore/html/track/VTTCue.cpp b/Source/WebCore/html/track/VTTCue.cpp
index 35c8176..68f5120 100644 (file)
--- a/Source/WebCore/html/track/VTTCue.cpp
+++ b/Source/WebCore/html/track/VTTCue.cpp
@@ -133,20 +133,25 @@ Ref<VTTCueBox> VTTCueBox::create(Document& document, VTTCue& cue)
 
 VTTCueBox::VTTCueBox(Document& document, VTTCue& cue)
     : HTMLElement(divTag, document)
-    , m_cue(cue)
+    , m_cue(makeWeakPtr(cue))
 {
     setPseudo(vttCueBoxShadowPseudoId());
 }
 
 VTTCue* VTTCueBox::getCue() const
 {
-    return &m_cue;
+    return m_cue.get();
 }
 
 void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
 {
+    if (!m_cue)
+        return;
+
+    auto cue = makeRef(*m_cue);
+
     // FIXME: Apply all the initial CSS positioning properties. http://wkb.ug/79916
-    if (!m_cue.regionId().isEmpty()) {
+    if (!cue->regionId().isEmpty()) {
         setInlineStyleProperty(CSSPropertyPosition, CSSValueRelative);
         return;
     }
@@ -160,12 +165,12 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
     setInlineStyleProperty(CSSPropertyUnicodeBidi, CSSValuePlaintext);
 
     // the 'direction' property must be set to direction
-    setInlineStyleProperty(CSSPropertyDirection, m_cue.getCSSWritingDirection());
+    setInlineStyleProperty(CSSPropertyDirection, cue->getCSSWritingDirection());
 
     // the 'writing-mode' property must be set to writing-mode
-    setInlineStyleProperty(CSSPropertyWritingMode, m_cue.getCSSWritingMode(), false);
+    setInlineStyleProperty(CSSPropertyWritingMode, cue->getCSSWritingMode(), false);
 
-    auto position = m_cue.getCSSPosition();
+    auto position = cue->getCSSPosition();
 
     // the 'top' property must be set to top,
     setInlineStyleProperty(CSSPropertyTop, position.second, CSSPrimitiveValue::CSS_PERCENTAGE);
@@ -178,39 +183,39 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
     if (authorFontSize)
         multiplier = m_fontSizeFromCaptionUserPrefs / authorFontSize;
 
-    double textPosition = m_cue.calculateComputedTextPosition();
+    double textPosition = cue->calculateComputedTextPosition();
     double maxSize = 100.0;
-    CSSValueID alignment = m_cue.getCSSAlignment();
+    CSSValueID alignment = cue->getCSSAlignment();
     if (alignment == CSSValueEnd || alignment == CSSValueRight)
         maxSize = textPosition;
     else if (alignment == CSSValueStart || alignment == CSSValueLeft)
         maxSize = 100.0 - textPosition;
 
-    double newCueSize = std::min(m_cue.getCSSSize() * multiplier, 100.0);
+    double newCueSize = std::min(cue->getCSSSize() * multiplier, 100.0);
     // the 'width' property must be set to width, and the 'height' property  must be set to height
-    if (m_cue.vertical() == horizontalKeyword()) {
+    if (cue->vertical() == horizontalKeyword()) {
         setInlineStyleProperty(CSSPropertyWidth, newCueSize, CSSPrimitiveValue::CSS_PERCENTAGE);
         setInlineStyleProperty(CSSPropertyHeight, CSSValueAuto);
         setInlineStyleProperty(CSSPropertyMinWidth, "min-content");
         setInlineStyleProperty(CSSPropertyMaxWidth, maxSize, CSSPrimitiveValue::CSS_PERCENTAGE);
         if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
-            setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(position.first - (newCueSize - m_cue.getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
+            setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(position.first - (newCueSize - cue->getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
     } else {
         setInlineStyleProperty(CSSPropertyWidth, CSSValueAuto);
         setInlineStyleProperty(CSSPropertyHeight, newCueSize, CSSPrimitiveValue::CSS_PERCENTAGE);
         setInlineStyleProperty(CSSPropertyMinHeight, "min-content");
         setInlineStyleProperty(CSSPropertyMaxHeight, maxSize, CSSPrimitiveValue::CSS_PERCENTAGE);
         if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
-            setInlineStyleProperty(CSSPropertyTop, static_cast<double>(position.second - (newCueSize - m_cue.getCSSSize()) / 2), CSSPrimitiveValue::CSS_PERCENTAGE);
+            setInlineStyleProperty(CSSPropertyTop, static_cast<double>(position.second - (newCueSize - cue->getCSSSize()) / 2), 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, m_cue.getCSSAlignment());
+    setInlineStyleProperty(CSSPropertyTextAlign, cue->getCSSAlignment());
     
-    if (!m_cue.snapToLines()) {
+    if (!cue->snapToLines()) {
         // 10.13.1 Set up x and y:
         // Note: x and y are set through the CSS left and top above.
 
@@ -229,7 +234,7 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
 
     // Make sure shadow or stroke is not clipped.
     setInlineStyleProperty(CSSPropertyOverflow, CSSValueVisible);
-    m_cue.element().setInlineStyleProperty(CSSPropertyOverflow, CSSValueVisible);
+    cue->element().setInlineStyleProperty(CSSPropertyOverflow, CSSValueVisible);
 }
 
 const AtomicString& VTTCueBox::vttCueBoxShadowPseudoId()
@@ -277,9 +282,6 @@ VTTCue::VTTCue(ScriptExecutionContext& context, const WebVTTCueData& cueData)
 
 VTTCue::~VTTCue()
 {
-    // FIXME: We should set m_cue in VTTCueBox to nullptr instead.
-    if (m_displayTree && m_displayTree->document().refCount())
-        m_displayTree->remove();
 }
 
 void VTTCue::initialize(ScriptExecutionContext& context)

diff --git a/Source/WebCore/html/track/VTTCue.h b/Source/WebCore/html/track/VTTCue.h
index 37cee4f..a5c230f 100644 (file)
--- a/Source/WebCore/html/track/VTTCue.h
+++ b/Source/WebCore/html/track/VTTCue.h
@@ -64,13 +64,16 @@ protected:
 
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
 
-    VTTCue& m_cue;
+    int fontSizeFromCaptionUserPrefs() const { return m_fontSizeFromCaptionUserPrefs; }
+
+private:
+    WeakPtr<VTTCue> m_cue;
     int m_fontSizeFromCaptionUserPrefs;
 };
 
 // ----------------------------
 
-class VTTCue : public TextTrackCue {
+class VTTCue : public TextTrackCue, public CanMakeWeakPtr<VTTCue> {
 public:
     static Ref<VTTCue> create(ScriptExecutionContext& context, double start, double end, const String& content)
     {