Bug 1266621 part 3 - Convert outline-color to store complex color. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 27 Sep 2016 20:44:19 +1000
changeset 418916 18ff94f359dde9cd174e2991b7dd848686058057
parent 418915 99766bfc2f7558935fa5adc869fa23844e3b909f
child 418917 9626d350649f4ba6dba597e3df05fe1829307610
push id30801
push userxquan@mozilla.com
push dateThu, 29 Sep 2016 09:22:37 +0000
reviewersheycam
bugs1266621
milestone52.0a1
Bug 1266621 part 3 - Convert outline-color to store complex color. r=heycam MozReview-Commit-ID: 70tre2pYqi3
layout/base/nsCSSRendering.cpp
layout/generic/nsFrame.cpp
layout/style/StyleAnimationValue.cpp
layout/style/nsCSSPropList.h
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleContext.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/style/test/test_transitions_events.html
layout/style/test/test_transitions_per_property.html
--- a/layout/base/nsCSSRendering.cpp
+++ b/layout/base/nsCSSRendering.cpp
@@ -871,17 +871,17 @@ nsCSSRendering::PaintOutline(nsPresConte
 {
   nscoord             twipsRadii[8];
 
   // Get our style context's color struct.
   const nsStyleOutline* ourOutline = aStyleContext->StyleOutline();
   MOZ_ASSERT(ourOutline != NS_STYLE_BORDER_STYLE_NONE,
              "shouldn't have created nsDisplayOutline item");
 
-  uint8_t outlineStyle = ourOutline->GetOutlineStyle();
+  uint8_t outlineStyle = ourOutline->mOutlineStyle;
   nscoord width = ourOutline->GetOutlineWidth();
 
   if (width == 0 && outlineStyle != NS_STYLE_BORDER_STYLE_AUTO) {
     // Empty outline
     return;
   }
 
   nsIFrame* bgFrame = nsCSSRendering::FindNonTransparentBackgroundFrame
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1794,18 +1794,19 @@ nsFrame::DisplaySelectionOverlay(nsDispl
   aList->AppendNewToTop(new (aBuilder)
     nsDisplaySelectionOverlay(aBuilder, this, selectionValue));
 }
 
 void
 nsFrame::DisplayOutlineUnconditional(nsDisplayListBuilder*   aBuilder,
                                      const nsDisplayListSet& aLists)
 {
-  if (StyleOutline()->GetOutlineStyle() == NS_STYLE_BORDER_STYLE_NONE)
+  if (StyleOutline()->mOutlineStyle == NS_STYLE_BORDER_STYLE_NONE) {
     return;
+  }
 
   aLists.Outlines()->AppendNewToTop(
     new (aBuilder) nsDisplayOutline(aBuilder, this));
 }
 
 void
 nsFrame::DisplayOutline(nsDisplayListBuilder*   aBuilder,
                         const nsDisplayListSet& aLists)
@@ -4842,17 +4843,17 @@ nsRect
 nsIFrame::ComputeTightBounds(DrawTarget* aDrawTarget) const
 {
   return GetVisualOverflowRect();
 }
 
 nsRect
 nsFrame::ComputeSimpleTightBounds(DrawTarget* aDrawTarget) const
 {
-  if (StyleOutline()->GetOutlineStyle() != NS_STYLE_BORDER_STYLE_NONE ||
+  if (StyleOutline()->mOutlineStyle != NS_STYLE_BORDER_STYLE_NONE ||
       StyleBorder()->HasBorder() || !StyleBackground()->IsTransparent() ||
       StyleDisplay()->mAppearance) {
     // Not necessarily tight, due to clipping, negative
     // outline-offset, and lots of other issues, but that's OK
     return GetVisualOverflowRect();
   }
 
   nsRect r(0, 0, 0, 0);
@@ -7965,17 +7966,17 @@ UnionBorderBoxes(nsIFrame* aFrame, bool 
   return u;
 }
 
 static void
 ComputeAndIncludeOutlineArea(nsIFrame* aFrame, nsOverflowAreas& aOverflowAreas,
                              const nsSize& aNewSize)
 {
   const nsStyleOutline* outline = aFrame->StyleOutline();
-  const uint8_t outlineStyle = outline->GetOutlineStyle();
+  const uint8_t outlineStyle = outline->mOutlineStyle;
   if (outlineStyle == NS_STYLE_BORDER_STYLE_NONE) {
     return;
   }
 
   nscoord width = outline->GetOutlineWidth();
   if (width <= 0 && outlineStyle != NS_STYLE_BORDER_STYLE_AUTO) {
     return;
   }
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -3847,26 +3847,16 @@ StyleAnimationValue::ExtractComputedValu
           ExtractBorderColor(aStyleContext, styleStruct, NS_SIDE_RIGHT,
                              aComputedValue);
           break;
         case eCSSProperty_border_top_color:
           ExtractBorderColor(aStyleContext, styleStruct, NS_SIDE_TOP,
                              aComputedValue);
           break;
 
-        case eCSSProperty_outline_color: {
-          const nsStyleOutline *styleOutline =
-            static_cast<const nsStyleOutline*>(styleStruct);
-          nscolor color;
-          if (!styleOutline->GetOutlineColor(color))
-            color = aStyleContext->StyleColor()->mColor;
-          aComputedValue.SetColorValue(color);
-          break;
-        }
-
         case eCSSProperty_column_count: {
           const nsStyleColumn *styleColumn =
             static_cast<const nsStyleColumn*>(styleStruct);
           if (styleColumn->mColumnCount == NS_STYLE_COLUMN_COUNT_AUTO) {
             aComputedValue.SetAutoValue();
           } else {
             aComputedValue.SetIntValue(styleColumn->mColumnCount,
                                        eUnit_Integer);
--- a/layout/style/nsCSSPropList.h
+++ b/layout/style/nsCSSPropList.h
@@ -3121,18 +3121,18 @@ CSS_PROP_OUTLINE(
     outline-color,
     outline_color,
     OutlineColor,
     CSS_PROPERTY_PARSE_VALUE |
         CSS_PROPERTY_IGNORED_WHEN_COLORS_DISABLED,
     "",
     VARIANT_HCK,
     kOutlineColorKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_Custom)
+    offsetof(nsStyleOutline, mOutlineColor),
+    eStyleAnimType_ComplexColor)
 CSS_PROP_OUTLINE(
     outline-offset,
     outline_offset,
     OutlineOffset,
     CSS_PROPERTY_PARSE_VALUE,
     "",
     VARIANT_HL | VARIANT_CALC,
     nullptr,
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -3290,33 +3290,33 @@ nsComputedDOMStyle::DoGetScrollSnapCoord
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOutlineWidth()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
 
   const nsStyleOutline* outline = StyleOutline();
 
   nscoord width;
-  if (outline->GetOutlineStyle() == NS_STYLE_BORDER_STYLE_NONE) {
+  if (outline->mOutlineStyle == NS_STYLE_BORDER_STYLE_NONE) {
     NS_ASSERTION(outline->GetOutlineWidth() == 0, "unexpected width");
     width = 0;
   } else {
     width = outline->GetOutlineWidth();
   }
   val->SetAppUnits(width);
 
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOutlineStyle()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
   val->SetIdent(
-    nsCSSProps::ValueToKeywordEnum(StyleOutline()->GetOutlineStyle(),
+    nsCSSProps::ValueToKeywordEnum(StyleOutline()->mOutlineStyle,
                                    nsCSSProps::kOutlineStyleKTable));
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOutlineOffset()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
@@ -3351,22 +3351,17 @@ nsComputedDOMStyle::DoGetOutlineRadiusTo
   return GetEllipseRadii(StyleOutline()->mOutlineRadius,
                          NS_CORNER_TOP_RIGHT, false);
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOutlineColor()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-
-  nscolor color;
-  if (!StyleOutline()->GetOutlineColor(color))
-    color = StyleColor()->mColor;
-
-  SetToRGBAColor(val, color);
+  SetValueFromComplexColor(val, StyleOutline()->mOutlineColor);
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::GetEllipseRadii(const nsStyleCorners& aRadius,
                                     uint8_t aFullCorner,
                                     bool aIsBorder) // else outline
 {
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -7823,44 +7823,21 @@ nsRuleNode::ComputeOutlineData(void* aSt
                aContext, mPresContext, conditions)) {
     outline->mOutlineOffset = tempCoord.GetCoordValue();
   } else {
     NS_ASSERTION(outlineOffsetValue->GetUnit() == eCSSUnit_Null,
                  "unexpected unit");
   }
 
   // outline-color: color, string, enum, inherit
-  nscolor outlineColor;
-  nscolor unused = NS_RGB(0,0,0);
-  const nsCSSValue* outlineColorValue = aRuleData->ValueForOutlineColor();
-  if (eCSSUnit_Inherit == outlineColorValue->GetUnit()) {
-    conditions.SetUncacheable();
-    if (parentContext) {
-      if (parentOutline->GetOutlineColor(outlineColor))
-        outline->SetOutlineColor(outlineColor);
-      else {
-        // We want to inherit the color from the parent, not use the
-        // color on the element where this chunk of style data will be
-        // used.  We can ensure that the data for the parent are fully
-        // computed (unlike for the element where this will be used, for
-        // which the color could be specified on a more specific rule).
-        outline->SetOutlineColor(parentContext->StyleColor()->mColor);
-      }
-    } else {
-      outline->SetOutlineInitialColor();
-    }
-  }
-  else if (SetColor(*outlineColorValue, unused, mPresContext,
-                    aContext, outlineColor, conditions))
-    outline->SetOutlineColor(outlineColor);
-  else if (eCSSUnit_Enumerated == outlineColorValue->GetUnit() ||
-           eCSSUnit_Initial == outlineColorValue->GetUnit() ||
-           eCSSUnit_Unset == outlineColorValue->GetUnit()) {
-    outline->SetOutlineInitialColor();
-  }
+  SetComplexColor<eUnsetInitial>(*aRuleData->ValueForOutlineColor(),
+                                 parentOutline->mOutlineColor,
+                                 StyleComplexColor::CurrentColor(),
+                                 mPresContext,
+                                 outline->mOutlineColor, conditions);
 
   // -moz-outline-radius: length, percent, inherit
   {
     const nsCSSPropertyID* subprops =
       nsCSSProps::SubpropertyEntryFor(eCSSProperty__moz_outline_radius);
     NS_FOR_CSS_FULL_CORNERS(corner) {
       int cx = NS_FULL_TO_HALF_CORNER(corner, false);
       int cy = NS_FULL_TO_HALF_CORNER(corner, true);
@@ -7881,23 +7858,23 @@ nsRuleNode::ComputeOutlineData(void* aSt
 
   // outline-style: enum, inherit, initial
   // cannot use SetValue because of SetOutlineStyle
   const nsCSSValue* outlineStyleValue = aRuleData->ValueForOutlineStyle();
   nsCSSUnit unit = outlineStyleValue->GetUnit();
   MOZ_ASSERT(eCSSUnit_None != unit && eCSSUnit_Auto != unit,
              "'none' and 'auto' should be handled as enumerated values");
   if (eCSSUnit_Enumerated == unit) {
-    outline->SetOutlineStyle(outlineStyleValue->GetIntValue());
+    outline->mOutlineStyle = outlineStyleValue->GetIntValue();
   } else if (eCSSUnit_Initial == unit ||
              eCSSUnit_Unset == unit) {
-    outline->SetOutlineStyle(NS_STYLE_BORDER_STYLE_NONE);
+    outline->mOutlineStyle = NS_STYLE_BORDER_STYLE_NONE;
   } else if (eCSSUnit_Inherit == unit) {
     conditions.SetUncacheable();
-    outline->SetOutlineStyle(parentOutline->GetOutlineStyle());
+    outline->mOutlineStyle = parentOutline->mOutlineStyle;
   }
 
   outline->RecalcData();
   COMPUTE_END_RESET(Outline, outline)
 }
 
 const void*
 nsRuleNode::ComputeListData(void* aStartStruct,
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -1188,26 +1188,17 @@ nsStyleContext::CalcStyleDifferenceInter
         }
       }
     }
 
     // NB: Calling Peek on |this|, not |thisVis| (see above).
     if (!change && PeekStyleOutline()) {
       const nsStyleOutline *thisVisOutline = thisVis->StyleOutline();
       const nsStyleOutline *otherVisOutline = otherVis->StyleOutline();
-      bool haveColor;
-      // Dummy initialisations to keep Valgrind/Memcheck happy.
-      // See bug 1289098 comment 1.
-      nscolor thisColor = NS_RGBA(0, 0, 0, 0);
-      nscolor otherColor = NS_RGBA(0, 0, 0, 0);
-      if (thisVisOutline->GetOutlineInitialColor() !=
-            otherVisOutline->GetOutlineInitialColor() ||
-          (haveColor = thisVisOutline->GetOutlineColor(thisColor)) !=
-            otherVisOutline->GetOutlineColor(otherColor) ||
-          (haveColor && thisColor != otherColor)) {
+      if (thisVisOutline->mOutlineColor != otherVisOutline->mOutlineColor) {
         change = true;
       }
     }
 
     // NB: Calling Peek on |this|, not |thisVis| (see above).
     if (!change && PeekStyleColumn()) {
       const nsStyleColumn *thisVisColumn = thisVis->StyleColumn();
       const nsStyleColumn *otherVisColumn = otherVis->StyleColumn();
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -640,47 +640,45 @@ nsStyleBorder::CalcDifference(const nsSt
   }
 
   return nsChangeHint(0);
 }
 
 nsStyleOutline::nsStyleOutline(StyleStructContext aContext)
   : mOutlineWidth(NS_STYLE_BORDER_WIDTH_MEDIUM, eStyleUnit_Enumerated)
   , mOutlineOffset(0)
+  , mOutlineColor(StyleComplexColor::CurrentColor())
+  , mOutlineStyle(NS_STYLE_BORDER_STYLE_NONE)
   , mActualOutlineWidth(0)
-  , mOutlineColor(NS_RGB(0, 0, 0))
-  , mOutlineStyle(NS_STYLE_BORDER_STYLE_NONE)
   , mTwipsPerPixel(aContext.DevPixelsToAppUnits(1))
 {
   MOZ_COUNT_CTOR(nsStyleOutline);
   // spacing values not inherited
   nsStyleCoord zero(0, nsStyleCoord::CoordConstructor);
   NS_FOR_CSS_HALF_CORNERS(corner) {
     mOutlineRadius.Set(corner, zero);
   }
-
-  SetOutlineInitialColor();
 }
 
 nsStyleOutline::nsStyleOutline(const nsStyleOutline& aSrc)
   : mOutlineRadius(aSrc.mOutlineRadius)
   , mOutlineWidth(aSrc.mOutlineWidth)
   , mOutlineOffset(aSrc.mOutlineOffset)
-  , mActualOutlineWidth(aSrc.mActualOutlineWidth)
   , mOutlineColor(aSrc.mOutlineColor)
   , mOutlineStyle(aSrc.mOutlineStyle)
+  , mActualOutlineWidth(aSrc.mActualOutlineWidth)
   , mTwipsPerPixel(aSrc.mTwipsPerPixel)
 {
   MOZ_COUNT_CTOR(nsStyleOutline);
 }
 
 void
 nsStyleOutline::RecalcData()
 {
-  if (NS_STYLE_BORDER_STYLE_NONE == GetOutlineStyle()) {
+  if (NS_STYLE_BORDER_STYLE_NONE == mOutlineStyle) {
     mActualOutlineWidth = 0;
   } else {
     MOZ_ASSERT(mOutlineWidth.ConvertsToLength() ||
                mOutlineWidth.GetUnit() == eStyleUnit_Enumerated);
     // Clamp negative calc() to 0.
     mActualOutlineWidth =
       std::max(CalcCoord(mOutlineWidth,
                          StaticPresData::Get()->GetBorderWidthTable(), 3), 0);
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -877,22 +877,18 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   inline bool HasLocalBackground() const;
 
   const nsStyleImageLayers::Layer& BottomLayer() const { return mImage.BottomLayer(); }
 
   nsStyleImageLayers mImage;
   nscolor mBackgroundColor;       // [reset]
 };
 
-// See https://bugzilla.mozilla.org/show_bug.cgi?id=271586#c43 for why
-// this is hard to replace with 'currentColor'.
 #define BORDER_COLOR_FOREGROUND   0x20
-#define OUTLINE_COLOR_INITIAL     0x80
-// FOREGROUND | INITIAL(OUTLINE)
-#define BORDER_COLOR_SPECIAL      0xA0
+#define BORDER_COLOR_SPECIAL      BORDER_COLOR_FOREGROUND
 #define BORDER_STYLE_MASK         0x1F
 
 #define NS_SPACING_MARGIN   0
 #define NS_SPACING_PADDING  1
 #define NS_SPACING_BORDER   2
 
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleMargin
@@ -1419,69 +1415,29 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
 
   // This is the specified value of outline-width, but with length values
   // computed to absolute.  mActualOutlineWidth stores the outline-width
   // value used by layout.  (We must store mOutlineWidth for the same
   // style struct resolution reasons that we do nsStyleBorder::mBorder;
   // see that field's comment.)
   nsStyleCoord  mOutlineWidth;    // [reset] coord, enum (see nsStyleConsts.h)
   nscoord       mOutlineOffset;   // [reset]
+  mozilla::StyleComplexColor mOutlineColor; // [reset]
+  uint8_t       mOutlineStyle;    // [reset] See nsStyleConsts.h
 
   nscoord GetOutlineWidth() const
   {
     return mActualOutlineWidth;
   }
 
-  uint8_t GetOutlineStyle() const
-  {
-    return (mOutlineStyle & BORDER_STYLE_MASK);
-  }
-
-  void SetOutlineStyle(uint8_t aStyle)
-  {
-    mOutlineStyle &= ~BORDER_STYLE_MASK;
-    mOutlineStyle |= (aStyle & BORDER_STYLE_MASK);
-  }
-
-  // false means initial value
-  bool GetOutlineColor(nscolor& aColor) const
-  {
-    if ((mOutlineStyle & BORDER_COLOR_SPECIAL) == 0) {
-      aColor = mOutlineColor;
-      return true;
-    }
-    return false;
-  }
-
-  void SetOutlineColor(nscolor aColor)
-  {
-    mOutlineColor = aColor;
-    mOutlineStyle &= ~BORDER_COLOR_SPECIAL;
-  }
-
-  void SetOutlineInitialColor()
-  {
-    mOutlineStyle |= OUTLINE_COLOR_INITIAL;
-  }
-
-  bool GetOutlineInitialColor() const
-  {
-    return !!(mOutlineStyle & OUTLINE_COLOR_INITIAL);
-  }
-
 protected:
   // The actual value of outline-width is the computed value (an absolute
   // length, forced to zero when outline-style is none) rounded to device
   // pixels.  This is the value used by layout.
   nscoord       mActualOutlineWidth;
-
-  nscolor       mOutlineColor;    // [reset]
-
-  uint8_t       mOutlineStyle;    // [reset] See nsStyleConsts.h
-
   nscoord       mTwipsPerPixel;
 };
 
 
 /**
  * An object that allows sharing of arrays that store 'quotes' property
  * values.  This is particularly important for inheritance, where we want
  * to share the same 'quotes' value with a parent style context.
--- a/layout/style/test/test_transitions_events.html
+++ b/layout/style/test/test_transitions_events.html
@@ -66,17 +66,16 @@ function $(id) { return document.getElem
 function cs(id) { return getComputedStyle($(id), ""); }
 
 var got_one_root = false;
 var got_one_target = false;
 var got_one_target_bordertop = false;
 var got_one_target_borderright = false;
 var got_one_target_borderbottom = false;
 var got_one_target_borderleft = false;
-var got_one_target_outlinecolor = false;
 var got_two_target = false;
 var got_three_top = false;
 var got_three_right = false;
 var got_three_bottom = false;
 var got_three_left = false;
 var got_four_root = false;
 var got_body = false;
 var did_stops = false;
@@ -171,22 +170,16 @@ document.documentElement.addEventListene
         event.stopPropagation();
         break;
       case "border-left-color":
         ok(!got_one_target_borderleft,
            "transitionend on one on target (border-left-color)");
         got_one_target_borderleft = true;
         event.stopPropagation();
         break;
-      case "outline-color":
-        ok(!got_one_target_outlinecolor,
-           "transitionend on one on target (outline-color)");
-        got_one_target_outlinecolor = true;
-        event.stopPropagation();
-        break;
       default:
         ok(false, "unexpected property name " + event.propertyName +
                   " for transitionend on one on target");
     }
     is(event.elapsedTime, 0.5,
        "elapsedTime for transitionend on one");
     is(cs("one").getPropertyValue(event.propertyName), "rgb(0, 255, 0)",
        "computed style of " + event.propertyName + " for transitionend on one");
@@ -194,17 +187,16 @@ document.documentElement.addEventListene
   }, false);
 
 started_test(); // color on #one
 started_test(); // border-top-color on #one
 started_test(); // border-right-color on #one
 started_test(); // border-right-color on #one (listener on root)
 started_test(); // border-bottom-color on #one
 started_test(); // border-left-color on #one
-started_test(); // outline-color on #one
 $("one").style.color = "lime";
 
 
 $("two").addEventListener("transitionend",
   function(event) {
     event.stopPropagation();
 
     ok(!got_two_target, "transitionend on two on target");
--- a/layout/style/test/test_transitions_per_property.html
+++ b/layout/style/test/test_transitions_per_property.html
@@ -208,17 +208,17 @@ var supported_properties = {
                    test_length_clamped, test_percent_clamped ],
     "object-position": [ test_background_position_transition ],
     "opacity" : [ test_float_zeroToOne_transition,
                   // opacity is clamped in computed style
                   // (not parsing/interpolation)
                   test_float_zeroToOne_clamped ],
     "order": [ test_integer_transition ],
     "outline-color": [ test_color_transition,
-                       test_currentcolor_transition ],
+                       test_true_currentcolor_transition ],
     "outline-offset": [ test_length_transition, test_length_unclamped ],
     "outline-width": [ test_length_transition, test_length_clamped ],
     "padding-bottom": [ test_length_transition, test_percent_transition,
                         test_length_percent_calc_transition,
                         test_length_clamped, test_percent_clamped ],
     "padding-left": [ test_length_transition, test_percent_transition,
                       test_length_percent_calc_transition,
                       test_length_clamped, test_percent_clamped ],