Bug 1516780 - Remove ComputedStyle::mBits. r=jfkthame,heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 19 Mar 2019 21:10:30 +0000
changeset 465112 8238c8aeb85146fcd9495357a0f2d8c7fcf2ff55
parent 465111 69c8acfbec73e624acef2fbe0f106a641db20816
child 465113 9499e5e8551918bf27f40114b6863de5a92a10a3
push id35732
push useropoprus@mozilla.com
push dateWed, 20 Mar 2019 10:52:37 +0000
treeherdermozilla-central@708979f9c3f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame, heycam
bugs1516780
milestone68.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 1516780 - Remove ComputedStyle::mBits. r=jfkthame,heycam Use the Servo flags instead. Differential Revision: https://phabricator.services.mozilla.com/D20729
layout/style/ComputedStyle.cpp
layout/style/ComputedStyle.h
servo/components/style/properties/computed_value_flags.rs
servo/ports/geckolib/glue.rs
--- a/layout/style/ComputedStyle.cpp
+++ b/layout/style/ComputedStyle.cpp
@@ -40,19 +40,17 @@
 #include "mozilla/ServoBindings.h"
 
 namespace mozilla {
 
 //----------------------------------------------------------------------
 
 ComputedStyle::ComputedStyle(PseudoStyleType aPseudoType,
                              ServoComputedDataForgotten aComputedValues)
-    : mSource(aComputedValues),
-      mBits(static_cast<Bit>(Servo_ComputedValues_GetStyleBits(this))),
-      mPseudoType(aPseudoType) {}
+    : mSource(aComputedValues), mPseudoType(aPseudoType) {}
 
 nsChangeHint ComputedStyle::CalcStyleDifference(const ComputedStyle& aNewStyle,
                                                 uint32_t* aEqualStructs) const {
   AUTO_PROFILER_LABEL("ComputedStyle::CalcStyleDifference", LAYOUT);
   static_assert(StyleStructConstants::kStyleStructCount <= 32,
                 "aEqualStructs is not big enough");
 
   *aEqualStructs = 0;
--- a/layout/style/ComputedStyle.h
+++ b/layout/style/ComputedStyle.h
@@ -59,33 +59,43 @@ class ComputedStyle;
  * ComputedStyles are reference counted. References are generally held by:
  *
  *  1. nsIFrame::mComputedStyle, for every frame
  *  2. Element::mServoData, for every element not inside a display:none subtree
  *  3. nsComputedDOMStyle, when created for elements in display:none subtrees
  *  4. media_queries::Device, which holds the initial value of every property
  */
 
-enum class ComputedStyleBit : uint8_t {
+// Various bits used by both Servo and Gecko.
+//
+// Please add an assert that this matches the Servo bit in
+// computed_value_flags::assert_match().
+//
+// FIXME(emilio): Would be nice to use cbindgen when it can handle bitflags!
+// without macro expansion, see https://github.com/eqrion/cbindgen/issues/100.
+enum class ComputedStyleBit : uint16_t {
   HasTextDecorationLines = 1 << 0,
-  HasPseudoElementData = 1 << 1,
-  SuppressLineBreak = 1 << 2,
-  IsTextCombined = 1 << 3,
-  RelevantLinkVisited = 1 << 4,
+  SuppressLineBreak = 1 << 1,
+  IsTextCombined = 1 << 2,
+  RelevantLinkVisited = 1 << 3,
+  HasPseudoElementData = 1 << 4,
+  DependsOnFontMetrics = 1 << 9,
 };
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ComputedStyleBit)
 
 class ComputedStyle {
   using Bit = ComputedStyleBit;
 
  public:
   ComputedStyle(PseudoStyleType aPseudoType,
                 ServoComputedDataForgotten aComputedValues);
 
+  Bit Bits() const { return static_cast<Bit>(mSource.flags.mFlags); }
+
   void AddRef() { Servo_ComputedStyle_AddRef(this); }
   void Release() { Servo_ComputedStyle_Release(this); }
 
   // Return the ComputedStyle whose style data should be used for the R,
   // G, and B components of color, background-color, and border-*-color
   // if RelevantLinkIsVisited().
   //
   // GetPseudo() and GetPseudoType() on this ComputedStyle return the
@@ -131,47 +141,53 @@ class ComputedStyle {
     return mPseudoType != PseudoStyleType::NotPseudo;
   }
 
   // Does this ComputedStyle or any of its ancestors have text
   // decoration lines?
   // Differs from nsStyleTextReset::HasTextDecorationLines, which tests
   // only the data for a single context.
   bool HasTextDecorationLines() const {
-    return bool(mBits & Bit::HasTextDecorationLines);
+    return bool(Bits() & Bit::HasTextDecorationLines);
   }
 
   // Whether any line break inside should be suppressed? If this returns
   // true, the line should not be broken inside, which means inlines act
   // as if nowrap is set, <br> is suppressed, and blocks are inlinized.
   // This bit is propogated to all children of line partitipants. It is
   // currently used by ruby to make its content frames unbreakable.
   // NOTE: for nsTextFrame, use nsTextFrame::ShouldSuppressLineBreak()
   // instead of this method.
   bool ShouldSuppressLineBreak() const {
-    return bool(mBits & Bit::SuppressLineBreak);
+    return bool(Bits() & Bit::SuppressLineBreak);
   }
 
   // Is this horizontal-in-vertical (tate-chu-yoko) text? This flag is
   // only set on ComputedStyles whose pseudo is nsCSSAnonBoxes::mozText().
-  bool IsTextCombined() const { return bool(mBits & Bit::IsTextCombined); }
+  bool IsTextCombined() const { return bool(Bits() & Bit::IsTextCombined); }
+
+  // Is this horizontal-in-vertical (tate-chu-yoko) text? This flag is
+  // only set on ComputedStyles whose pseudo is nsCSSAnonBoxes::mozText().
+  bool DependsOnFontMetrics() const {
+    return bool(Bits() & Bit::DependsOnFontMetrics);
+  }
 
   // Does this ComputedStyle represent the style for a pseudo-element or
   // inherit data from such a ComputedStyle?  Whether this returns true
   // is equivalent to whether it or any of its ancestors returns
   // non-null for IsPseudoElement().
   bool HasPseudoElementData() const {
-    return bool(mBits & Bit::HasPseudoElementData);
+    return bool(Bits() & Bit::HasPseudoElementData);
   }
 
   // Is the only link whose visitedness is allowed to influence the
   // style of the node this ComputedStyle is for (which is that element
   // or its nearest ancestor that is a link) visited?
   bool RelevantLinkVisited() const {
-    return bool(mBits & Bit::RelevantLinkVisited);
+    return bool(Bits() & Bit::RelevantLinkVisited);
   }
 
   ComputedStyle* GetCachedInheritingAnonBoxStyle(
       PseudoStyleType aPseudoType) const {
     MOZ_ASSERT(PseudoStyle::IsInheritingAnonBox(aPseudoType));
     return mCachedInheritingStyles.Lookup(aPseudoType);
   }
 
@@ -280,15 +296,14 @@ class ComputedStyle {
 
   ~ComputedStyle() = default;
 
   ServoComputedData mSource;
 
   // A cache of anonymous box and lazy pseudo styles inheriting from this style.
   CachedInheritingStyles mCachedInheritingStyles;
 
-  const Bit mBits;
   const PseudoStyleType mPseudoType;
 };
 
 }  // namespace mozilla
 
 #endif
--- a/servo/components/style/properties/computed_value_flags.rs
+++ b/servo/components/style/properties/computed_value_flags.rs
@@ -92,8 +92,27 @@ impl ComputedValueFlags {
 
     /// Flags that are conditionally propagated to descendants, just to handle
     /// properly style invalidation.
     #[inline]
     pub fn maybe_inherited(self) -> Self {
         self & Self::maybe_inherited_flags()
     }
 }
+
+/// Asserts that the relevant servo and Gecko representations match.
+#[cfg(feature = "gecko")]
+#[inline]
+pub fn assert_match() {
+    use crate::gecko_bindings::structs;
+    macro_rules! assert_bit {
+        ($rust:ident, $cpp:ident) => {
+            debug_assert_eq!(ComputedValueFlags::$rust.bits, structs::$cpp);
+        }
+    }
+
+    assert_bit!(HAS_TEXT_DECORATION_LINES, ComputedStyleBit_HasTextDecorationLines);
+    assert_bit!(IS_IN_PSEUDO_ELEMENT_SUBTREE, ComputedStyleBit_HasPseudoElementData);
+    assert_bit!(SHOULD_SUPPRESS_LINEBREAK, ComputedStyleBit_SuppressLineBreak);
+    assert_bit!(IS_TEXT_COMBINED, ComputedStyleBit_IsTextCombined);
+    assert_bit!(IS_RELEVANT_LINK_VISITED, ComputedStyleBit_RelevantLinkVisited);
+    assert_bit!(DEPENDS_ON_FONT_METRICS, ComputedStyleBit_DependsOnFontMetrics);
+}
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -229,23 +229,25 @@ impl ClosureHelper for DeclarationBlockM
 
 // A dummy url data for where we don't pass url data in.
 // We need to get rid of this sooner than later.
 static mut DUMMY_URL_DATA: *mut URLExtraData = 0 as *mut _;
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_Initialize(dummy_url_data: *mut URLExtraData) {
     use style::gecko_bindings::sugar::origin_flags;
+    use style::properties::computed_value_flags;
 
     // Pretend that we're a Servo Layout thread, to make some assertions happy.
     thread_state::initialize(thread_state::ThreadState::LAYOUT);
 
     // Perform some debug-only runtime assertions.
     origin_flags::assert_flags_match();
     parser::assert_parsing_mode_match();
+    computed_value_flags::assert_match();
     traversal_flags::assert_traversal_flags_match();
     specified::font::assert_variant_east_asian_matches();
     specified::font::assert_variant_ligatures_matches();
 
     DUMMY_URL_DATA = dummy_url_data;
 }
 
 #[no_mangle]
@@ -3513,40 +3515,16 @@ pub unsafe extern "C" fn Servo_ComputedV
     if for_text {
         StyleAdjuster::new(&mut style).adjust_for_text();
     }
 
     style.build().into()
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_ComputedValues_GetStyleBits(values: ComputedStyleBorrowed) -> u8 {
-    use style::properties::computed_value_flags::ComputedValueFlags;
-    // FIXME(emilio): We could do this more efficiently I'm quite sure.
-    let flags = values.flags;
-    let mut result = 0;
-    if flags.contains(ComputedValueFlags::IS_RELEVANT_LINK_VISITED) {
-        result |= structs::ComputedStyleBit_RelevantLinkVisited;
-    }
-    if flags.contains(ComputedValueFlags::HAS_TEXT_DECORATION_LINES) {
-        result |= structs::ComputedStyleBit_HasTextDecorationLines;
-    }
-    if flags.contains(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK) {
-        result |= structs::ComputedStyleBit_SuppressLineBreak;
-    }
-    if flags.contains(ComputedValueFlags::IS_TEXT_COMBINED) {
-        result |= structs::ComputedStyleBit_IsTextCombined;
-    }
-    if flags.contains(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE) {
-        result |= structs::ComputedStyleBit_HasPseudoElementData;
-    }
-    result
-}
-
-#[no_mangle]
 pub extern "C" fn Servo_ComputedValues_SpecifiesAnimationsOrTransitions(
     values: ComputedStyleBorrowed,
 ) -> bool {
     let b = values.get_box();
     b.specifies_animations() || b.specifies_transitions()
 }
 
 #[no_mangle]