Bug 1476054: Fixes and cleanups for Servo PR #21156. r=me
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 16 Jul 2018 19:01:24 +0200
changeset 426927 eb66264387a0eaddbc5ff8e07aea2e7d667daafa
parent 426926 fb59c8181fc969659ce9727af15d36cd86ac7ccb
child 426928 cf7d50bca2644f402e675b486e6834c4c7d1edc8
push id34289
push usertoros@mozilla.com
push dateTue, 17 Jul 2018 21:56:05 +0000
treeherdermozilla-central@afa310dc89be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1476054, 21156
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1476054: Fixes and cleanups for Servo PR #21156. r=me Similar to the previous patch, logical clear doesn't appear in computed style objects. MozReview-Commit-ID: FbN0hiUGzYa
layout/generic/BRFrame.cpp
layout/generic/BlockReflowInput.cpp
layout/generic/WritingModes.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/generic/nsLineBox.cpp
layout/style/ServoCSSPropList.mako.py
layout/style/nsStyleConsts.h
layout/style/nsStyleStruct.h
servo/components/style/properties/gecko.mako.rs
servo/ports/geckolib/glue.rs
--- a/layout/generic/BRFrame.cpp
+++ b/layout/generic/BRFrame.cpp
@@ -153,17 +153,17 @@ BRFrame::Reflow(nsPresContext* aPresCont
       // XXX This also fixes bug 10036!
       // Warning: nsTextControlFrame::CalculateSizeStandard depends on
       // the following line, see bug 228752.
       // The code below in AddInlinePrefISize also adds 1 appunit to width
       finalSize.ISize(wm) = 1;
     }
 
     // Return our reflow status
-    StyleClear breakType = aReflowInput.mStyleDisplay->PhysicalBreakType(wm);
+    StyleClear breakType = aReflowInput.mStyleDisplay->mBreakType;
     if (StyleClear::None == breakType) {
       breakType = StyleClear::Line;
     }
 
     aStatus.SetInlineLineBreakAfter(breakType);
     ll->SetLineEndsInBR(true);
   }
 
