Bug 1476054: Fixes and cleanups for Servo PR #21156. r=me
☠☠ backed out by 6339739cba9a ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 16 Jul 2018 19:01:24 +0200
changeset 426845 0a712d7bcb66d19a94ba05d020c28b845071aee6
parent 426844 62293a989ed23d16194315a37bed2b19dd6aec46
child 426846 cc571c618e4c256cedbdd8859ef0d9f7e97d0afd
push id34287
push userccoroiu@mozilla.com
push dateTue, 17 Jul 2018 09:42:00 +0000
treeherdermozilla-central@547144f5596c [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
@@ -3970,17 +3970,16 @@ pub extern "C" fn Servo_DeclarationBlock
                 RIGHT => Float::Right,
                 NONE => Float::None,
                 _ => 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 {