Bug 1394297 - style: Don't mutate style structs when attempting to store the same value. r?emilio
MozReview-Commit-ID: 2ha3VyT4wHd
--- a/layout/style/CounterStyleManager.h
+++ b/layout/style/CounterStyleManager.h
@@ -232,16 +232,21 @@ public:
bool operator!() const { return !mRaw; }
bool operator==(const CounterStylePtr& aOther) const
{ return mRaw == aOther.mRaw; }
bool operator!=(const CounterStylePtr& aOther) const
{ return mRaw != aOther.mRaw; }
bool IsResolved() const { return !IsUnresolved(); }
inline void Resolve(CounterStyleManager* aManager);
+ nsIAtom* AsAtom() const
+ {
+ MOZ_ASSERT(IsUnresolved());
+ return reinterpret_cast<nsIAtom*>(mRaw & ~eMask);
+ }
private:
CounterStyle* Get() const
{
MOZ_ASSERT(IsResolved());
return reinterpret_cast<CounterStyle*>(mRaw & ~eMask);
}
template<typename T>
@@ -261,21 +266,16 @@ private:
eAnonymousCounterStyle = 1,
eUnresolvedAtom = 2,
eMask = 3,
};
Type GetType() const { return static_cast<Type>(mRaw & eMask); }
bool IsUnresolved() const { return GetType() == eUnresolvedAtom; }
bool IsAnonymous() const { return GetType() == eAnonymousCounterStyle; }
- nsIAtom* AsAtom()
- {
- MOZ_ASSERT(IsUnresolved());
- return reinterpret_cast<nsIAtom*>(mRaw & ~eMask);
- }
AnonymousCounterStyle* AsAnonymous()
{
MOZ_ASSERT(IsAnonymous());
return static_cast<AnonymousCounterStyle*>(
reinterpret_cast<CounterStyle*>(mRaw & ~eMask));
}
void Reset()
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1449,57 +1449,80 @@ Gecko_SetCounterStyleToString(CounterSty
void
Gecko_CopyCounterStyle(CounterStylePtr* aDst, const CounterStylePtr* aSrc)
{
*aDst = *aSrc;
}
bool
-Gecko_CounterStyle_IsNone(const CounterStylePtr* aPtr) {
+Gecko_CounterStyle_IsUnresolved(const CounterStylePtr* aPtr)
+{
+ MOZ_ASSERT(aPtr);
+ return !aPtr->IsResolved();
+}
+
+bool
+Gecko_CounterStyle_IsNone(const CounterStylePtr* aPtr)
+{
MOZ_ASSERT(aPtr);
return (*aPtr)->IsNone();
}
bool
-Gecko_CounterStyle_IsName(const CounterStylePtr* aPtr) {
- return !Gecko_CounterStyle_IsNone(aPtr) && !(*aPtr)->AsAnonymous();
+Gecko_CounterStyle_IsName(const CounterStylePtr* aPtr)
+{
+ return aPtr->IsResolved() &&
+ !Gecko_CounterStyle_IsNone(aPtr) &&
+ !(*aPtr)->AsAnonymous();
}
void
Gecko_CounterStyle_GetName(const CounterStylePtr* aPtr,
- nsAString* aResult) {
+ nsAString* aResult)
+{
MOZ_ASSERT(Gecko_CounterStyle_IsName(aPtr));
(*aPtr)->GetStyleName(*aResult);
}
+nsIAtom*
+Gecko_CounterStyle_GetAtom(const CounterStylePtr* aPtr)
+{
+ MOZ_ASSERT(!aPtr->IsResolved());
+ return do_AddRef(aPtr->AsAtom()).take();
+}
+
const nsTArray<nsString>&
-Gecko_CounterStyle_GetSymbols(const CounterStylePtr* aPtr) {
+Gecko_CounterStyle_GetSymbols(const CounterStylePtr* aPtr)
+{
MOZ_ASSERT((*aPtr)->AsAnonymous());
AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
return anonymous->GetSymbols();
}
uint8_t
-Gecko_CounterStyle_GetSystem(const CounterStylePtr* aPtr) {
+Gecko_CounterStyle_GetSystem(const CounterStylePtr* aPtr)
+{
MOZ_ASSERT((*aPtr)->AsAnonymous());
AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
return anonymous->GetSystem();
}
bool
-Gecko_CounterStyle_IsSingleString(const CounterStylePtr* aPtr) {
+Gecko_CounterStyle_IsSingleString(const CounterStylePtr* aPtr)
+{
MOZ_ASSERT(aPtr);
AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
return anonymous ? anonymous->IsSingleString() : false;
}
void
Gecko_CounterStyle_GetSingleString(const CounterStylePtr* aPtr,
- nsAString* aResult) {
+ nsAString* aResult)
+{
MOZ_ASSERT(Gecko_CounterStyle_IsSingleString(aPtr));
const nsTArray<nsString>& symbols = Gecko_CounterStyle_GetSymbols(aPtr);
MOZ_ASSERT(symbols.Length() == 1);
aResult->Assign(symbols[0]);
}
already_AddRefed<css::URLValue>
ServoBundledURI::IntoCssUrl()
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -327,20 +327,22 @@ void Gecko_SetCounterStyleToName(mozilla
void Gecko_SetCounterStyleToSymbols(mozilla::CounterStylePtr* ptr,
uint8_t symbols_type,
nsACString const* const* symbols,
uint32_t symbols_count);
void Gecko_SetCounterStyleToString(mozilla::CounterStylePtr* ptr,
const nsACString* symbol);
void Gecko_CopyCounterStyle(mozilla::CounterStylePtr* dst,
const mozilla::CounterStylePtr* src);
+bool Gecko_CounterStyle_IsUnresolved(const mozilla::CounterStylePtr* ptr);
bool Gecko_CounterStyle_IsNone(const mozilla::CounterStylePtr* ptr);
bool Gecko_CounterStyle_IsName(const mozilla::CounterStylePtr* ptr);
void Gecko_CounterStyle_GetName(const mozilla::CounterStylePtr* ptr,
nsAString* result);
+nsIAtom* Gecko_CounterStyle_GetAtom(const mozilla::CounterStylePtr* ptr);
const nsTArray<nsString>& Gecko_CounterStyle_GetSymbols(const mozilla::CounterStylePtr* ptr);
uint8_t Gecko_CounterStyle_GetSystem(const mozilla::CounterStylePtr* ptr);
bool Gecko_CounterStyle_IsSingleString(const mozilla::CounterStylePtr* ptr);
void Gecko_CounterStyle_GetSingleString(const mozilla::CounterStylePtr* ptr,
nsAString* result);
// background-image style.
void Gecko_SetNullImageValue(nsStyleImage* image);
--- a/servo/components/style/gecko/values.rs
+++ b/servo/components/style/gecko/values.rs
@@ -1,16 +1,17 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#![allow(unsafe_code)]
//! Different kind of helpers to interact with Gecko values.
+use Atom;
use app_units::Au;
use counter_style::Symbol;
use cssparser::RGBA;
use gecko_bindings::structs::{CounterStylePtr, nsStyleCoord};
use gecko_bindings::structs::{StyleGridTrackBreadth, StyleShapeRadius};
use gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
use media_queries::Device;
use nsstring::{nsACString, nsCString};
@@ -474,26 +475,33 @@ impl CounterStyleOrNone {
symbols.as_ptr(), symbols.len() as u32) };
}
}
}
/// Convert Gecko CounterStylePtr to CounterStyleOrNone.
pub fn from_gecko_value(gecko_value: &CounterStylePtr) -> Self {
use counter_style::{Symbol, Symbols};
+ use gecko_bindings::bindings::Gecko_CounterStyle_GetAtom;
use gecko_bindings::bindings::Gecko_CounterStyle_GetName;
use gecko_bindings::bindings::Gecko_CounterStyle_GetSymbols;
use gecko_bindings::bindings::Gecko_CounterStyle_GetSystem;
use gecko_bindings::bindings::Gecko_CounterStyle_IsName;
use gecko_bindings::bindings::Gecko_CounterStyle_IsNone;
+ use gecko_bindings::bindings::Gecko_CounterStyle_IsUnresolved;
use values::CustomIdent;
use values::generics::SymbolsType;
if unsafe { Gecko_CounterStyle_IsNone(gecko_value) } {
CounterStyleOrNone::None
+ } else if unsafe { Gecko_CounterStyle_IsUnresolved(gecko_value) } {
+ let atom = unsafe {
+ Atom::from_addrefed(Gecko_CounterStyle_GetAtom(gecko_value))
+ };
+ CounterStyleOrNone::Name(CustomIdent(atom))
} else if unsafe { Gecko_CounterStyle_IsName(gecko_value) } {
ns_auto_string!(name);
unsafe { Gecko_CounterStyle_GetName(gecko_value, &mut *name) };
CounterStyleOrNone::Name(CustomIdent((&*name).into()))
} else {
let system = unsafe { Gecko_CounterStyle_GetSystem(gecko_value) };
let symbol_type = SymbolsType::from_gecko_keyword(system as u32);
let symbols = unsafe {
--- a/servo/components/style/properties/longhand/box.mako.rs
+++ b/servo/components/style/properties/longhand/box.mako.rs
@@ -671,17 +671,16 @@
gecko_enum_prefix="PlaybackDirection",
custom_consts=animation_direction_custom_consts,
extra_prefixes="moz webkit",
spec="https://drafts.csswg.org/css-animations/#propdef-animation-direction",
allowed_in_keyframe_block=False)}
${helpers.single_keyword("animation-play-state",
"running paused",
- need_clone=True,
need_index=True,
animation_value_type="none",
vector=True,
extra_prefixes="moz webkit",
spec="https://drafts.csswg.org/css-animations/#propdef-animation-play-state",
allowed_in_keyframe_block=False)}
${helpers.single_keyword("animation-fill-mode",
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2698,47 +2698,91 @@ impl<'a> StyleBuilder<'a> {
% if property.ident == "content":
self.flags.insert(::properties::computed_value_flags::INHERITS_CONTENT);
% endif
% if property.ident == "display":
self.flags.insert(::properties::computed_value_flags::INHERITS_DISPLAY);
% endif
+ % if property.need_clone:
+ if self.${property.style_struct.ident}.get_if_mutated().is_none() {
+ % if property.style_struct.inherited:
+ // Since this is an inherited property, there is no need to
+ // check what value we have; copying the inherited value will
+ // never have an effect.
+ return;
+ % else:
+ // Check if setting this value would have no effect.
+ if self.${property.style_struct.ident}.clone_${property.ident}() ==
+ inherited_struct.clone_${property.ident}() {
+ return;
+ }
+ % endif
+ }
+ % endif
+
self.${property.style_struct.ident}.mutate()
.copy_${property.ident}_from(
inherited_struct,
% if property.logical:
self.writing_mode,
% endif
);
}
/// Reset `${property.ident}` to the initial value.
#[allow(non_snake_case)]
pub fn reset_${property.ident}(&mut self) {
let reset_struct =
self.reset_style.get_${property.style_struct.name_lower}();
+ % if property.need_clone:
+ if self.${property.style_struct.ident}.get_if_mutated().is_none() {
+ % if not property.style_struct.inherited:
+ // Since this is a non-inherited property, there is no need to
+ // check what value we have; copying the reset value will
+ // never have an effect.
+ return;
+ % else:
+ // Check if setting this value would have no effect.
+ if self.${property.style_struct.ident}.clone_${property.ident}() ==
+ reset_struct.clone_${property.ident}() {
+ return;
+ }
+ % endif
+ }
+ % endif
+
self.${property.style_struct.ident}.mutate()
.reset_${property.ident}(
reset_struct,
% if property.logical:
self.writing_mode,
% endif
);
}
% if not property.is_vector:
/// Set the `${property.ident}` to the computed value `value`.
#[allow(non_snake_case)]
pub fn set_${property.ident}(
&mut self,
value: longhands::${property.ident}::computed_value::T
) {
+ % if property.need_clone:
+ // FIXME(heycam): We only skip logical properties because we
+ // don't have clone functions for them that take a WritingMode.
+ if self.${property.style_struct.ident}.get_if_mutated().is_none() {
+ if self.${property.style_struct.ident}.clone_${property.ident}() == value {
+ return;
+ }
+ }
+ % endif
+
<% props_need_device = ["content", "list_style_type", "font_variant_alternates"] %>
self.${property.style_struct.ident}.mutate()
.set_${property.ident}(
value,
% if property.logical:
self.writing_mode,
% elif product == "gecko" and property.ident in props_need_device:
self.device,