Bug 1465307 - P2: Fix nsStyleBorder::mBorderColor for GCC. r?xidorn draft
authorDan Glastonbury <dan.glastonbury@gmail.com>
Tue, 05 Jun 2018 11:24:12 +1000
changeset 805076 9a18f55ba8097a9ebf7ff9b3a550ac48442c14d5
parent 805075 2cd2b26ddd25d3bbe408c300e43cd437386da909
child 805077 a07ff9e14104522f0c33ea5a8cd4f3632e1a3d02
push id112554
push userbmo:dglastonbury@mozilla.com
push dateThu, 07 Jun 2018 04:56:38 +0000
reviewersxidorn
bugs1465307
milestone62.0a1
Bug 1465307 - P2: Fix nsStyleBorder::mBorderColor for GCC. r?xidorn GCC doesn't like StyleComplexColor with constructor in an anonymous struct in an anonymous union. Replace the use of a union to access `mBorder[..]Color` fields as an array with an accessor methods. MozReview-Commit-ID: 1Wulh1qKYCZ
layout/generic/nsImageFrame.cpp
layout/painting/nsCSSRendering.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
servo/components/style/properties/gecko.mako.rs
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1255,17 +1255,17 @@ nsImageFrame::DisplayAltText(nsPresConte
   }
 }
 
 struct nsRecessedBorder : public nsStyleBorder {
   nsRecessedBorder(nscoord aBorderWidth, nsPresContext* aPresContext)
     : nsStyleBorder(aPresContext)
   {
     NS_FOR_CSS_SIDES(side) {
-      mBorderColor[side] = StyleComplexColor::FromColor(NS_RGB(0, 0, 0));
+      BorderColorFor(side) = StyleComplexColor::FromColor(NS_RGB(0, 0, 0));
       mBorder.Side(side) = aBorderWidth;
       // Note: use SetBorderStyle here because we want to affect
       // mComputedBorder
       SetBorderStyle(side, NS_STYLE_BORDER_STYLE_INSET);
     }
   }
 };
 
--- a/layout/painting/nsCSSRendering.cpp
+++ b/layout/painting/nsCSSRendering.cpp
@@ -650,17 +650,17 @@ nsCSSRendering::PaintBorder(nsPresContex
                                       aComputedStyle, aFlags, aSkipSides);
   }
 
   nsStyleBorder newStyleBorder(*styleBorder);
 
   NS_FOR_CSS_SIDES(side) {
     nscolor color = aComputedStyle->
       GetVisitedDependentColor(nsStyleBorder::BorderColorFieldFor(side));
-    newStyleBorder.mBorderColor[side] = StyleComplexColor::FromColor(color);
+    newStyleBorder.BorderColorFor(side) = StyleComplexColor::FromColor(color);
   }
   return PaintBorderWithStyleBorder(aPresContext, aRenderingContext, aForFrame,
                                     aDirtyRect, aBorderArea, newStyleBorder,
                                     aComputedStyle, aFlags, aSkipSides);
 }
 
 Maybe<nsCSSBorderRenderer>
 nsCSSRendering::CreateBorderRenderer(nsPresContext* aPresContext,
@@ -684,17 +684,17 @@ nsCSSRendering::CreateBorderRenderer(nsP
                                                aSkipSides);
   }
 
   nsStyleBorder newStyleBorder(*styleBorder);
 
   NS_FOR_CSS_SIDES(side) {
     nscolor color = aComputedStyle->
       GetVisitedDependentColor(nsStyleBorder::BorderColorFieldFor(side));
-    newStyleBorder.mBorderColor[side] = StyleComplexColor::FromColor(color);
+    newStyleBorder.BorderColorFor(side) = StyleComplexColor::FromColor(color);
   }
   return CreateBorderRendererWithStyleBorder(aPresContext, aDrawTarget,
                                              aForFrame, aDirtyRect, aBorderArea,
                                              newStyleBorder, aComputedStyle,
                                              aOutBorderIsEmpty, aSkipSides);
 }
 
 
