Add support for multiple text decorations with same line positioning

third_party/WebKit/Source/core/style/ComputedStyle.cpp

 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 724 matching lines @@
         if (rareNonInheritedData->m_filter != other.rareNonInheritedData->m_filter)
             diff.setFilterChanged();
 
         if (rareNonInheritedData->m_backdropFilter != other.rareNonInheritedData->m_backdropFilter)
             diff.setBackdropFilterChanged();
     }
 
     if (!diff.needsPaintInvalidation()) {
         if (inherited->color != other.inherited->color
             || inherited->visitedLinkColor != other.inherited->visitedLinkColor
-            || inherited_flags.m_textUnderline != other.inherited_flags.m_textUnderline
+            || inherited_flags.m_hasSimpleUnderline != other.inherited_flags.m_hasSimpleUnderline
             || visual->textDecoration != other.visual->textDecoration) {
             diff.setTextDecorationOrColorChanged();
         } else if (rareNonInheritedData.get() != other.rareNonInheritedData.get()
             && (rareNonInheritedData->m_textDecorationStyle != other.rareNonInheritedData->m_textDecorationStyle
                 || rareNonInheritedData->m_textDecorationColor != other.rareNonInheritedData->m_textDecorationColor
                 || rareNonInheritedData->m_visitedLinkTextDecorationColor != other.rareNonInheritedData->m_visitedLinkTextDecorationColor)) {
                 diff.setTextDecorationOrColorChanged();
         } else if (rareInheritedData.get() != other.rareInheritedData.get()
             && (rareInheritedData->textFillColor() != other.rareInheritedData->textFillColor()
                 || rareInheritedData->textStrokeColor() != other.rareInheritedData->textStrokeColor()
@@ skipping 482 matching lines @@
 float ComputedStyle::specifiedFontSize() const { return fontDescription().specifiedSize(); }
 float ComputedStyle::computedFontSize() const { return fontDescription().computedSize(); }
 int ComputedStyle::fontSize() const { return fontDescription().computedPixelSize(); }
 float ComputedStyle::fontSizeAdjust() const { return fontDescription().sizeAdjust(); }
 bool ComputedStyle::hasFontSizeAdjust() const { return fontDescription().hasSizeAdjust(); }
 FontWeight ComputedStyle::fontWeight() const { return fontDescription().weight(); }
 FontStretch ComputedStyle::fontStretch() const { return fontDescription().stretch(); }
 
 TextDecoration ComputedStyle::textDecorationsInEffect() const
 {
+    if (!inherited_flags.m_hasSimpleUnderline && !rareInheritedData->appliedTextDecorations)

[Timothy Loh, 2015/10/12 00:11:16]

can we move this if statement below the other one so we only have a single
check?

[sashab, 2015/10/12 23:11:01]

Yeahhh.. It's a bit weird because applideTextDecorations() below uses a similar
pattern, and I feel they should match. OK, changed both.

+        return TextDecorationNone;
+    if (inherited_flags.m_hasSimpleUnderline)
+        return TextDecorationUnderline;
+
     int decorations = 0;
 
     const Vector<AppliedTextDecoration>& applied = appliedTextDecorations();
 
     for (size_t i = 0; i < applied.size(); ++i)
-        decorations |= applied[i].line();
+        decorations |= applied[i].lines();
 
     return static_cast<TextDecoration>(decorations);
 }
 
 const Vector<AppliedTextDecoration>& ComputedStyle::appliedTextDecorations() const
 {
-    if (!inherited_flags.m_textUnderline && !rareInheritedData->appliedTextDecorations) {
+    if (!inherited_flags.m_hasSimpleUnderline && !rareInheritedData->appliedTextDecorations) {
         DEFINE_STATIC_LOCAL(Vector<AppliedTextDecoration>, empty, ());
         return empty;
     }
-    if (inherited_flags.m_textUnderline) {
-        DEFINE_STATIC_LOCAL(Vector<AppliedTextDecoration>, underline, (1, AppliedTextDecoration(TextDecorationUnderline)));
+    if (inherited_flags.m_hasSimpleUnderline) {
+        DEFINE_STATIC_LOCAL(Vector<AppliedTextDecoration>, underline, (1, AppliedTextDecoration(TextDecorationUnderline, TextDecorationStyleSolid, visitedDependentColor(CSSPropertyTextDecorationColor))));
+        // Since we only have one of these in memory, just update the color before returning.
+        underline.at(0).setColor(visitedDependentColor(CSSPropertyTextDecorationColor));
         return underline;
     }
 
     return rareInheritedData->appliedTextDecorations->vector();
 }
 
 float ComputedStyle::wordSpacing() const { return fontDescription().wordSpacing(); }
 float ComputedStyle::letterSpacing() const { return fontDescription().letterSpacing(); }
 
 bool ComputedStyle::setFontDescription(const FontDescription& v)
@@ skipping 87 matching lines @@
 
 void ComputedStyle::addAppliedTextDecoration(const AppliedTextDecoration& decoration)
 {
     RefPtr<AppliedTextDecorationList>& list = rareInheritedData.access()->appliedTextDecorations;
 
     if (!list)
         list = AppliedTextDecorationList::create();
     else if (!list->hasOneRef())
         list = list->copy();
 
-    if (inherited_flags.m_textUnderline) {
-        inherited_flags.m_textUnderline = false;
-        list->append(AppliedTextDecoration(TextDecorationUnderline));
-    }
-
     list->append(decoration);
 }
 
-void ComputedStyle::applyTextDecorations()
+void ComputedStyle::updateAppliedTextDecorations(Color propagatedColor)

[Timothy Loh, 2015/10/12 00:11:16]

"update" is really vague. How about overrideTextDecorationColors?

[sashab, 2015/10/12 23:11:01]

Hey, good one. Yup, this name is an artifact from an older patch. Way better
now.

 {
+    RefPtr<AppliedTextDecorationList>& list = rareInheritedData.access()->appliedTextDecorations;
+
+    ASSERT(list);
+    if (!list->hasOneRef())
+        list = list->copy();
+
+    for (size_t i = 0; i < rareInheritedData->appliedTextDecorations->size(); ++i)
+        list->at(i).setColor(propagatedColor);
+}
+
+void ComputedStyle::applyTextDecorations(const Color& parentTextDecorationColor, bool overrideExistingColors)
+{
+    // If we have no decorations, return early.

[Timothy Loh, 2015/10/12 00:11:16]

This comment doesn't add value.

[sashab, 2015/10/12 23:11:02]

Removed!

+    if (textDecoration() == TextDecorationNone && !inherited_flags.m_hasSimpleUnderline && !rareInheritedData->appliedTextDecorations)
+        return;
+
+    // If there are any color changes or decorations set by this element, stop using m_hasSimpleUnderline.
+    Color currentTextDecorationColor = visitedDependentColor(CSSPropertyTextDecorationColor);
+    if (inherited_flags.m_hasSimpleUnderline && (textDecoration() != TextDecorationNone || currentTextDecorationColor != parentTextDecorationColor)) {
+        inherited_flags.m_hasSimpleUnderline = false;
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationUnderline, TextDecorationStyleSolid, parentTextDecorationColor));
+    }
+
+    // If we have a propagated color, update any other affected decorations.

[Timothy Loh, 2015/10/12 00:11:16]

Don't think this comment helps (or makes sense)

[sashab, 2015/10/12 23:11:01]

Yup, 'propagated' doesn't belong here. Removed!

+    if (overrideExistingColors && rareInheritedData->appliedTextDecorations)
+        updateAppliedTextDecorations(currentTextDecorationColor);
+
     if (textDecoration() == TextDecorationNone)
         return;
+    ASSERT(!inherited_flags.m_hasSimpleUnderline);
 
-    TextDecorationStyle style = textDecorationStyle();
-    StyleColor styleColor = decorationColorIncludingFallback(insideLink() == InsideVisitedLink);
+    // To save memory, we don't use AppliedTextDecoration objects in the common case of a single simple underline.
+    TextDecoration decorationLines = textDecoration();
+    TextDecorationStyle decorationStyle = textDecorationStyle();
+    StyleColor decorationColor = decorationColorIncludingFallback(insideLink() == InsideVisitedLink);
+    bool isSimpleUnderline = decorationLines == TextDecorationUnderline && decorationStyle == TextDecorationStyleSolid && decorationColor.isCurrentColor();
+    if (!rareInheritedData->appliedTextDecorations && !inherited_flags.m_hasSimpleUnderline && isSimpleUnderline) {
+        inherited_flags.m_hasSimpleUnderline = true;
+        return;
+    }
 
-    int decorations = textDecoration();
-
-    if (decorations & TextDecorationUnderline) {
-        // To save memory, we don't use AppliedTextDecoration objects in the
-        // common case of a single simple underline.
-        AppliedTextDecoration underline(TextDecorationUnderline, style, styleColor);
-
-        if (!rareInheritedData->appliedTextDecorations && underline.isSimpleUnderline())
-            inherited_flags.m_textUnderline = true;
-        else
-            addAppliedTextDecoration(underline);
-    }
-    if (decorations & TextDecorationOverline)
-        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationOverline, style, styleColor));
-    if (decorations & TextDecorationLineThrough)
-        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationLineThrough, style, styleColor));
+    Color resolvedDecorationColor = decorationColor.resolve(currentTextDecorationColor);

[Timothy Loh, 2015/10/12 00:11:16]

isn't this exactly the same as currentTextDecorationColor?

[sashab, 2015/10/12 23:11:02]

Oh yeah, you're right :) Done.

+    addAppliedTextDecoration(AppliedTextDecoration(decorationLines, decorationStyle, resolvedDecorationColor));
 }
 
 void ComputedStyle::clearAppliedTextDecorations()
 {
-    inherited_flags.m_textUnderline = false;
+    inherited_flags.m_hasSimpleUnderline = false;
 
     if (rareInheritedData->appliedTextDecorations)
         rareInheritedData.access()->appliedTextDecorations = nullptr;
 }
 
 void ComputedStyle::clearMultiCol()
 {
     rareNonInheritedData.access()->m_multiCol = nullptr;
     rareNonInheritedData.access()->m_multiCol.init();
 }
@@ skipping 378 matching lines @@
 }
 
 void ComputedStyle::copyChildDependentFlagsFrom(const ComputedStyle& other)
 {
     setEmptyState(other.emptyState());
     if (other.hasExplicitlyInheritedProperties())
         setHasExplicitlyInheritedProperties();
 }
 
 } // namespace blink