--- a/layout/generic/BlockReflowInput.cpp
+++ b/layout/generic/BlockReflowInput.cpp
@@ -737,17 +737,17 @@ BlockReflowInput::FlowAndPlaceFloat(nsIF
   // ``above'' another float that preceded it in the flow.
   mBCoord = std::max(FloatManager()->GetLowestFloatTop(), mBCoord);
 
   // See if the float should clear any preceding floats...
   // XXX We need to mark this float somehow so that it gets reflowed
   // when floats are inserted before it.
   if (StyleClear::None != floatDisplay->mBreakType) {
     // XXXldb Does this handle vertical margins correctly?
-    mBCoord = ClearFloats(mBCoord, floatDisplay->PhysicalBreakType(wm));
+    mBCoord = ClearFloats(mBCoord, floatDisplay->mBreakType);
   }
   // Get the band of available space with respect to margin box.
   nsFlowAreaRect floatAvailableSpace =
     GetFloatAvailableSpaceForPlacingFloat(mBCoord);
   LogicalRect adjustedAvailableSpace =
     mBlock->AdjustFloatAvailableSpace(*this, floatAvailableSpace.mRect, aFloat);
 
   NS_ASSERTION(aFloat->GetParent() == mBlock,
--- a/layout/generic/WritingModes.h
+++ b/layout/generic/WritingModes.h
@@ -2182,29 +2182,16 @@ nsStylePosition::MinBSizeDependsOnContai
 }
 inline bool
 nsStylePosition::MaxBSizeDependsOnContainer(mozilla::WritingMode aWM) const
 {
   return aWM.IsVertical() ? MaxWidthDependsOnContainer()
                           : MaxHeightDependsOnContainer();
 }
 
-inline mozilla::StyleClear
-nsStyleDisplay::PhysicalBreakType(mozilla::WritingMode aWM) const
-{
-  using StyleClear = mozilla::StyleClear;
-  if (mBreakType == StyleClear::InlineStart) {
-    return aWM.IsBidiLTR() ? StyleClear::Left : StyleClear::Right;
-  }
-  if (mBreakType == StyleClear::InlineEnd) {
-    return aWM.IsBidiLTR() ? StyleClear::Right : StyleClear::Left;
-  }
-  return mBreakType;
-}
-
 inline bool
 nsStyleMargin::HasBlockAxisAuto(mozilla::WritingMode aWM) const
 {
   return mMargin.HasBlockAxisAuto(aWM);
 }
 inline bool
 nsStyleMargin::HasInlineAxisAuto(mozilla::WritingMode aWM) const
 {
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -837,18 +837,17 @@ nsBlockFrame::GetPrefISize(gfxContext *a
       }
       AutoNoisyIndenter lineindent(gNoisyIntrinsic);
 #endif
       if (line->IsBlock()) {
         StyleClear breakType;
         if (!data.mLineIsEmpty || BlockCanIntersectFloats(line->mFirstChild)) {
           breakType = StyleClear::Both;
         } else {
-          breakType = line->mFirstChild->
-            StyleDisplay()->PhysicalBreakType(data.mLineContainerWM);
+          breakType = line->mFirstChild->StyleDisplay()->mBreakType;
         }
         data.ForceBreak(breakType);
         data.mCurrentLine = nsLayoutUtils::IntrinsicForContainer(aRenderingContext,
                         line->mFirstChild, nsLayoutUtils::PREF_ISIZE);
         data.ForceBreak();
       } else {
         if (!curFrame->GetPrevContinuation() &&
             line == curFrame->LinesBegin()) {
@@ -3221,18 +3220,17 @@ nsBlockFrame::ReflowBlockFrame(BlockRefl
   if (!frame) {
     NS_ASSERTION(false, "program error - unexpected empty line");
     return;
   }
 
   // Prepare the block reflow engine
   nsBlockReflowContext brc(aState.mPresContext, aState.mReflowInput);
 
-  StyleClear breakType = frame->StyleDisplay()->
-    PhysicalBreakType(aState.mReflowInput.GetWritingMode());
+  StyleClear breakType = frame->StyleDisplay()->mBreakType;
   if (StyleClear::None != aState.mFloatBreakType) {
     breakType = nsLayoutUtils::CombineBreakType(breakType,
                                                 aState.mFloatBreakType);
     aState.mFloatBreakType = StyleClear::None;
   }
 
   // Clear past floats before the block if the clear style is not none
   aLine->SetBreakTypeBefore(breakType);
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5352,22 +5352,21 @@ nsIFrame::InlinePrefISizeData::ForceBrea
   if (mFloats.Length() != 0 && aBreakType != StyleClear::None) {
             // preferred widths accumulated for floats that have already
             // been cleared past
     nscoord floats_done = 0,
             // preferred widths accumulated for floats that have not yet
             // been cleared past
             floats_cur_left = 0,
             floats_cur_right = 0;
-    const WritingMode wm = mLineContainerWM;
 
     for (uint32_t i = 0, i_end = mFloats.Length(); i != i_end; ++i) {
       const FloatInfo& floatInfo = mFloats[i];
       const nsStyleDisplay* floatDisp = floatInfo.Frame()->StyleDisplay();
-      StyleClear breakType = floatDisp->PhysicalBreakType(wm);
+      StyleClear breakType = floatDisp->mBreakType;
       if (breakType == StyleClear::Left ||
           breakType == StyleClear::Right ||
           breakType == StyleClear::Both) {
         nscoord floats_cur = NSCoordSaturatingAdd(floats_cur_left,
                                                   floats_cur_right);
         if (floats_cur > floats_done) {
           floats_done = floats_cur;
         }
@@ -5419,17 +5418,17 @@ nsIFrame::InlinePrefISizeData::ForceBrea
         } else {
           // This is a float on the side that this break directly clears
           // which means we're not keeping it in mFloats. However, if
           // this float clears floats on the opposite side (via a value
           // of either 'both' or one of 'left'/'right'), any remaining
           // (earlier) floats on that side would be indirectly cleared
           // as well. Thus, we should break out of this loop and stop
           // considering earlier floats to be kept in mFloats.
-          StyleClear floatBreakType = floatDisp->PhysicalBreakType(wm);
+          StyleClear floatBreakType = floatDisp->mBreakType;
           if (floatBreakType != aBreakType &&
               floatBreakType != StyleClear::None) {
             break;
           }
         }
       }
       newFloats.Reverse();
       mFloats = std::move(newFloats);
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -2185,32 +2185,29 @@ public:
       , mSkipWhitespace(true)
     {}
 
     // The line. This may be null if the inlines are not associated with
     // a block or if we just don't know the line.
     const nsLineList_iterator* mLine;
 
     // The line container. Private, to ensure we always use SetLineContainer
-    // to update it (so that we have a chance to store the mLineContainerWM).
+    // to update it.
     //
     // Note that nsContainerFrame::DoInlineIntrinsicISize will clear the
     // |mLine| and |mLineContainer| fields when following a next-in-flow link,
     // so we must not assume these can always be dereferenced.
   private:
     nsIFrame* mLineContainer;
 
     // Setter and getter for the lineContainer field:
   public:
     void SetLineContainer(nsIFrame* aLineContainer)
     {
       mLineContainer = aLineContainer;
-      if (mLineContainer) {
-        mLineContainerWM = mLineContainer->GetWritingMode();
-      }
     }
     nsIFrame* LineContainer() const { return mLineContainer; }
 
     // The maximum intrinsic width for all previous lines.
     nscoord mPrevLines;
 
     // The maximum intrinsic width for the current line.  At a line
     // break (mandatory for preferred width; allowed for minimum width),
@@ -2221,20 +2218,16 @@ public:
     // |mCurrentLine|; it is zero if there is no such whitespace.
     nscoord mTrailingWhitespace;
 
     // True if initial collapsable whitespace should be skipped.  This
     // should be true at the beginning of a block, after hard breaks
     // and when the last text ended with whitespace.
     bool mSkipWhitespace;
 
-    // Writing mode of the line container (stored here so that we don't
-    // lose track of it if the mLineContainer field is reset).
-    mozilla::WritingMode mLineContainerWM;
-
     // Floats encountered in the lines.
     class FloatInfo {
     public:
       FloatInfo(const nsIFrame* aFrame, nscoord aWidth)
         : mFrame(aFrame), mWidth(aWidth)
       { }
       const nsIFrame* Frame() const { return mFrame; }
       nscoord         Width() const { return mWidth; }
--- a/layout/generic/nsLineBox.cpp
+++ b/layout/generic/nsLineBox.cpp
@@ -205,18 +205,16 @@ ListFloats(FILE* out, const char* aPrefi
 
 /* static */ const char*
 nsLineBox::BreakTypeToString(StyleClear aBreakType)
 {
   switch (aBreakType) {
     case StyleClear::None: return "nobr";
     case StyleClear::Left: return "leftbr";
     case StyleClear::Right: return "rightbr";
-    case StyleClear::InlineStart: return "inlinestartbr";
-    case StyleClear::InlineEnd: return "inlineendbr";
     case StyleClear::Both: return "leftbr+rightbr";
     case StyleClear::Line: return "linebr";
     case StyleClear::Max: return "leftbr+rightbr+linebr";
   }
   return "unknown";
 }
 
 char*
--- a/layout/style/ServoCSSPropList.mako.py
+++ b/layout/style/ServoCSSPropList.mako.py
@@ -69,16 +69,17 @@ def method(prop):
     return prop.camel_case
 
 # Colors, integers and lengths are easy as well.
 #
 # TODO(emilio): This will go away once the rest of the longhands have been
 # moved or perhaps using a blacklist for the ones with non-layout-dependence
 # but other non-trivial dependence like scrollbar colors.
 SERIALIZED_PREDEFINED_TYPES = [
+    "Clear",
     "Color",
     "Content",
     "CounterIncrement",
     "CounterReset",
     "Float",
     "FontFamily",
     "FontFeatureSettings",
     "FontLanguageOverride",
--- a/layout/style/nsStyleConsts.h
+++ b/layout/style/nsStyleConsts.h
@@ -72,21 +72,22 @@ enum class StyleBoxShadowType : uint8_t 
   Inset,
 };
 
 // clear
 enum class StyleClear : uint8_t {
   None = 0,
   Left,
   Right,
-  InlineStart,
-  InlineEnd,
   Both,
   // StyleClear::Line can be added to one of the other values in layout
   // so it needs to use a bit value that none of the other values can have.
+  //
+  // FIXME(emilio): Doesn't look like we do that anymore, so probably can be
+  // made a single value instead, and Max removed.
   Line = 8,
   Max = 13  // Max = (Both | Line)
 };
 
 // Counters and generated content.
 enum class StyleContentType : uint8_t {
   String = 1,
   Image = 10,
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -2450,18 +2450,16 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
 
 private:
   // Helpers for above functions, which do some but not all of the tests
   // for them (since transform must be tested separately for each).
   inline bool HasAbsPosContainingBlockStyleInternal() const;
   inline bool HasFixedPosContainingBlockStyleInternal(
     mozilla::ComputedStyle&) const;
   void GenerateCombinedTransform();
-public:
-  inline mozilla::StyleClear PhysicalBreakType(mozilla::WritingMode aWM) const;
 };
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleTable
 {
   explicit nsStyleTable(const nsPresContext* aContext);
   nsStyleTable(const nsStyleTable& aOther);
   ~nsStyleTable();
   void FinishStyle(nsPresContext*, const nsStyleTable*) {}
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3081,16 +3081,24 @@ fn static_assert() {
         self.gecko.mDisplay = Self::match_display_keyword(v);
     }
 
     <%call expr="impl_keyword_clone('display', 'mDisplay', display_keyword)"></%call>
 
     <% float_keyword = Keyword("float", "Left Right None", gecko_enum_prefix="StyleFloat") %>
     ${impl_keyword('float', 'mFloat', float_keyword)}
 
+    <% clear_keyword = Keyword(
+        "clear",
+        "Left Right None Both",
+        gecko_enum_prefix="StyleClear",
+        gecko_inexhaustive=True,
+    ) %>
+    ${impl_keyword('clear', 'mBreakType', clear_keyword)}
+
     <% overflow_x = data.longhands_by_name["overflow-x"] %>
     pub fn set_overflow_y(&mut self, v: longhands::overflow_y::computed_value::T) {
         use properties::longhands::overflow_x::computed_value::T as BaseType;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
         self.gecko.mOverflowY = match v {
             % for value in overflow_x.keyword.values_for('gecko'):
                 BaseType::${to_camel_case(value)} => structs::${overflow_x.keyword.gecko_constant(value)} as u8,
             % endfor
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -3946,17 +3946,17 @@ pub unsafe extern "C" fn Servo_Declarati
 pub extern "C" fn Servo_DeclarationBlock_SetKeywordValue(
     declarations: RawServoDeclarationBlockBorrowed,
     property: nsCSSPropertyID,
     value: i32
 ) {
     use style::properties::{PropertyDeclaration, LonghandId};
     use style::properties::longhands;
     use style::values::specified::BorderStyle;
-    use style::values::specified::Float;
+    use style::values::specified::{Clear, Float};
     use style::values::generics::font::FontStyle;
 
     let long = get_longhand_from_id!(property);
     let value = value as u32;
 
     let prop = match_wrap_declared! { long,
         MozUserModify => longhands::_moz_user_modify::SpecifiedValue::from_gecko_keyword(value),
         Direction => longhands::direction::SpecifiedValue::from_gecko_keyword(value),
@@ -3967,20 +3967,32 @@ pub extern "C" fn Servo_DeclarationBlock
             const NONE: u32 = structs::StyleFloat::None as u32;
             match value {
                 LEFT => Float::Left,
                 RIGHT => Float::Right,
                 NONE => Float::None,
                 _ => unreachable!(),
             }
         },
+        Clear => {
+            const LEFT: u32 = structs::StyleClear::Left as u32;
+            const RIGHT: u32 = structs::StyleClear::Right as u32;
+            const NONE: u32 = structs::StyleClear::None as u32;
+            const BOTH: u32 = structs::StyleClear::Both as u32;
+            match value {
+                LEFT => Clear::Left,
+                RIGHT => Clear::Right,
+                NONE => Clear::None,
+                BOTH => Clear::Both,
+                _ => unreachable!(),
+            }
+        },
         VerticalAlign => longhands::vertical_align::SpecifiedValue::from_gecko_keyword(value),
         TextAlign => longhands::text_align::SpecifiedValue::from_gecko_keyword(value),
         TextEmphasisPosition => longhands::text_emphasis_position::SpecifiedValue::from_gecko_keyword(value),
-        Clear => longhands::clear::SpecifiedValue::from_gecko_keyword(value),
         FontSize => {
             // We rely on Gecko passing in font-size values (0...7) here.
             longhands::font_size::SpecifiedValue::from_html_size(value as u8)
         },
         FontStyle => {
             let val = if value == structs::NS_FONT_STYLE_ITALIC {
                 FontStyle::Italic
             } else {