From 6fefe522a393d579805426a7fe71bcf3da87f071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Jun 2017 13:01:44 +0200 Subject: [PATCH] style: Assert we never style a root element from another document. Right now when calling getComputedStyle with an element from another document, we return the style using the pres context of that document, not of the document of the window getComputedStyle was called on, so this holds. But it will stop holding if we ever change this, so assert it doesn't happen. Bug: 1374062 Reviewed-By: Manishearth MozReview-Commit-ID: 3g8yQWWdsen --- components/style/dom.rs | 8 +++++++ components/style/gecko/data.rs | 2 +- components/style/gecko/media_queries.rs | 23 +++++++++++-------- components/style/gecko/values.rs | 2 +- components/style/gecko/wrapper.rs | 7 +++++- components/style/matching.rs | 1 + components/style/properties/gecko.mako.rs | 4 ++-- .../style/properties/longhand/color.mako.rs | 2 +- .../style/properties/longhand/font.mako.rs | 9 +++++--- .../style/properties/properties.mako.rs | 11 +++++---- components/style/stylesheets/document_rule.rs | 2 +- components/style/values/specified/color.rs | 4 ++-- components/style/values/specified/length.rs | 3 +-- ports/geckolib/glue.rs | 4 ++-- 14 files changed, 52 insertions(+), 30 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 545463f4bd0..fdeb2db7fd8 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -14,6 +14,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use data::ElementData; use element_state::ElementState; use font_metrics::FontMetricsProvider; +use media_queries::Device; use properties::{ComputedValues, PropertyDeclarationBlock}; #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue; #[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty; @@ -304,6 +305,13 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Get this element as a node. fn as_node(&self) -> Self::ConcreteNode; + /// A debug-only check that the device's owner doc matches the actual doc + /// we're the root of. + /// + /// Otherwise we may set document-level state incorrectly, like the root + /// font-size used for rem units. + fn owner_doc_matches_for_testing(&self, _: &Device) -> bool { true } + /// Returns the depth of this element in the DOM. fn depth(&self) -> usize { let mut depth = 0; diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 3ca07ff8ae1..20f45d32671 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -46,7 +46,7 @@ impl PerDocumentStyleData { pub fn new(pres_context: RawGeckoPresContextOwned) -> Self { let device = Device::new(pres_context); let quirks_mode = unsafe { - (*(*device.pres_context).mDocument.raw::()).mCompatMode + (*device.pres_context().mDocument.raw::()).mCompatMode }; PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl { diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index ae867edfda0..58dbe0f9c9d 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -14,7 +14,7 @@ use gecko_bindings::bindings; use gecko_bindings::structs::{nsCSSKeyword, nsCSSProps_KTableEntry, nsCSSValue, nsCSSUnit, nsStringBuffer}; use gecko_bindings::structs::{nsMediaExpression_Range, nsMediaFeature}; use gecko_bindings::structs::{nsMediaFeature_ValueType, nsMediaFeature_RangeType, nsMediaFeature_RequirementFlags}; -use gecko_bindings::structs::RawGeckoPresContextOwned; +use gecko_bindings::structs::{nsPresContext, RawGeckoPresContextOwned}; use media_queries::MediaType; use parser::ParserContext; use properties::{ComputedValues, StyleBuilder}; @@ -36,7 +36,7 @@ pub struct Device { /// NB: The pres context lifetime is tied to the styleset, who owns the /// stylist, and thus the `Device`, so having a raw pres context pointer /// here is fine. - pub pres_context: RawGeckoPresContextOwned, + pres_context: RawGeckoPresContextOwned, default_values: Arc, viewport_override: Option, /// The font size of the root element @@ -105,11 +105,16 @@ impl Device { self.root_font_size.store(size.0 as isize, Ordering::Relaxed) } + /// Gets the pres context associated with this document. + pub fn pres_context(&self) -> &nsPresContext { + unsafe { &*self.pres_context } + } + /// Recreates the default computed values. pub fn reset_computed_values(&mut self) { // NB: A following stylesheet flush will populate this if appropriate. self.viewport_override = None; - self.default_values = ComputedValues::default_values(unsafe { &*self.pres_context }); + self.default_values = ComputedValues::default_values(self.pres_context()); self.used_root_font_size.store(false, Ordering::Relaxed); } @@ -134,10 +139,10 @@ impl Device { // FIXME(emilio): Gecko allows emulating random media with // mIsEmulatingMedia / mMediaEmulated . Refactor both sides so that // is supported (probably just making MediaType an Atom). - if (*self.pres_context).mMedium == atom!("screen").as_ptr() { + if self.pres_context().mMedium == atom!("screen").as_ptr() { MediaType::Screen } else { - debug_assert!((*self.pres_context).mMedium == atom!("print").as_ptr()); + debug_assert!(self.pres_context().mMedium == atom!("print").as_ptr()); MediaType::Print } } @@ -150,19 +155,19 @@ impl Device { Au::from_f32_px(v.size.height)) }).unwrap_or_else(|| unsafe { // TODO(emilio): Need to take into account scrollbars. - Size2D::new(Au((*self.pres_context).mVisibleArea.width), - Au((*self.pres_context).mVisibleArea.height)) + let area = &self.pres_context().mVisibleArea; + Size2D::new(Au(area.width), Au(area.height)) }) } /// Returns whether document colors are enabled. pub fn use_document_colors(&self) -> bool { - unsafe { (*self.pres_context).mUseDocumentColors() != 0 } + self.pres_context().mUseDocumentColors() != 0 } /// Returns the default background color. pub fn default_background_color(&self) -> RGBA { - convert_nscolor_to_rgba(unsafe { (*self.pres_context).mBackgroundColor }) + convert_nscolor_to_rgba(self.pres_context().mBackgroundColor) } } diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index cd774901897..f256def2ac3 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -401,7 +401,7 @@ impl CounterStyleOrNone { pub fn to_gecko_value(self, gecko_value: &mut CounterStylePtr, device: &Device) { use gecko_bindings::bindings::Gecko_SetCounterStyleToName as set_name; use gecko_bindings::bindings::Gecko_SetCounterStyleToSymbols as set_symbols; - let pres_context = unsafe { &*device.pres_context }; + let pres_context = device.pres_context(); match self { CounterStyleOrNone::None => unsafe { set_name(gecko_value, atom!("none").into_addrefed(), pres_context); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 42c9997e691..2e7267f0869 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -672,7 +672,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { in_media_query: bool, device: &Device) -> FontMetricsQueryResult { use gecko_bindings::bindings::Gecko_GetFontMetrics; let gecko_metrics = unsafe { - Gecko_GetFontMetrics(&*device.pres_context, + Gecko_GetFontMetrics(device.pres_context(), wm.is_vertical() && !wm.is_sideways(), font.gecko(), font_size.0, @@ -771,6 +771,11 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) } } + fn owner_doc_matches_for_testing(&self, device: &Device) -> bool { + self.as_node().owner_doc() as *const structs::nsIDocument == + device.pres_context().mDocument.raw::() + } + fn style_attribute(&self) -> Option<&Arc>> { if !self.may_have_style_attribute() { return None; diff --git a/components/style/matching.rs b/components/style/matching.rs index 44b02daba48..6640e5e33de 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -274,6 +274,7 @@ trait PrivateMatchMethods: TElement { if self.is_native_anonymous() || cascade_target == CascadeTarget::EagerPseudo { cascade_flags.insert(PROHIBIT_DISPLAY_CONTENTS); } else if self.is_root() { + debug_assert!(self.owner_doc_matches_for_testing(shared_context.stylist.device())); cascade_flags.insert(IS_ROOT_ELEMENT); } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index d62499cdd27..c75ba4d3e68 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1551,7 +1551,7 @@ fn static_assert() { pub fn fixup_none_generic(&mut self, device: &Device) { unsafe { - bindings::Gecko_nsStyleFont_FixupNoneGeneric(&mut self.gecko, &*device.pres_context) + bindings::Gecko_nsStyleFont_FixupNoneGeneric(&mut self.gecko, device.pres_context()) } } @@ -1621,7 +1621,7 @@ fn static_assert() { } pub fn fixup_font_min_size(&mut self, device: &Device) { - unsafe { bindings::Gecko_nsStyleFont_FixupMinFontSize(&mut self.gecko, &*device.pres_context) } + unsafe { bindings::Gecko_nsStyleFont_FixupMinFontSize(&mut self.gecko, device.pres_context()) } } pub fn apply_unconstrained_font_size(&mut self, v: Au) { diff --git a/components/style/properties/longhand/color.mako.rs b/components/style/properties/longhand/color.mako.rs index 7b7db26f021..34860cb6e10 100644 --- a/components/style/properties/longhand/color.mako.rs +++ b/components/style/properties/longhand/color.mako.rs @@ -108,7 +108,7 @@ fn to_computed_value(&self, cx: &Context) -> Self::ComputedValue { unsafe { Gecko_GetLookAndFeelSystemColor(*self as i32, - &*cx.device.pres_context) + cx.device.pres_context()) } } diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index 934acde1a38..c2082af3ef9 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -2388,9 +2388,12 @@ ${helpers.single_keyword("-moz-math-variant", let mut system: nsFont = unsafe { mem::uninitialized() }; unsafe { - bindings::Gecko_nsFont_InitSystem(&mut system, id as i32, - cx.style.get_font().gecko(), - &*cx.device.pres_context) + bindings::Gecko_nsFont_InitSystem( + &mut system, + id as i32, + cx.style.get_font().gecko(), + cx.device.pres_context() + ) } let family = system.fontlist.mFontlist.iter().map(|font| { use properties::longhands::font_family::computed_value::*; diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 1df4615722a..03f9802e019 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2783,13 +2783,14 @@ pub fn apply_declarations<'a, F, I>(device: &Device, // which Gecko just does regular cascading with. Do the same. // This can only happen in the case where the language changed but the family did not if generic != structs::kGenericFont_NONE { - let pres_context = context.device.pres_context; - let gecko_font = context.mutate_style().mutate_font().gecko_mut(); + let gecko_font = context.style.mutate_font().gecko_mut(); gecko_font.mGenericID = generic; unsafe { - bindings::Gecko_nsStyleFont_PrefillDefaultForGeneric(gecko_font, - &*pres_context, - generic); + bindings::Gecko_nsStyleFont_PrefillDefaultForGeneric( + gecko_font, + context.device.pres_context(), + generic + ); } } } diff --git a/components/style/stylesheets/document_rule.rs b/components/style/stylesheets/document_rule.rs index f69062b0049..a4ac747f2a0 100644 --- a/components/style/stylesheets/document_rule.rs +++ b/components/style/stylesheets/document_rule.rs @@ -141,7 +141,7 @@ impl UrlMatchingFunction { UrlMatchingFunction::RegExp(ref pat) => pat, }); unsafe { - Gecko_DocumentRule_UseForPresentation(&*device.pres_context, &*pattern, func) + Gecko_DocumentRule_UseForPresentation(device.pres_context(), &*pattern, func) } } diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index fdf6212151f..b19543c535f 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -254,7 +254,7 @@ impl ToComputedValue for Color { #[cfg(feature = "gecko")] Color::Special(special) => { use self::gecko::SpecialColorKeyword as Keyword; - let pres_context = unsafe { &*_context.device.pres_context }; + let pres_context = _context.device.pres_context(); convert_nscolor_to_computedcolor(match special { Keyword::MozDefaultColor => pres_context.mDefaultColor, Keyword::MozDefaultBackgroundColor => pres_context.mBackgroundColor, @@ -268,7 +268,7 @@ impl ToComputedValue for Color { use dom::TElement; use gecko::wrapper::GeckoElement; use gecko_bindings::bindings::Gecko_GetBody; - let pres_context = unsafe { &*_context.device.pres_context }; + let pres_context = _context.device.pres_context(); let body = unsafe { Gecko_GetBody(pres_context) }; diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index b29c8f47129..61789ac8d0d 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -351,8 +351,7 @@ impl PhysicalLength { const MM_PER_INCH: f32 = 25.4; let physical_inch = unsafe { - let pres_context = &*context.device.pres_context; - bindings::Gecko_GetAppUnitsPerPhysicalInch(&pres_context) + bindings::Gecko_GetAppUnitsPerPhysicalInch(context.device.pres_context()) }; let inch = self.0 / MM_PER_INCH; diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index dbc1624527b..ce643601647 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1597,8 +1597,8 @@ pub extern "C" fn Servo_StyleSet_Drop(data: RawServoStyleSetOwned) { pub extern "C" fn Servo_StyleSet_CompatModeChanged(raw_data: RawServoStyleSetBorrowed) { let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let quirks_mode = unsafe { - (*(*data.stylist.device().pres_context).mDocument - .raw::()).mCompatMode + (*data.stylist.device().pres_context().mDocument.raw::()) + .mCompatMode }; data.stylist.set_quirks_mode(quirks_mode.into());