Bug 1476054: Fixes and cleanups for Servo PR #21139. r=me
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 16 Jul 2018 18:50:18 +0200
changeset 484665 449941e7a840f4d64cc5f53d7967b65981b5a2ed
parent 484664 0486cd62a807989f85459e360f015bdb1b7a0cf1
child 484666 fb59c8181fc969659ce9727af15d36cd86ac7ccb
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1476054, 21139
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 #21139. r=me Logical floats don't appear in the computed style objects, so there's no need to check for them. MozReview-Commit-ID: 3ocJrRB3jeO
layout/generic/BlockReflowInput.cpp
layout/generic/WritingModes.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsFloatManager.cpp
layout/generic/nsFrame.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/BlockReflowInput.cpp
+++ b/layout/generic/BlockReflowInput.cpp
@@ -780,17 +780,17 @@ BlockReflowInput::FlowAndPlaceFloat(nsIF
                  "letter frames and orthogonal floats with auto block-size "
                  "shouldn't break, and if they do now, then they're breaking "
                  "at the wrong point");
   }
 
   // Find a place to place the float. The CSS2 spec doesn't want
   // floats overlapping each other or sticking out of the containing
   // block if possible (CSS2 spec section 9.5.1, see the rule list).
-  StyleFloat floatStyle = floatDisplay->PhysicalFloats(wm);
+  StyleFloat floatStyle = floatDisplay->mFloat;
   MOZ_ASSERT(StyleFloat::Left == floatStyle || StyleFloat::Right == floatStyle,
              "Invalid float type!");
 
   // Can the float fit here?
   bool keepFloatOnSameLine = false;
 
   // Are we required to place at least part of the float because we're
   // at the top of the page (to avoid an infinite loop of pushing and
