WebKit Bugzilla
Attachment 357015 Details for
Bug 192575
: Use WeakPtr to refer to VTTCue in VTTCueBox
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Cleanup
bug-192575-20181210163905.patch (text/plain), 9.85 KB, created by
Ryosuke Niwa
on 2018-12-10 16:39:06 PST
(
hide
)
Description:
Cleanup
Filename:
MIME Type:
Creator:
Ryosuke Niwa
Created:
2018-12-10 16:39:06 PST
Size:
9.85 KB
patch
obsolete
>Index: Source/WebCore/ChangeLog >=================================================================== >--- Source/WebCore/ChangeLog (revision 239060) >+++ Source/WebCore/ChangeLog (working copy) >@@ -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 NOBODY (OOPS!). >+ >+ 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 Antti Koivisto <antti@apple.com> > > Rename "forced style recalc" to "full style rebuild" >Index: Source/WebCore/html/track/TextTrackCueGeneric.cpp >=================================================================== >--- Source/WebCore/html/track/TextTrackCueGeneric.cpp (revision 239058) >+++ Source/WebCore/html/track/TextTrackCueGeneric.cpp (working copy) >@@ -68,10 +68,13 @@ TextTrackCueGenericBoxElement::TextTrack > > 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::appl > 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); > } > } > >Index: Source/WebCore/html/track/VTTCue.cpp >=================================================================== >--- Source/WebCore/html/track/VTTCue.cpp (revision 239058) >+++ Source/WebCore/html/track/VTTCue.cpp (working copy) >@@ -133,20 +133,25 @@ Ref<VTTCueBox> VTTCueBox::create(Documen > > 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 > 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 > 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 > > // 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& c > > 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) >Index: Source/WebCore/html/track/VTTCue.h >=================================================================== >--- Source/WebCore/html/track/VTTCue.h (revision 239058) >+++ Source/WebCore/html/track/VTTCue.h (working copy) >@@ -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) > {
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
eric.carlson
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192575
: 357015