Bug 1624080 - Simplify the implementation of HasAuthorSpecifiedRules. r=heycam
☠☠ backed out by 11a006e5dfc6 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 26 Mar 2020 13:23:42 +0000
changeset 520552 ac0d06c0ca9330d5356717094910e977cf8f6c29
parent 520551 a5255aaee6ad48974932e064328d8cd11e21bbc0
child 520553 a266e7a5a2e1b5141b42f635d93eb6604849e345
push id37253
push usernerli@mozilla.com
push dateThu, 26 Mar 2020 21:36:52 +0000
treeherdermozilla-central@c644dd16e2cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1624080, 452969
milestone76.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 1624080 - Simplify the implementation of HasAuthorSpecifiedRules. r=heycam This patch computes the author-specified properties during the CSS cascade, and removes the complex rule-tree-based implementation that tries to do the cascade again. This changes behavior in two ways, one of them which is not observable to content, I believe: * revert now re-enables the native styling. This was brought up in https://github.com/w3c/csswg-drafts/issues/4777 and I think it is a bug-fix. This is observable to content, and I'm adding a test for it. * We don't look at inherited styles from our ancestors when `inherit` is specified in a non-author stylesheet. This was introduced for bug 452969 but we don't seem to inherit background anymore for file controls or such. It seems back then file controls used to have a text-field. I audited forms.css and ua.css and we don't explicitly inherit padding / border / background-color into any nested form control. We keep the distinction between border/background and padding, because the later has some callers. I think we should try to align with Chromium in the long run and remove the padding bit. We need to give an appearance to the range-thumb and such so that we can assert that we don't call HasAuthorSpecifiedRules on non-themed stuff. I used a new internal value for that. Differential Revision: https://phabricator.services.mozilla.com/D67722
devtools/shared/css/generated/properties-db.js
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/forms/nsMeterFrame.cpp
layout/forms/nsNumberControlFrame.cpp
layout/forms/nsProgressFrame.cpp
layout/forms/nsRangeFrame.cpp
layout/reftests/forms/button/appearance-revert-ref.html
layout/reftests/forms/button/appearance-revert.html
layout/reftests/forms/button/reftest.list
layout/style/ComputedStyle.h
layout/style/ServoBindings.toml
layout/style/res/forms.css
servo/components/style/properties/cascade.rs
servo/components/style/properties/computed_value_flags.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/rule_tree/mod.rs
servo/components/style/values/specified/box.rs
servo/ports/geckolib/glue.rs
widget/nsNativeTheme.cpp
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -259,17 +259,19 @@ exports.CSS_PROPERTIES = {
       "progressbar",
       "progressbar-vertical",
       "progresschunk",
       "radio",
       "radio-container",
       "radio-label",
       "radiomenuitem",
       "range",
+      "range-progress",
       "range-thumb",
+      "range-track",
       "resizer",
       "resizerpanel",
       "revert",
       "scale-horizontal",
       "scale-vertical",
       "scalethumb-horizontal",
       "scalethumb-vertical",
       "scalethumbend",
@@ -1576,17 +1578,19 @@ exports.CSS_PROPERTIES = {
       "progressbar",
       "progressbar-vertical",
       "progresschunk",
       "radio",
       "radio-container",
       "radio-label",
       "radiomenuitem",
       "range",
+      "range-progress",
       "range-thumb",
+      "range-track",
       "resizer",
       "resizerpanel",
       "revert",
       "scale-horizontal",
       "scale-vertical",
       "scalethumb-horizontal",
       "scalethumb-vertical",
       "scalethumbend",
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1623,38 +1623,33 @@ void nsPresContext::CountReflows(const c
   if (mPresShell) {
     mPresShell->CountReflows(aName, aFrame);
   }
 }
 #endif
 
 bool nsPresContext::HasAuthorSpecifiedRules(const nsIFrame* aFrame,
                                             uint32_t aRuleTypeMask) const {
-  Element* elem = aFrame->GetContent()->AsElement();
-
-  // We need to handle non-generated content pseudos too, so we use
-  // the parent of generated content pseudo to be consistent.
-  if (elem->GetPseudoElementType() != PseudoStyleType::NotPseudo) {
-    MOZ_ASSERT(elem->GetParent(), "Pseudo element has no parent element?");
-    elem = elem->GetParent()->AsElement();
+  MOZ_ASSERT(aFrame->StyleDisplay()->HasAppearance(),
+             "This should only be used to disable native appearance");
+  const bool padding = aRuleTypeMask & NS_AUTHOR_SPECIFIED_PADDING;
+  const bool borderBackground =
+      aRuleTypeMask & NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND;
+  const auto& style = *aFrame->Style();
+
+  if (padding && style.HasAppearanceAndAuthorSpecifiedPadding()) {
+    return true;
   }
-  if (MOZ_UNLIKELY(!elem->HasServoData())) {
-    // Probably shouldn't happen, but does. See bug 1387953
-    return false;
+
+  if (borderBackground &&
+      style.HasAppearanceAndAuthorSpecifiedBorderOrBackground()) {
+    return true;
   }
 
-  // Anonymous boxes are more complicated, and we just assume that they
-  // cannot have any author-specified rules here.
-  if (aFrame->Style()->IsAnonBox()) {
-    return false;
-  }
-
-  auto* set = PresShell()->StyleSet()->RawSet();
-  return Servo_HasAuthorSpecifiedRules(set, aFrame->Style(), elem,
-                                       aRuleTypeMask);
+  return false;
 }
 
 gfxUserFontSet* nsPresContext::GetUserFontSet() {
   return mDocument->GetUserFontSet();
 }
 
 void nsPresContext::UserFontSetUpdated(gfxUserFontEntry* aUpdatedFont) {
   if (!mPresShell) {
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -112,19 +112,18 @@ enum class nsLayoutPhase : uint8_t {
   DisplayListBuilding,  // sometimes a subset of the paint phase
   Reflow,
   FrameC,
   COUNT
 };
 #endif
 
 /* Used by nsPresContext::HasAuthorSpecifiedRules */
-#define NS_AUTHOR_SPECIFIED_BACKGROUND (1 << 0)
-#define NS_AUTHOR_SPECIFIED_BORDER (1 << 1)
-#define NS_AUTHOR_SPECIFIED_PADDING (1 << 2)
+#define NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND (1 << 0)
+#define NS_AUTHOR_SPECIFIED_PADDING (1 << 1)
 
 class nsRootPresContext;
 
 // An interface for presentation contexts. Presentation contexts are
 // objects that provide an outer context for a presentation shell.
 
 class nsPresContext : public nsISupports,
                       public mozilla::SupportsWeakPtr<nsPresContext> {
--- a/layout/forms/nsMeterFrame.cpp
+++ b/layout/forms/nsMeterFrame.cpp
@@ -227,16 +227,14 @@ bool nsMeterFrame::ShouldUseNativeStyle(
   nsIFrame* barFrame = mBarDiv->GetPrimaryFrame();
 
   // Use the native style if these conditions are satisfied:
   // - both frames use the native appearance;
   // - neither frame has author specified rules setting the border or the
   //   background.
   return StyleDisplay()->mAppearance == StyleAppearance::Meter &&
          !PresContext()->HasAuthorSpecifiedRules(
-             this,
-             NS_AUTHOR_SPECIFIED_BORDER | NS_AUTHOR_SPECIFIED_BACKGROUND) &&
+             this, NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND) &&
          barFrame &&
          barFrame->StyleDisplay()->mAppearance == StyleAppearance::Meterchunk &&
          !PresContext()->HasAuthorSpecifiedRules(
-             barFrame,
-             NS_AUTHOR_SPECIFIED_BORDER | NS_AUTHOR_SPECIFIED_BACKGROUND);
+             barFrame, NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND);
 }
--- a/layout/forms/nsNumberControlFrame.cpp
+++ b/layout/forms/nsNumberControlFrame.cpp
@@ -226,19 +226,18 @@ bool nsNumberControlFrame::SpinnerUpButt
       ->NumberSpinnerUpButtonIsDepressed();
 }
 
 bool nsNumberControlFrame::SpinnerDownButtonIsDepressed() const {
   return HTMLInputElement::FromNode(mContent)
       ->NumberSpinnerDownButtonIsDepressed();
 }
 
-#define STYLES_DISABLING_NATIVE_THEMING                          \
-  NS_AUTHOR_SPECIFIED_BACKGROUND | NS_AUTHOR_SPECIFIED_PADDING | \
-      NS_AUTHOR_SPECIFIED_BORDER
+#define STYLES_DISABLING_NATIVE_THEMING \
+  NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND | NS_AUTHOR_SPECIFIED_PADDING
 
 bool nsNumberControlFrame::ShouldUseNativeStyleForSpinner() const {
   MOZ_ASSERT(mSpinUp && mSpinDown,
              "We should not be called when we have no spinner");
 
   nsIFrame* spinUpFrame = mSpinUp->GetPrimaryFrame();
   nsIFrame* spinDownFrame = mSpinDown->GetPrimaryFrame();
 
--- a/layout/forms/nsProgressFrame.cpp
+++ b/layout/forms/nsProgressFrame.cpp
@@ -243,17 +243,15 @@ bool nsProgressFrame::ShouldUseNativeSty
   nsIFrame* barFrame = PrincipalChildList().FirstChild();
 
   // Use the native style if these conditions are satisfied:
   // - both frames use the native appearance;
   // - neither frame has author specified rules setting the border or the
   //   background.
   return StyleDisplay()->mAppearance == StyleAppearance::ProgressBar &&
          !PresContext()->HasAuthorSpecifiedRules(
-             this,
-             NS_AUTHOR_SPECIFIED_BORDER | NS_AUTHOR_SPECIFIED_BACKGROUND) &&
+             this, NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND) &&
          barFrame &&
          barFrame->StyleDisplay()->mAppearance ==
              StyleAppearance::Progresschunk &&
          !PresContext()->HasAuthorSpecifiedRules(
-             barFrame,
-             NS_AUTHOR_SPECIFIED_BORDER | NS_AUTHOR_SPECIFIED_BACKGROUND);
+             barFrame, NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND);
 }
--- a/layout/forms/nsRangeFrame.cpp
+++ b/layout/forms/nsRangeFrame.cpp
@@ -795,27 +795,25 @@ double nsRangeFrame::GetMax() const {
 }
 
 double nsRangeFrame::GetValue() const {
   return static_cast<dom::HTMLInputElement*>(GetContent())
       ->GetValueAsDecimal()
       .toDouble();
 }
 
-#define STYLES_DISABLING_NATIVE_THEMING                          \
-  NS_AUTHOR_SPECIFIED_BACKGROUND | NS_AUTHOR_SPECIFIED_PADDING | \
-      NS_AUTHOR_SPECIFIED_BORDER
+#define STYLES_DISABLING_NATIVE_THEMING \
+  NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND | NS_AUTHOR_SPECIFIED_PADDING
 
 bool nsRangeFrame::ShouldUseNativeStyle() const {
   nsIFrame* trackFrame = mTrackDiv->GetPrimaryFrame();
   nsIFrame* progressFrame = mProgressDiv->GetPrimaryFrame();
   nsIFrame* thumbFrame = mThumbDiv->GetPrimaryFrame();
 
-  return (StyleDisplay()->mAppearance == StyleAppearance::Range) &&
-         trackFrame &&
+  return StyleDisplay()->mAppearance == StyleAppearance::Range && trackFrame &&
          !PresContext()->HasAuthorSpecifiedRules(
              trackFrame, STYLES_DISABLING_NATIVE_THEMING) &&
          progressFrame &&
          !PresContext()->HasAuthorSpecifiedRules(
              progressFrame, STYLES_DISABLING_NATIVE_THEMING) &&
          thumbFrame &&
          !PresContext()->HasAuthorSpecifiedRules(
              thumbFrame, STYLES_DISABLING_NATIVE_THEMING);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/appearance-revert-ref.html
@@ -0,0 +1,2 @@
+<!doctype html>
+<button>Foo</button>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/appearance-revert.html
@@ -0,0 +1,2 @@
+<!doctype html>
+<button style="border: revert">Foo</button>
--- a/layout/reftests/forms/button/reftest.list
+++ b/layout/reftests/forms/button/reftest.list
@@ -45,8 +45,10 @@ fails-if(Android&&nativeThemePref) == di
 == width-exact-fit-rtl.html width-auto-size-rtl-ref.html
 == display-grid-flex-columnset.html display-grid-flex-columnset-ref.html
 == button-empty-columns.html button-empty-columns-ref.html
 == 1317351.html 1317351-ref.html
 
 == dynamic-text-indent.html dynamic-text-indent-ref.html
 
 == 1349646.html 1349646-ref.html
+
+== appearance-revert.html appearance-revert-ref.html
--- a/layout/style/ComputedStyle.h
+++ b/layout/style/ComputedStyle.h
@@ -117,16 +117,29 @@ class ComputedStyle {
   }
 
   bool IsAnonBox() const { return PseudoStyle::IsAnonBox(mPseudoType); }
 
   bool IsPseudoOrAnonBox() const {
     return mPseudoType != PseudoStyleType::NotPseudo;
   }
 
+  // Whether there are author-specified rules for padding properties.
+  // Only returns something meaningful if the appearance property is not `none`.
+  bool HasAppearanceAndAuthorSpecifiedPadding() const {
+    return bool(Flags() & Flag::HAS_AUTHOR_SPECIFIED_PADDING);
+  }
+
+  // Whether there are author-specified rules for border or background
+  // properties.
+  // Only returns something meaningful if the appearance property is not `none`.
+  bool HasAppearanceAndAuthorSpecifiedBorderOrBackground() const {
+    return bool(Flags() & Flag::HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND);
+  }
+
   // 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(Flags() & Flag::HAS_TEXT_DECORATION_LINES);
   }
 
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -162,18 +162,16 @@ rusty-enums = [
     "mozilla::StyleScrollBehavior",
     "mozilla::StyleColorInterpolation",
     "mozilla::StyleVectorEffect",
     "mozilla::StyleBackfaceVisibility",
     "mozilla::StyleBlend",
     "mozilla::StyleMaskComposite",
 ]
 whitelist-vars = [
-    "NS_AUTHOR_SPECIFIED_.*",
-    "NS_THEME_.*",
     "NS_ATTRVALUE_.*",
     "NODE_.*",
     "ELEMENT_.*",
     "NS_FONT_.*",
     "NS_STYLE_.*",
     "NS_MATHML_.*",
     "NS_RADIUS_.*",
     "BORDER_COLOR_.*",
--- a/layout/style/res/forms.css
+++ b/layout/style/res/forms.css
@@ -904,16 +904,17 @@ input[type=range]::-moz-focus-outer {
  * authors can concentrate on styling the thumb without worrying about the
  * logic to position it). Specifically the 'margin', 'top' and 'left'
  * properties are ignored.
  *
  * If content authors want to have a vertical range, they will also need to
  * set the width/height of this pseudo-element.
  */
 input[type=range]::-moz-range-track {
+  -moz-appearance: range-track !important;
   /* Prevent styling that would change the type of frame we construct. */
   display: block !important;
   float: none !important;
   position: static !important;
   writing-mode: unset !important;
   direction: unset !important;
   block-size: 0.2em; /* same as inline-size below */
   /* Prevent nsFrame::HandlePress setting mouse capture to this element. */
@@ -929,16 +930,17 @@ input[type=range][orient=vertical]::-moz
  * Layout handles positioning of this pseudo-element specially (so that content
  * authors can concentrate on styling this pseudo-element without worrying
  * about the logic to position it). Specifically the 'margin', 'top' and 'left'
  * properties are ignored. Additionally, if the range is horizontal, the width
  * property is ignored, and if the range range is vertical, the height property
  * is ignored.
  */
 input[type=range]::-moz-range-progress {
+  -moz-appearance: range-progress !important;
   /* Prevent styling that would change the type of frame we construct. */
   display: block !important;
   float: none !important;
   position: static !important;
   writing-mode: unset !important;
   direction: unset !important;
   /* Since one of width/height will be ignored, this just sets the "other"
      dimension.
--- a/servo/components/style/properties/cascade.rs
+++ b/servo/components/style/properties/cascade.rs
@@ -8,17 +8,17 @@ use crate::context::QuirksMode;
 use crate::custom_properties::CustomPropertiesBuilder;
 use crate::dom::TElement;
 use crate::font_metrics::FontMetricsProvider;
 use crate::logical_geometry::WritingMode;
 use crate::media_queries::Device;
 use crate::properties::{ComputedValues, StyleBuilder};
 use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword};
 use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator};
-use crate::properties::CASCADE_PROPERTY;
+use crate::properties::{CASCADE_PROPERTY, ComputedValueFlags};
 use crate::rule_cache::{RuleCache, RuleCacheConditions};
 use crate::rule_tree::StrongRuleNode;
 use crate::selector_parser::PseudoElement;
 use crate::stylesheets::{Origin, PerOrigin};
 use servo_arc::Arc;
 use crate::shared_lock::StylesheetGuards;
 use smallbitvec::SmallBitVec;
 use smallvec::SmallVec;
@@ -406,25 +406,27 @@ fn tweak_when_ignoring_colors(
     *declaration.to_mut() = PropertyDeclaration::css_wide_keyword(longhand_id, CSSWideKeyword::Revert);
 
 }
 
 struct Cascade<'a, 'b: 'a> {
     context: &'a mut computed::Context<'b>,
     cascade_mode: CascadeMode<'a>,
     seen: LonghandIdSet,
+    author_specified: LonghandIdSet,
     reverted: PerOrigin<LonghandIdSet>,
 }
 
 impl<'a, 'b: 'a> Cascade<'a, 'b> {
     fn new(context: &'a mut computed::Context<'b>, cascade_mode: CascadeMode<'a>) -> Self {
         Self {
             context,
             cascade_mode,
             seen: LonghandIdSet::default(),
+            author_specified: LonghandIdSet::default(),
             reverted: Default::default(),
         }
     }
 
     fn substitute_variables_if_needed<'decl>(
         &mut self,
         declaration: &'decl PropertyDeclaration,
     ) -> Cow<'decl, PropertyDeclaration> {
@@ -552,16 +554,19 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                     self.reverted
                         .borrow_mut_for_origin(&origin)
                         .insert(physical_longhand_id);
                 }
                 continue;
             }
 
             self.seen.insert(physical_longhand_id);
+            if origin == Origin::Author {
+                self.author_specified.insert(physical_longhand_id);
+            }
 
             let unset = css_wide_keyword.map_or(false, |css_wide_keyword| {
                 match css_wide_keyword {
                     CSSWideKeyword::Unset => true,
                     CSSWideKeyword::Inherit => inherited,
                     CSSWideKeyword::Initial => !inherited,
                     CSSWideKeyword::Revert => unreachable!(),
                 }
@@ -674,16 +679,25 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
         {
             if let Some(bg) = builder.get_background_if_mutated() {
                 bg.fill_arrays();
             }
 
             if let Some(svg) = builder.get_svg_if_mutated() {
                 svg.fill_arrays();
             }
+
+            if !builder.get_box().clone__moz_appearance().is_none() {
+                if self.author_specified.contains_any(LonghandIdSet::border_background_properties()) {
+                    builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND);
+                }
+                if self.author_specified.contains_any(LonghandIdSet::padding_properties()) {
+                    builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_PADDING);
+                }
+            }
         }
 
         #[cfg(feature = "servo")]
         {
             if let Some(font) = builder.get_font_if_mutated() {
                 font.compute_font_hash();
             }
         }
@@ -694,22 +708,36 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
         cache: Option<&'b RuleCache>,
         guards: &StylesheetGuards,
     ) -> bool {
         let cache = match cache {
             Some(cache) => cache,
             None => return false,
         };
 
-        let cached_style = match cache.find(guards, &self.context.builder) {
+        let builder = &mut self.context.builder;
+
+        let cached_style = match cache.find(guards, &builder) {
             Some(style) => style,
             None => return false,
         };
 
-        self.context.builder.copy_reset_from(cached_style);
+        builder.copy_reset_from(cached_style);
+
+        // We're using the same reset style as another element, and we'll skip
+        // applying the relevant properties. So we need to do the relevant
+        // bookkeeping here to keep these two bits correct.
+        //
+        // Note that all the properties involved are non-inherited, so we don't
+        // need to do anything else other than just copying the bits over.
+        let reset_props_bits =
+            ComputedValueFlags::HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND |
+            ComputedValueFlags::HAS_AUTHOR_SPECIFIED_PADDING;
+        builder.add_flags(cached_style.flags & reset_props_bits);
+
         true
     }
 
     /// The default font type (which is stored in FontFamilyList's
     /// `mDefaultFontType`) depends on the current lang group and generic font
     /// family, so we may need to recompute it if or the family changed.
     ///
     /// Also, we prioritize non-document fonts here if we need to (see the pref
--- a/servo/components/style/properties/computed_value_flags.rs
+++ b/servo/components/style/properties/computed_value_flags.rs
@@ -65,16 +65,30 @@ bitflags! {
         /// Only used in Servo.
         const CAN_BE_FRAGMENTED = 1 << 10;
 
         /// Whether this style is the style of the document element.
         const IS_ROOT_ELEMENT_STYLE = 1 << 11;
 
         /// Whether this element is inside an `opacity: 0` subtree.
         const IS_IN_OPACITY_ZERO_SUBTREE = 1 << 12;
+
+        /// Whether there are author-specified rules for border-* properties
+        /// (except border-image-*), background-color, or background-image.
+        ///
+        /// TODO(emilio): Maybe do include border-image, see:
+        ///
+        /// https://github.com/w3c/csswg-drafts/issues/4777#issuecomment-604424845
+        const HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND = 1 << 13;
+
+        /// Whether there are author-specified rules for padding-* properties.
+        ///
+        /// FIXME(emilio): Try to merge this with BORDER_BACKGROUND, see
+        /// https://github.com/w3c/csswg-drafts/issues/4777
+        const HAS_AUTHOR_SPECIFIED_PADDING = 1 << 14;
     }
 }
 
 impl ComputedValueFlags {
     /// Flags that are unconditionally propagated to descendants.
     #[inline]
     fn inherited_flags() -> Self {
         Self::IS_RELEVANT_LINK_VISITED |
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -745,16 +745,62 @@ static ${name}: LonghandIdSet = Longhand
         for i, property in enumerate(data.longhands):
             if is_member(property):
                 storage[i / 32] |= 1 << (i % 32)
     %>
     storage: [${", ".join("0x%x" % word for word in storage)}]
 };
 </%def>
 
+<%
+    logical_groups = defaultdict(list)
+    for prop in data.longhands:
+        if prop.logical_group:
+            logical_groups[prop.logical_group].append(prop)
+
+    for group, props in logical_groups.iteritems():
+        logical_count = sum(1 for p in props if p.logical)
+        if logical_count * 2 != len(props):
+            raise RuntimeError("Logical group {} has ".format(group) +
+                               "unbalanced logical / physical properties")
+
+    FIRST_LINE_RESTRICTIONS = PropertyRestrictions.first_line(data)
+    FIRST_LETTER_RESTRICTIONS = PropertyRestrictions.first_letter(data)
+    MARKER_RESTRICTIONS = PropertyRestrictions.marker(data)
+    PLACEHOLDER_RESTRICTIONS = PropertyRestrictions.placeholder(data)
+    CUE_RESTRICTIONS = PropertyRestrictions.cue(data)
+
+    def restriction_flags(property):
+        name = property.name
+        flags = []
+        if name in FIRST_LINE_RESTRICTIONS:
+            flags.append("APPLIES_TO_FIRST_LINE")
+        if name in FIRST_LETTER_RESTRICTIONS:
+            flags.append("APPLIES_TO_FIRST_LETTER")
+        if name in PLACEHOLDER_RESTRICTIONS:
+            flags.append("APPLIES_TO_PLACEHOLDER")
+        if name in MARKER_RESTRICTIONS:
+            flags.append("APPLIES_TO_MARKER")
+        if name in CUE_RESTRICTIONS:
+            flags.append("APPLIES_TO_CUE")
+        return flags
+
+%>
+
+/// A group for properties which may override each other
+/// via logical resolution.
+#[derive(Clone, Copy, Eq, Hash, PartialEq)]
+pub enum LogicalGroup {
+    % for group in logical_groups.iterkeys():
+    /// ${group}
+    ${to_camel_case(group)},
+    % endfor
+}
+
+
 /// A set of longhand properties
 #[derive(Clone, Copy, Debug, Default, MallocSizeOf, PartialEq)]
 pub struct LonghandIdSet {
     storage: [u32; (${len(data.longhands)} - 1 + 32) / 32]
 }
 
 impl_trivial_to_shmem!(LonghandIdSet);
 
@@ -832,16 +878,39 @@ impl LonghandIdSet {
         // and is None for all other properties.
         ${static_longhand_id_set(
             "HAS_NO_EFFECT_ON_SCROLLBARS",
             lambda p: p.has_effect_on_gecko_scrollbars is False
         )}
         &HAS_NO_EFFECT_ON_SCROLLBARS
     }
 
+    /// Returns the set of padding properties for the purpose of disabling
+    /// native appearance.
+    #[inline]
+    pub fn padding_properties() -> &'static Self {
+        <% assert "padding" in logical_groups %>
+        ${static_longhand_id_set(
+            "PADDING_PROPERTIES",
+            lambda p: p.logical_group == "padding"
+        )}
+        &PADDING_PROPERTIES
+    }
+
+    /// Returns the set of border properties for the purpose of disabling native
+    /// appearance.
+    #[inline]
+    pub fn border_background_properties() -> &'static Self {
+        ${static_longhand_id_set(
+            "BORDER_BACKGROUND_PROPERTIES",
+            lambda p: (p.logical_group and p.logical_group.startswith("border")) or p.name in ["background-color", "background-image"]
+        )}
+        &BORDER_BACKGROUND_PROPERTIES
+    }
+
     /// Iterate over the current longhand id set.
     pub fn iter(&self) -> LonghandIdSetIterator {
         LonghandIdSetIterator { longhands: self, cur: 0, }
     }
 
     /// Returns whether this set contains at least every longhand that `other`
     /// also contains.
     pub fn contains_all(&self, other: &Self) -> bool {
@@ -993,61 +1062,16 @@ bitflags! {
          * they can be checked in the C++ side via ServoCSSPropList.h. */
         /// This property can be animated on the compositor.
         const CAN_ANIMATE_ON_COMPOSITOR = 0;
         /// This shorthand property is accessible from getComputedStyle.
         const SHORTHAND_IN_GETCS = 0;
     }
 }
 
-<%
-    logical_groups = defaultdict(list)
-    for prop in data.longhands:
-        if prop.logical_group:
-            logical_groups[prop.logical_group].append(prop)
-
-    for group, props in logical_groups.iteritems():
-        logical_count = sum(1 for p in props if p.logical)
-        if logical_count * 2 != len(props):
-            raise RuntimeError("Logical group {} has ".format(group) +
-                               "unbalanced logical / physical properties")
-
-    FIRST_LINE_RESTRICTIONS = PropertyRestrictions.first_line(data)
-    FIRST_LETTER_RESTRICTIONS = PropertyRestrictions.first_letter(data)
-    MARKER_RESTRICTIONS = PropertyRestrictions.marker(data)
-    PLACEHOLDER_RESTRICTIONS = PropertyRestrictions.placeholder(data)
-    CUE_RESTRICTIONS = PropertyRestrictions.cue(data)
-
-    def restriction_flags(property):
-        name = property.name
-        flags = []
-        if name in FIRST_LINE_RESTRICTIONS:
-            flags.append("APPLIES_TO_FIRST_LINE")
-        if name in FIRST_LETTER_RESTRICTIONS:
-            flags.append("APPLIES_TO_FIRST_LETTER")
-        if name in PLACEHOLDER_RESTRICTIONS:
-            flags.append("APPLIES_TO_PLACEHOLDER")
-        if name in MARKER_RESTRICTIONS:
-            flags.append("APPLIES_TO_MARKER")
-        if name in CUE_RESTRICTIONS:
-            flags.append("APPLIES_TO_CUE")
-        return flags
-
-%>
-
-/// A group for properties which may override each other
-/// via logical resolution.
-#[derive(Clone, Copy, Eq, Hash, PartialEq)]
-pub enum LogicalGroup {
-    % for group in logical_groups.iterkeys():
-    /// ${group}
-    ${to_camel_case(group)},
-    % endfor
-}
-
 /// An identifier for a given longhand property.
 #[derive(Clone, Copy, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[repr(u16)]
 pub enum LonghandId {
     % for i, property in enumerate(data.longhands):
         /// ${property.name}
         ${property.camel_case} = ${i},
     % endfor
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -2,18 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #![allow(unsafe_code)]
 
 //! The rule tree.
 
 use crate::applicable_declarations::ApplicableDeclarationList;
-#[cfg(feature = "gecko")]
-use crate::gecko::selector_parser::PseudoElement;
 use crate::hash::{self, FxHashMap};
 use crate::properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
 use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use crate::stylesheets::{Origin, StyleRule};
 use crate::thread_state;
 use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
 use parking_lot::RwLock;
 use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow};
@@ -1413,208 +1411,16 @@ impl StrongRuleNode {
 
     unsafe fn maybe_gc(&self) {
         debug_assert!(self.get().is_root(), "Can't call GC on a non-root node!");
         if self.get().free_count().load(Ordering::Relaxed) > RULE_TREE_GC_INTERVAL {
             self.gc();
         }
     }
 
-    /// Returns true if any properties specified by `rule_type_mask` was set by
-    /// an author rule.
-    #[cfg(feature = "gecko")]
-    pub fn has_author_specified_rules<E>(
-        &self,
-        mut element: E,
-        mut pseudo: Option<PseudoElement>,
-        guards: &StylesheetGuards,
-        rule_type_mask: u32,
-        author_colors_allowed: bool,
-    ) -> bool
-    where
-        E: crate::dom::TElement,
-    {
-        use crate::gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BACKGROUND;
-        use crate::gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BORDER;
-        use crate::gecko_bindings::structs::NS_AUTHOR_SPECIFIED_PADDING;
-        use crate::properties::{CSSWideKeyword, LonghandId};
-        use crate::properties::{PropertyDeclaration, PropertyDeclarationId};
-        use std::borrow::Cow;
-
-        // Reset properties:
-        const BACKGROUND_PROPS: &'static [LonghandId] =
-            &[LonghandId::BackgroundColor, LonghandId::BackgroundImage];
-
-        const BORDER_PROPS: &'static [LonghandId] = &[
-            LonghandId::BorderTopColor,
-            LonghandId::BorderTopStyle,
-            LonghandId::BorderTopWidth,
-            LonghandId::BorderRightColor,
-            LonghandId::BorderRightStyle,
-            LonghandId::BorderRightWidth,
-            LonghandId::BorderBottomColor,
-            LonghandId::BorderBottomStyle,
-            LonghandId::BorderBottomWidth,
-            LonghandId::BorderLeftColor,
-            LonghandId::BorderLeftStyle,
-            LonghandId::BorderLeftWidth,
-            LonghandId::BorderTopLeftRadius,
-            LonghandId::BorderTopRightRadius,
-            LonghandId::BorderBottomRightRadius,
-            LonghandId::BorderBottomLeftRadius,
-            LonghandId::BorderInlineStartColor,
-            LonghandId::BorderInlineStartStyle,
-            LonghandId::BorderInlineStartWidth,
-            LonghandId::BorderInlineEndColor,
-            LonghandId::BorderInlineEndStyle,
-            LonghandId::BorderInlineEndWidth,
-            LonghandId::BorderBlockStartColor,
-            LonghandId::BorderBlockStartStyle,
-            LonghandId::BorderBlockStartWidth,
-            LonghandId::BorderBlockEndColor,
-            LonghandId::BorderBlockEndStyle,
-            LonghandId::BorderBlockEndWidth,
-        ];
-
-        const PADDING_PROPS: &'static [LonghandId] = &[
-            LonghandId::PaddingTop,
-            LonghandId::PaddingRight,
-            LonghandId::PaddingBottom,
-            LonghandId::PaddingLeft,
-            LonghandId::PaddingInlineStart,
-            LonghandId::PaddingInlineEnd,
-            LonghandId::PaddingBlockStart,
-            LonghandId::PaddingBlockEnd,
-        ];
-
-        // Set of properties that we are currently interested in.
-        let mut properties = LonghandIdSet::new();
-
-        if rule_type_mask & NS_AUTHOR_SPECIFIED_BACKGROUND != 0 {
-            for id in BACKGROUND_PROPS {
-                properties.insert(*id);
-            }
-        }
-        if rule_type_mask & NS_AUTHOR_SPECIFIED_BORDER != 0 {
-            for id in BORDER_PROPS {
-                properties.insert(*id);
-            }
-        }
-        if rule_type_mask & NS_AUTHOR_SPECIFIED_PADDING != 0 {
-            for id in PADDING_PROPS {
-                properties.insert(*id);
-            }
-        }
-
-        // If author colors are not allowed, don't look at those properties
-        // (except for background-color which is special and we handle below).
-        if !author_colors_allowed {
-            properties.remove_all(LonghandIdSet::ignored_when_colors_disabled());
-            if rule_type_mask & NS_AUTHOR_SPECIFIED_BACKGROUND != 0 {
-                properties.insert(LonghandId::BackgroundColor);
-            }
-        }
-
-        let mut element_rule_node = Cow::Borrowed(self);
-
-        loop {
-            // We need to be careful not to count styles covered up by
-            // user-important or UA-important declarations.  But we do want to
-            // catch explicit inherit styling in those and check our parent
-            // element to see whether we have user styling for those properties.
-            // Note that we don't care here about inheritance due to lack of a
-            // specified value, since all the properties we care about are reset
-            // properties.
-
-            let mut inherited_properties = LonghandIdSet::new();
-            let mut have_explicit_ua_inherit = false;
-
-            for node in element_rule_node.self_and_ancestors() {
-                let source = node.style_source();
-                let declarations = if source.is_some() {
-                    source
-                        .as_ref()
-                        .unwrap()
-                        .read(node.cascade_level().guard(guards))
-                        .declaration_importance_iter()
-                } else {
-                    continue;
-                };
-
-                // Iterate over declarations of the longhands we care about.
-                let node_importance = node.importance();
-                let longhands = declarations.rev().filter_map(|(declaration, importance)| {
-                    if importance != node_importance {
-                        return None;
-                    }
-                    match declaration.id() {
-                        PropertyDeclarationId::Longhand(id) => Some((id, declaration)),
-                        _ => None,
-                    }
-                });
-
-                let is_author = node.cascade_level().origin() == Origin::Author;
-                for (id, declaration) in longhands {
-                    if !properties.contains(id) {
-                        continue;
-                    }
-
-                    if is_author {
-                        if !author_colors_allowed {
-                            if let PropertyDeclaration::BackgroundColor(ref color) = *declaration {
-                                if color.is_transparent() {
-                                    return true;
-                                }
-                                continue;
-                            }
-                        }
-                        return true;
-                    }
-
-                    // This property was set by a non-author rule.
-                    // Stop looking for it in this element's rule
-                    // nodes.
-                    properties.remove(id);
-
-                    // However, if it is inherited, then it might be
-                    // inherited from an author rule from an
-                    // ancestor element's rule nodes.
-                    if declaration.get_css_wide_keyword() == Some(CSSWideKeyword::Inherit) {
-                        have_explicit_ua_inherit = true;
-                        inherited_properties.insert(id);
-                    }
-                }
-            }
-
-            if !have_explicit_ua_inherit {
-                break;
-            }
-
-            // Continue to the parent element and search for the inherited properties.
-            if let Some(pseudo) = pseudo.take() {
-                if pseudo.inherits_from_default_values() {
-                    break;
-                }
-            } else {
-                element = match element.inheritance_parent() {
-                    Some(parent) => parent,
-                    None => break,
-                };
-
-                let parent_data = element.mutate_data().unwrap();
-                let parent_rule_node = parent_data.styles.primary().rules().clone();
-                element_rule_node = Cow::Owned(parent_rule_node);
-            }
-
-            properties = inherited_properties;
-        }
-
-        false
-    }
-
     /// Returns true if there is either animation or transition level rule.
     pub fn has_animation_or_transition_rules(&self) -> bool {
         self.self_and_ancestors()
             .take_while(|node| node.cascade_level() >= CascadeLevel::SMILOverride)
             .any(|node| node.cascade_level().is_animation())
     }
 
     /// Get a set of properties whose CascadeLevel are higher than Animations
--- a/servo/components/style/values/specified/box.rs
+++ b/servo/components/style/values/specified/box.rs
@@ -1625,17 +1625,21 @@ pub enum Appearance {
     /// The label part of a checkbox or radio button, used for painting a focus
     /// outline.
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     CheckboxLabel,
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     RadioLabel,
     /// nsRangeFrame and its subparts
     Range,
-    RangeThumb,
+    RangeThumb, // FIXME: This should not be exposed to content.
+    #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
+    RangeProgress,
+    #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
+    RangeTrack,
     /// The resizer background area in a status bar for the resizer widget in
     /// the corner of a window.
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     Resizerpanel,
     /// The resizer itself.
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     Resizer,
     /// A slider.
@@ -1851,16 +1855,24 @@ pub enum Appearance {
     #[css(skip)]
     FocusOutline,
 
     /// A dummy variant that should be last to let the GTK widget do hackery.
     #[css(skip)]
     Count,
 }
 
+impl Appearance {
+    /// Returns whether we're the `none` value.
+    #[inline]
+    pub fn is_none(self) -> bool {
+        self == Appearance::None
+    }
+}
+
 /// A kind of break between two boxes.
 ///
 /// https://drafts.csswg.org/css-break/#break-between
 #[allow(missing_docs)]
 #[derive(
     Clone,
     Copy,
     Debug,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -3718,41 +3718,16 @@ pub extern "C" fn Servo_SetExplicitStyle
     debug!("Servo_SetExplicitStyle: {:?}", element);
     // We only support this API for initial styling. There's no reason it couldn't
     // work for other things, we just haven't had a reason to do so.
     debug_assert!(element.get_data().is_none());
     let mut data = unsafe { element.ensure_data() };
     data.styles.primary = Some(unsafe { ArcBorrow::from_ref(style) }.clone_arc());
 }
 
-#[no_mangle]
-pub extern "C" fn Servo_HasAuthorSpecifiedRules(
-    raw_data: &RawServoStyleSet,
-    style: &ComputedValues,
-    element: &RawGeckoElement,
-    rule_type_mask: u32,
-) -> bool {
-    let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    let element = GeckoElement(element);
-
-    let guard = (*GLOBAL_STYLE_DATA).shared_lock.read();
-    let guards = StylesheetGuards::same(&guard);
-
-    let pseudo = style.pseudo();
-    let author_colors_allowed = data.stylist.device().use_document_colors();
-
-    style.rules().has_author_specified_rules(
-        element,
-        pseudo,
-        &guards,
-        rule_type_mask,
-        author_colors_allowed,
-    )
-}
-
 fn get_pseudo_style(
     guard: &SharedRwLockReadGuard,
     element: GeckoElement,
     pseudo: &PseudoElement,
     rule_inclusion: RuleInclusion,
     styles: &ElementStyles,
     inherited_styles: Option<&ComputedValues>,
     doc_data: &PerDocumentStyleDataImpl,
--- a/widget/nsNativeTheme.cpp
+++ b/widget/nsNativeTheme.cpp
@@ -287,18 +287,17 @@ bool nsNativeTheme::IsWidgetStyled(nsPre
           aAppearance == StyleAppearance::MenulistTextfield ||
           aAppearance == StyleAppearance::Textfield ||
           aAppearance == StyleAppearance::Textarea ||
           aAppearance == StyleAppearance::Listbox ||
           aAppearance == StyleAppearance::Menulist ||
           aAppearance == StyleAppearance::MenulistButton) &&
          aFrame->GetContent()->IsHTMLElement() &&
          aPresContext->HasAuthorSpecifiedRules(
-             aFrame,
-             NS_AUTHOR_SPECIFIED_BORDER | NS_AUTHOR_SPECIFIED_BACKGROUND);
+             aFrame, NS_AUTHOR_SPECIFIED_BORDER_OR_BACKGROUND);
 }
 
 bool nsNativeTheme::IsDisabled(nsIFrame* aFrame, EventStates aEventStates) {
   if (!aFrame) {
     return false;
   }
 
   nsIContent* content = aFrame->GetContent();