InlineTextBox should own shadow data
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Mar 2018 23:26:46 +0000 (23:26 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Mar 2018 23:26:46 +0000 (23:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183359
<rdar://problem/38171343>

Reviewed by Darin Adler.

Following r229147 we recompute the selection style, including any shadow data, whenever we
paint the inline text box. Therefore, InlineTextBox needs to take ownership of the shadow
data or it may be deallocated before it can be used.

Covered by existing tests.

* rendering/InlineTextBox.cpp: Changed data type of InlineTextBox::MarkedTextStyle::textShadow
from const ShadowData* to std::optional<ShadowData>. Also removed explicitly deleted equality
and inequality operators as they are unnecessary. Layout tests should catch if these are ever
implemented and used when painting because the painted results will be wrong.
(WebCore::InlineTextBox::computeStyleForUnmarkedMarkedText const): Clone ShadowData.
(WebCore::InlineTextBox::resolveStyleForMarkedText): Simplified logic.
(WebCore::InlineTextBox::paintMarkedTextForeground): Modified code now that MarkedTextStyle
holds a std::optional<ShadowData>.
(WebCore::InlineTextBox::paintMarkedTextDecoration): Ditto.
* rendering/TextPaintStyle.cpp:
(WebCore::computeTextSelectionPaintStyle): Changed the out parameter type from const ShadowData*
to std::optional<ShadowData>& and modified code as needed.
* rendering/TextPaintStyle.h:
* rendering/style/ShadowData.cpp: Removed unncessary #include of header LayoutRect.h.
This header will be included via ShadowData.h.
(WebCore::ShadowData::clone): Convenience method that returns an std::optional to a
cloned ShadowData object.
* rendering/style/ShadowData.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/TextPaintStyle.cpp
Source/WebCore/rendering/TextPaintStyle.h
Source/WebCore/rendering/style/ShadowData.cpp
Source/WebCore/rendering/style/ShadowData.h

index 7857b27..80a696b 100644 (file)
@@ -1,3 +1,36 @@
+2018-03-10  Daniel Bates  <dabates@apple.com>
+
+        InlineTextBox should own shadow data
+        https://bugs.webkit.org/show_bug.cgi?id=183359
+        <rdar://problem/38171343>
+
+        Reviewed by Darin Adler.
+
+        Following r229147 we recompute the selection style, including any shadow data, whenever we
+        paint the inline text box. Therefore, InlineTextBox needs to take ownership of the shadow
+        data or it may be deallocated before it can be used.
+
+        Covered by existing tests.
+
+        * rendering/InlineTextBox.cpp: Changed data type of InlineTextBox::MarkedTextStyle::textShadow
+        from const ShadowData* to std::optional<ShadowData>. Also removed explicitly deleted equality
+        and inequality operators as they are unnecessary. Layout tests should catch if these are ever
+        implemented and used when painting because the painted results will be wrong.
+        (WebCore::InlineTextBox::computeStyleForUnmarkedMarkedText const): Clone ShadowData.
+        (WebCore::InlineTextBox::resolveStyleForMarkedText): Simplified logic.
+        (WebCore::InlineTextBox::paintMarkedTextForeground): Modified code now that MarkedTextStyle
+        holds a std::optional<ShadowData>.
+        (WebCore::InlineTextBox::paintMarkedTextDecoration): Ditto.
+        * rendering/TextPaintStyle.cpp:
+        (WebCore::computeTextSelectionPaintStyle): Changed the out parameter type from const ShadowData*
+        to std::optional<ShadowData>& and modified code as needed.
+        * rendering/TextPaintStyle.h:
+        * rendering/style/ShadowData.cpp: Removed unncessary #include of header LayoutRect.h.
+        This header will be included via ShadowData.h.
+        (WebCore::ShadowData::clone): Convenience method that returns an std::optional to a
+        cloned ShadowData object.
+        * rendering/style/ShadowData.h:
+
 2018-03-09  Zalan Bujtas  <zalan@apple.com>
 
         Turn off offset*/scroll* optimization for input elements with shadow content
index edea662..8551e2d 100644 (file)
@@ -393,8 +393,6 @@ bool InlineTextBox::emphasisMarkExistsAndIsAbove(const RenderStyle& style, bool&
 }
 
 struct InlineTextBox::MarkedTextStyle {
-    bool operator==(const MarkedTextStyle& other) const = delete;
-    bool operator!=(const MarkedTextStyle& other) const = delete;
     static bool areBackgroundMarkedTextStylesEqual(const MarkedTextStyle& a, const MarkedTextStyle& b)
     {
         return a.backgroundColor == b.backgroundColor;
@@ -411,7 +409,7 @@ struct InlineTextBox::MarkedTextStyle {
     Color backgroundColor;
     TextPaintStyle textStyles;
     TextDecorationPainter::Styles textDecorationStyles;
-    const ShadowData* textShadow;
+    std::optional<ShadowData> textShadow;
     float alpha;
 };
 
@@ -736,7 +734,7 @@ auto InlineTextBox::computeStyleForUnmarkedMarkedText(const PaintInfo& paintInfo
     MarkedTextStyle style;
     style.textDecorationStyles = TextDecorationPainter::stylesForRenderer(renderer(), lineStyle.textDecorationsInEffect(), isFirstLine());
     style.textStyles = computeTextPaintStyle(renderer().frame(), lineStyle, paintInfo);
-    style.textShadow = paintInfo.forceTextColor() ? nullptr : lineStyle.textShadow();
+    style.textShadow = ShadowData::clone(paintInfo.forceTextColor() ? nullptr : lineStyle.textShadow());
     style.alpha = 1;
     return style;
 }
@@ -759,9 +757,7 @@ auto InlineTextBox::resolveStyleForMarkedText(const MarkedText& markedText, cons
         style.alpha = 0.25;
         break;
     case MarkedText::Selection: {
-        const ShadowData* selectionShadow = nullptr;
-        style.textStyles = computeTextSelectionPaintStyle(style.textStyles, renderer(), lineStyle(), paintInfo, selectionShadow);
-        style.textShadow = selectionShadow;
+        style.textStyles = computeTextSelectionPaintStyle(style.textStyles, renderer(), lineStyle(), paintInfo, style.textShadow);
 
         Color selectionBackgroundColor = renderer().selectionBackgroundColor();
         style.backgroundColor = selectionBackgroundColor;
@@ -1016,7 +1012,8 @@ void InlineTextBox::paintMarkedTextForeground(GraphicsContext& context, const Fl
     textPainter.setFont(font);
     textPainter.setStyle(markedText.style.textStyles);
     textPainter.setIsHorizontal(isHorizontal());
-    textPainter.setShadow(markedText.style.textShadow);
+    if (markedText.style.textShadow)
+        textPainter.setShadow(&markedText.style.textShadow.value());
     textPainter.setEmphasisMark(emphasisMark, emphasisMarkOffset, combinedText());
 
     GraphicsContextStateSaver stateSaver { context, false };
@@ -1066,7 +1063,8 @@ void InlineTextBox::paintMarkedTextDecoration(GraphicsContext& context, const Fl
     decorationPainter.setWidth(snappedSelectionRect.width());
     decorationPainter.setBaseline(lineStyle().fontMetrics().ascent());
     decorationPainter.setIsHorizontal(isHorizontal());
-    decorationPainter.addTextShadow(markedText.style.textShadow);
+    if (markedText.style.textShadow)
+        decorationPainter.addTextShadow(&markedText.style.textShadow.value());
 
     {
         GraphicsContextStateSaver stateSaver { context, false };
index b06a8b2..06f9165 100644 (file)
@@ -132,10 +132,8 @@ TextPaintStyle computeTextPaintStyle(const Frame& frame, const RenderStyle& line
     return paintStyle;
 }
 
-TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle& textPaintStyle, const RenderText& renderer, const RenderStyle& lineStyle, const PaintInfo& paintInfo, const ShadowData*& selectionShadow)
+TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle& textPaintStyle, const RenderText& renderer, const RenderStyle& lineStyle, const PaintInfo& paintInfo, std::optional<ShadowData>& selectionShadow)
 {
-    selectionShadow = (paintInfo.forceTextColor()) ? nullptr : lineStyle.textShadow();
-
     TextPaintStyle selectionPaintStyle = textPaintStyle;
 
 #if ENABLE(TEXT_SELECTION)
@@ -148,10 +146,7 @@ TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle& textPaintSty
         selectionPaintStyle.emphasisMarkColor = emphasisMarkForeground;
 
     if (auto pseudoStyle = renderer.selectionPseudoStyle()) {
-        const ShadowData* shadow = paintInfo.forceTextColor() ? nullptr : pseudoStyle->textShadow();
-        if (shadow != selectionShadow)
-            selectionShadow = shadow;
-
+        selectionShadow = ShadowData::clone(paintInfo.forceTextColor() ? nullptr : pseudoStyle->textShadow());
         auto viewportSize = renderer.frame().view() ? renderer.frame().view()->size() : IntSize();
         float strokeWidth = pseudoStyle->computedStrokeWidth(viewportSize);
         if (strokeWidth != selectionPaintStyle.strokeWidth)
@@ -160,11 +155,13 @@ TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle& textPaintSty
         Color stroke = paintInfo.forceTextColor() ? paintInfo.forcedTextColor() : pseudoStyle->computedStrokeColor();
         if (stroke != selectionPaintStyle.strokeColor)
             selectionPaintStyle.strokeColor = stroke;
-    }
+    } else
+        selectionShadow = ShadowData::clone(paintInfo.forceTextColor() ? nullptr : lineStyle.textShadow());
 #else
     UNUSED_PARAM(renderer);
     UNUSED_PARAM(lineStyle);
     UNUSED_PARAM(paintInfo);
+    selectionShadow = ShadowData::clone(paintInfo.forceTextColor() ? nullptr : lineStyle.textShadow());
 #endif
     return selectionPaintStyle;
 }
index 0c02844..260c4d0 100644 (file)
@@ -60,7 +60,7 @@ struct TextPaintStyle {
 
 bool textColorIsLegibleAgainstBackgroundColor(const Color& textColor, const Color& backgroundColor);
 TextPaintStyle computeTextPaintStyle(const Frame&, const RenderStyle&, const PaintInfo&);
-TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle&, const RenderText&, const RenderStyle&, const PaintInfo&, const ShadowData*& selectionShadow);
+TextPaintStyle computeTextSelectionPaintStyle(const TextPaintStyle&, const RenderText&, const RenderStyle&, const PaintInfo&, std::optional<ShadowData>& selectionShadow);
 
 enum FillColorType { UseNormalFillColor, UseEmphasisMarkColor };
 void updateGraphicsContext(GraphicsContext&, const TextPaintStyle&, FillColorType = UseNormalFillColor);
index 4e5cb90..acad18e 100644 (file)
@@ -22,7 +22,6 @@
 #include "config.h"
 #include "ShadowData.h"
 
-#include "LayoutRect.h"
 #include <wtf/PointerComparison.h>
 
 namespace WebCore {
@@ -38,6 +37,13 @@ ShadowData::ShadowData(const ShadowData& o)
 {
 }
 
+std::optional<ShadowData> ShadowData::clone(const ShadowData* data)
+{
+    if (!data)
+        return std::nullopt;
+    return *data;
+}
+
 bool ShadowData::operator==(const ShadowData& o) const
 {
     if (!arePointingToEqualData(m_next, o.m_next))
index f063043..0177103 100644 (file)
@@ -55,7 +55,10 @@ public:
     {
     }
 
-    ShadowData(const ShadowData& o);
+    ShadowData(const ShadowData&);
+    static std::optional<ShadowData> clone(const ShadowData*);
+
+    ShadowData& operator=(ShadowData&&) = default;
 
     bool operator==(const ShadowData& o) const;
     bool operator!=(const ShadowData& o) const