@@ -853,17 +853,17 @@ ConstructBorderRenderer(nsPresContext* a
   Rect dirtyRect = NSRectToRect(aDirtyRect, oneDevPixel);
 
   uint8_t borderStyles[4];
   nscolor borderColors[4];
 
   // pull out styles, colors
   NS_FOR_CSS_SIDES (i) {
     borderStyles[i] = aStyleBorder.GetBorderStyle(i);
-    borderColors[i] = aStyleBorder.mBorderColor[i].CalcColor(aComputedStyle);
+    borderColors[i] = aStyleBorder.BorderColorFor(i).CalcColor(aComputedStyle);
   }
 
   PrintAsFormatString(" borderStyles: %d %d %d %d\n", borderStyles[0], borderStyles[1], borderStyles[2], borderStyles[3]);
 
   nsIDocument* document = nullptr;
   nsIContent* content = aForFrame->GetContent();
   if (content) {
     document = content->OwnerDoc();
@@ -2157,17 +2157,17 @@ IsOpaqueBorderEdge(const nsStyleBorder& 
 
   // If we're using a border image, assume it's not fully opaque,
   // because we may not even have the image loaded at this point, and
   // even if we did, checking whether the relevant tile is fully
   // opaque would be too much work.
   if (aBorder.mBorderImageSource.GetType() != eStyleImageType_Null)
     return false;
 
-  StyleComplexColor color = aBorder.mBorderColor[aSide];
+  StyleComplexColor color = aBorder.BorderColorFor(aSide);
   // We don't know the foreground color here, so if it's being used
   // we must assume it might be transparent.
   return !color.MaybeTransparent();
 }
 
 /**
  * Returns true if all border edges are either missing or opaque.
  */
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -5752,17 +5752,17 @@ nsComputedDOMStyle::GetBorderWidthFor(mo
 
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::GetBorderColorFor(mozilla::Side aSide)
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-  SetValueFromComplexColor(val, StyleBorder()->mBorderColor[aSide]);
+  SetValueFromComplexColor(val, StyleBorder()->BorderColorFor(aSide));
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::GetMarginWidthFor(mozilla::Side aSide)
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
 
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -301,16 +301,20 @@ nsStylePadding::CalcDifference(const nsS
 }
 
 nsStyleBorder::nsStyleBorder(const nsPresContext* aContext)
   : mBorderImageFill(NS_STYLE_BORDER_IMAGE_SLICE_NOFILL)
   , mBorderImageRepeatH(StyleBorderImageRepeat::Stretch)
   , mBorderImageRepeatV(StyleBorderImageRepeat::Stretch)
   , mFloatEdge(StyleFloatEdge::ContentBox)
   , mBoxDecorationBreak(StyleBoxDecorationBreak::Slice)
+  , mBorderTopColor(StyleComplexColor::CurrentColor())
+  , mBorderRightColor(StyleComplexColor::CurrentColor())
+  , mBorderBottomColor(StyleComplexColor::CurrentColor())
+  , mBorderLeftColor(StyleComplexColor::CurrentColor())
   , mComputedBorder(0, 0, 0, 0)
 {
   MOZ_COUNT_CTOR(nsStyleBorder);
 
   NS_FOR_CSS_HALF_CORNERS (corner) {
     mBorderRadius.Set(corner, nsStyleCoord(0, nsStyleCoord::CoordConstructor));
   }
 
@@ -318,41 +322,43 @@ nsStyleBorder::nsStyleBorder(const nsPre
     (StaticPresData::Get()->GetBorderWidthTable())[NS_STYLE_BORDER_WIDTH_MEDIUM];
   NS_FOR_CSS_SIDES(side) {
     mBorderImageSlice.Set(side, nsStyleCoord(1.0f, eStyleUnit_Percent));
     mBorderImageWidth.Set(side, nsStyleCoord(1.0f, eStyleUnit_Factor));
     mBorderImageOutset.Set(side, nsStyleCoord(0.0f, eStyleUnit_Factor));
 
     mBorder.Side(side) = medium;
     mBorderStyle[side] = NS_STYLE_BORDER_STYLE_NONE;
-    mBorderColor[side] = StyleComplexColor::CurrentColor();
   }
 
   mTwipsPerPixel = aContext->DevPixelsToAppUnits(1);
 }
 
 nsStyleBorder::nsStyleBorder(const nsStyleBorder& aSrc)
   : mBorderRadius(aSrc.mBorderRadius)
   , mBorderImageSource(aSrc.mBorderImageSource)
   , mBorderImageSlice(aSrc.mBorderImageSlice)
   , mBorderImageWidth(aSrc.mBorderImageWidth)
   , mBorderImageOutset(aSrc.mBorderImageOutset)
   , mBorderImageFill(aSrc.mBorderImageFill)
   , mBorderImageRepeatH(aSrc.mBorderImageRepeatH)
   , mBorderImageRepeatV(aSrc.mBorderImageRepeatV)
   , mFloatEdge(aSrc.mFloatEdge)
   , mBoxDecorationBreak(aSrc.mBoxDecorationBreak)
+  , mBorderTopColor(aSrc.mBorderTopColor)
+  , mBorderRightColor(aSrc.mBorderRightColor)
+  , mBorderBottomColor(aSrc.mBorderBottomColor)
+  , mBorderLeftColor(aSrc.mBorderLeftColor)
   , mComputedBorder(aSrc.mComputedBorder)
   , mBorder(aSrc.mBorder)
   , mTwipsPerPixel(aSrc.mTwipsPerPixel)
 {
   MOZ_COUNT_CTOR(nsStyleBorder);
   NS_FOR_CSS_SIDES(side) {
     mBorderStyle[side] = aSrc.mBorderStyle[side];
-    mBorderColor[side] = aSrc.mBorderColor[side];
   }
 }
 
 nsStyleBorder::~nsStyleBorder()
 {
   MOZ_COUNT_DTOR(nsStyleBorder);
 }
 
@@ -424,17 +430,17 @@ nsStyleBorder::CalcDifference(const nsSt
 
   // Note that mBorderStyle stores not only the border style but also
   // color-related flags.  Given that we've already done an mComputedBorder
   // comparison, border-style differences can only lead to a repaint hint.  So
   // it's OK to just compare the values directly -- if either the actual
   // style or the color flags differ we want to repaint.
   NS_FOR_CSS_SIDES(ix) {
     if (mBorderStyle[ix] != aNewData.mBorderStyle[ix] ||
-        mBorderColor[ix] != aNewData.mBorderColor[ix]) {
+        BorderColorFor(ix) != aNewData.BorderColorFor(ix)) {
       return nsChangeHint_RepaintFrame;
     }
   }
 
   if (mBorderRadius != aNewData.mBorderRadius) {
     return nsChangeHint_RepaintFrame;
   }
 
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1087,25 +1087,44 @@ public:
   mozilla::StyleBoxDecorationBreak mBoxDecorationBreak; // [reset]
 
 protected:
   uint8_t       mBorderStyle[4];  // [reset] See nsStyleConsts.h
 
 public:
   // [reset] the colors to use for a simple border.
   // not used for -moz-border-colors
-  union {
-    struct {
-      mozilla::StyleComplexColor mBorderTopColor;
-      mozilla::StyleComplexColor mBorderRightColor;
-      mozilla::StyleComplexColor mBorderBottomColor;
-      mozilla::StyleComplexColor mBorderLeftColor;
-    };
-    mozilla::StyleComplexColor mBorderColor[4];
-  };
+  mozilla::StyleComplexColor mBorderTopColor;
+  mozilla::StyleComplexColor mBorderRightColor;
+  mozilla::StyleComplexColor mBorderBottomColor;
+  mozilla::StyleComplexColor mBorderLeftColor;
+
+  mozilla::StyleComplexColor&
+  BorderColorFor(mozilla::Side aSide) {
+    switch (aSide) {
+    case mozilla::eSideTop:    return mBorderTopColor;
+    case mozilla::eSideRight:  return mBorderRightColor;
+    case mozilla::eSideBottom: return mBorderBottomColor;
+    case mozilla::eSideLeft:   return mBorderLeftColor;
+    }
+    MOZ_ASSERT_UNREACHABLE("Unknown side");
+    return mBorderTopColor;
+  }
+
+  const mozilla::StyleComplexColor&
+  BorderColorFor(mozilla::Side aSide) const {
+    switch (aSide) {
+    case mozilla::eSideTop:    return mBorderTopColor;
+    case mozilla::eSideRight:  return mBorderRightColor;
+    case mozilla::eSideBottom: return mBorderBottomColor;
+    case mozilla::eSideLeft:   return mBorderLeftColor;
+    }
+    MOZ_ASSERT_UNREACHABLE("Unknown side");
+    return mBorderTopColor;
+  }
 
   static mozilla::StyleComplexColor nsStyleBorder::*
   BorderColorFieldFor(mozilla::Side aSide) {
     switch (aSide) {
       case mozilla::eSideTop:
         return &nsStyleBorder::mBorderTopColor;
       case mozilla::eSideRight:
         return &nsStyleBorder::mBorderRightColor;
@@ -1141,30 +1160,16 @@ protected:
   nsMargin      mBorder;
 
 private:
   nscoord       mTwipsPerPixel;
 
   nsStyleBorder& operator=(const nsStyleBorder& aOther) = delete;
 };
 
-#define ASSERT_BORDER_COLOR_FIELD(side_)                          \
-  static_assert(offsetof(nsStyleBorder, mBorder##side_##Color) == \
-                  offsetof(nsStyleBorder, mBorderColor) +         \
-                    size_t(mozilla::eSide##side_) *               \
-                    sizeof(mozilla::StyleComplexColor),           \
-                "mBorder" #side_ "Color must be at same offset "  \
-                "as mBorderColor[mozilla::eSide" #side_ "]")
-ASSERT_BORDER_COLOR_FIELD(Top);
-ASSERT_BORDER_COLOR_FIELD(Right);
-ASSERT_BORDER_COLOR_FIELD(Bottom);
-ASSERT_BORDER_COLOR_FIELD(Left);
-#undef ASSERT_BORDER_COLOR_FIELD
-
-
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleOutline
 {
   explicit nsStyleOutline(const nsPresContext* aContext);
   nsStyleOutline(const nsStyleOutline& aOutline);
   ~nsStyleOutline() {
     MOZ_COUNT_DTOR(nsStyleOutline);
   }
   void FinishStyle(nsPresContext*, const nsStyleOutline*) {}
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -383,27 +383,19 @@ impl ${style_struct.gecko_struct_name} {
     #[allow(non_snake_case)]
     pub fn reset_${ident}(&mut self, other: &Self) {
         self.copy_${ident}_from(other)
     }
 </%def>
 
 <%!
 def get_gecko_property(ffi_name, self_param = "self"):
-    if "mBorderColor" in ffi_name:
-        return ffi_name.replace("mBorderColor",
-                                "unsafe { *%s.gecko.__bindgen_anon_1.mBorderColor.as_ref() }"
-                                % self_param)
     return "%s.gecko.%s" % (self_param, ffi_name)
 
 def set_gecko_property(ffi_name, expr):
-    if "mBorderColor" in ffi_name:
-        ffi_name = ffi_name.replace("mBorderColor",
-                                    "*self.gecko.__bindgen_anon_1.mBorderColor.as_mut()")
-        return "unsafe { %s = %s };" % (ffi_name, expr)
     return "self.gecko.%s = %s;" % (ffi_name, expr)
 %>
 
 <%def name="impl_keyword_setter(ident, gecko_ffi_name, keyword, cast_type='u8', on_set=None)">
     #[allow(non_snake_case)]
     pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
         use properties::longhands::${ident}::computed_value::T as Keyword;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
@@ -1589,17 +1581,17 @@ fn static_assert() {
     //
     // Once we're here, we know that we'll run style fixups, so it's fine to
     // just copy the specified border here, we'll adjust it if it's incorrect
     // later.
     fn update_border_${side.ident}(&mut self) {
         self.gecko.mComputedBorder.${side.ident} = self.gecko.mBorder.${side.ident};
     }
 
-    <% impl_color("border_%s_color" % side.ident, "(mBorderColor)[%s]" % side.index) %>
+    <% impl_color("border_%s_color" % side.ident, "mBorder%sColor" % side.name) %>
 
     <% impl_non_negative_length("border_%s_width" % side.ident,
                                 "mComputedBorder.%s" % side.ident,
                                 inherit_from="mBorder.%s" % side.ident,
                                 round_to_pixels=True) %>
 
     pub fn border_${side.ident}_has_nonzero_width(&self) -> bool {
         self.gecko.mComputedBorder.${side.ident} != 0