@@ -1040,18 +1040,17 @@ BlockReflowInput::FlowAndPlaceFloat(nsIF
 void
 BlockReflowInput::PushFloatPastBreak(nsIFrame *aFloat)
 {
   // This ensures that we:
   //  * don't try to place later but smaller floats (which CSS says
   //    must have their tops below the top of this float)
   //  * don't waste much time trying to reflow this float again until
   //    after the break
-  StyleFloat floatStyle =
-    aFloat->StyleDisplay()->PhysicalFloats(mReflowInput.GetWritingMode());
+  StyleFloat floatStyle = aFloat->StyleDisplay()->mFloat;
   if (floatStyle == StyleFloat::Left) {
     FloatManager()->SetPushedLeftFloatPastBreak();
   } else {
     MOZ_ASSERT(floatStyle == StyleFloat::Right, "Unexpected float value!");
     FloatManager()->SetPushedRightFloatPastBreak();
   }
 
   // Put the float on the pushed floats list, even though it
--- 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::StyleFloat
-nsStyleDisplay::PhysicalFloats(mozilla::WritingMode aWM) const
-{
-  using StyleFloat = mozilla::StyleFloat;
-  if (mFloat == StyleFloat::InlineStart) {
-    return aWM.IsBidiLTR() ? StyleFloat::Left : StyleFloat::Right;
-  }
-  if (mFloat == StyleFloat::InlineEnd) {
-    return aWM.IsBidiLTR() ? StyleFloat::Right : StyleFloat::Left;
-  }
-  return mFloat;
-}
-
 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) {
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -4341,18 +4341,17 @@ nsBlockFrame::SplitFloat(BlockReflowInpu
   } else {
     nextInFlow = aState.mPresContext->PresShell()->FrameConstructor()->
       CreateContinuingFrame(aState.mPresContext, aFloat, this);
   }
   if (aFloatStatus.IsOverflowIncomplete()) {
     nextInFlow->AddStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER);
   }
 
-  StyleFloat floatStyle =
-    aFloat->StyleDisplay()->PhysicalFloats(aState.mReflowInput.GetWritingMode());
+  StyleFloat floatStyle = aFloat->StyleDisplay()->mFloat;
   if (floatStyle == StyleFloat::Left) {
     aState.FloatManager()->SetSplitLeftFloatAcrossBreak();
   } else {
     MOZ_ASSERT(floatStyle == StyleFloat::Right, "Unexpected float side!");
     aState.FloatManager()->SetSplitRightFloatAcrossBreak();
   }
 
   aState.AppendPushedFloatChain(nextInFlow);
--- a/layout/generic/nsFloatManager.cpp
+++ b/layout/generic/nsFloatManager.cpp
@@ -201,17 +201,17 @@ nsFloatManager::GetFlowArea(WritingMode 
     // WidthWithinHeight call is at least as narrow on both sides as a
     // BandFromPoint call beginning at its blockStart.
     else if (blockStart < floatBEnd &&
              (floatBStart < blockEnd ||
               (floatBStart == blockEnd && blockStart == blockEnd))) {
       // This float is in our band.
 
       // Shrink our band's width if needed.
-      StyleFloat floatStyle = fi.mFrame->StyleDisplay()->PhysicalFloats(aWM);
+      StyleFloat floatStyle = fi.mFrame->StyleDisplay()->mFloat;
 
       // When aBandInfoType is BandFromPoint, we're only intended to
       // consider a point along the y axis rather than a band.
       const nscoord bandBlockEnd =
         aBandInfoType == BandInfoType::BandFromPoint ? blockStart : blockEnd;
       if (floatStyle == StyleFloat::Left) {
         // A left float
         nscoord lineRightEdge =
@@ -278,17 +278,17 @@ nsFloatManager::AddFloat(nsIFrame* aFloa
   if (HasAnyFloats()) {
     FloatInfo &tail = mFloats[mFloats.Length() - 1];
     info.mLeftBEnd = tail.mLeftBEnd;
     info.mRightBEnd = tail.mRightBEnd;
   } else {
     info.mLeftBEnd = nscoord_MIN;
     info.mRightBEnd = nscoord_MIN;
   }
-  StyleFloat floatStyle = aFloatFrame->StyleDisplay()->PhysicalFloats(aWM);
+  StyleFloat floatStyle = aFloatFrame->StyleDisplay()->mFloat;
   MOZ_ASSERT(floatStyle == StyleFloat::Left || floatStyle == StyleFloat::Right,
              "Unexpected float style!");
   nscoord& sideBEnd =
     floatStyle == StyleFloat::Left ? info.mLeftBEnd : info.mRightBEnd;
   nscoord thisBEnd = info.BEnd();
   if (thisBEnd > sideBEnd)
     sideBEnd = thisBEnd;
 
@@ -311,17 +311,17 @@ nsFloatManager::CalculateRegionFor(Writi
   region.Inflate(aWM, aMargin);
 
   // Don't store rectangles with negative margin-box width or height in
   // the float manager; it can't deal with them.
   if (region.ISize(aWM) < 0) {
     // Preserve the right margin-edge for left floats and the left
     // margin-edge for right floats
     const nsStyleDisplay* display = aFloat->StyleDisplay();
-    StyleFloat floatStyle = display->PhysicalFloats(aWM);
+    StyleFloat floatStyle = display->mFloat;
     if ((StyleFloat::Left == floatStyle) == aWM.IsBidiLTR()) {
       region.IStart(aWM) = region.IEnd(aWM);
     }
     region.ISize(aWM) = 0;
   }
   if (region.BSize(aWM) < 0) {
     region.BSize(aWM) = 0;
   }
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5374,17 +5374,17 @@ nsIFrame::InlinePrefISizeData::ForceBrea
         if (breakType != StyleClear::Right) {
           floats_cur_left = 0;
         }
         if (breakType != StyleClear::Left) {
           floats_cur_right = 0;
         }
       }
 
-      StyleFloat floatStyle = floatDisp->PhysicalFloats(wm);
+      StyleFloat floatStyle = floatDisp->mFloat;
       nscoord& floats_cur =
         floatStyle == StyleFloat::Left ? floats_cur_left : floats_cur_right;
       nscoord floatWidth = floatInfo.Width();
       // Negative-width floats don't change the available space so they
       // shouldn't change our intrinsic line width either.
       floats_cur =
         NSCoordSaturatingAdd(floats_cur, std::max(0, floatWidth));
     }
@@ -5409,17 +5409,17 @@ nsIFrame::InlinePrefISizeData::ForceBrea
                  aBreakType == StyleClear::Right,
                  "Other values should have been handled in other branches");
       StyleFloat clearFloatType =
         aBreakType == StyleClear::Left ? StyleFloat::Left : StyleFloat::Right;
       // Iterate the array in reverse so that we can stop when there are
       // no longer any floats we need to keep. See below.
       for (FloatInfo& floatInfo : Reversed(mFloats)) {
         const nsStyleDisplay* floatDisp = floatInfo.Frame()->StyleDisplay();
-        if (floatDisp->PhysicalFloats(wm) != clearFloatType) {
+        if (floatDisp->mFloat != clearFloatType) {
           newFloats.AppendElement(floatInfo);
         } 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
--- a/layout/style/ServoCSSPropList.mako.py
+++ b/layout/style/ServoCSSPropList.mako.py
@@ -73,16 +73,17 @@ def method(prop):
 # 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 = [
     "Color",
     "Content",
     "CounterIncrement",
     "CounterReset",
+    "Float",
     "FontFamily",
     "FontFeatureSettings",
     "FontLanguageOverride",
     "FontSize",
     "FontSizeAdjust",
     "FontStretch",
     "FontStyle",
     "FontSynthesis",
--- a/layout/style/nsStyleConsts.h
+++ b/layout/style/nsStyleConsts.h
@@ -139,18 +139,16 @@ enum class StyleFillRule : uint8_t {
 };
 
 // float
 // https://developer.mozilla.org/en-US/docs/Web/CSS/float
 enum class StyleFloat : uint8_t {
   None,
   Left,
   Right,
-  InlineStart,
-  InlineEnd
 };
 
 // float-edge
 enum class StyleFloatEdge : uint8_t {
   ContentBox,
   MarginBox,
 };
 
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -2451,20 +2451,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:
-  // Return the 'float' and 'clear' properties, with inline-{start,end} values
-  // resolved to {left,right} according to the given writing mode. These are
-  // defined in WritingModes.h.
-  inline mozilla::StyleFloat PhysicalFloats(mozilla::WritingMode aWM) const;
   inline mozilla::StyleClear PhysicalBreakType(mozilla::WritingMode aWM) const;
 };
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleTable
 {
   explicit nsStyleTable(const nsPresContext* aContext);
   nsStyleTable(const nsStyleTable& aOther);
   ~nsStyleTable();
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3078,16 +3078,19 @@ fn static_assert() {
         v: longhands::display::computed_value::T,
         _is_item_or_root: bool
     ) {
         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)}
+
     <% 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,27 +3946,37 @@ 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::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),
-        // TextEmphasisPosition => FIXME implement text-emphasis-position
         Direction => longhands::direction::SpecifiedValue::from_gecko_keyword(value),
         Display => longhands::display::SpecifiedValue::from_gecko_keyword(value),
-        Float => longhands::float::SpecifiedValue::from_gecko_keyword(value),
+        Float => {
+            const LEFT: u32 = structs::StyleFloat::Left as u32;
+            const RIGHT: u32 = structs::StyleFloat::Right as u32;
+            const NONE: u32 = structs::StyleFloat::None as u32;
+            match value {
+                LEFT => Float::Left,
+                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)
         },