Skip to content

Commit 05a26a2

Browse files
author
bors-servo
authored
Auto merge of servo#16967 - emilio:after, r=heycam,emilio
Bug 1366144: Correctly diff ::before and ::after pseudo-element styles if there's no generated content. r=heycam <!-- Reviewable:start --> This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16967) <!-- Reviewable:end -->
2 parents 4f0b24a + b30d48b commit 05a26a2

File tree

15 files changed

+691
-226
lines changed

15 files changed

+691
-226
lines changed

components/layout/animation.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@ pub fn recalc_style_for_animations(context: &LayoutContext,
160160
animation,
161161
&mut fragment.style,
162162
&ServoMetricsProvider);
163-
damage |= RestyleDamage::compute(&old_style, &fragment.style);
163+
let difference =
164+
RestyleDamage::compute_style_difference(&old_style, &fragment.style);
165+
damage |= difference.damage;
164166
}
165167
}
166168
});

components/layout_thread/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ impl LayoutThread {
11001100
let el = node.as_element().unwrap();
11011101
if let Some(mut d) = element.mutate_data() {
11021102
if d.has_styles() {
1103-
d.ensure_restyle().hint.insert(&StoredRestyleHint::subtree());
1103+
d.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
11041104
}
11051105
}
11061106
if let Some(p) = el.parent_element() {
@@ -1136,7 +1136,7 @@ impl LayoutThread {
11361136
if needs_dirtying {
11371137
if let Some(mut d) = element.mutate_data() {
11381138
if d.has_styles() {
1139-
d.ensure_restyle().hint.insert(&StoredRestyleHint::subtree());
1139+
d.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
11401140
}
11411141
}
11421142
}
@@ -1184,7 +1184,7 @@ impl LayoutThread {
11841184
let mut restyle_data = style_data.ensure_restyle();
11851185

11861186
// Stash the data on the element for processing by the style system.
1187-
restyle_data.hint.insert(&restyle.hint.into());
1187+
restyle_data.hint.insert(restyle.hint.into());
11881188
restyle_data.damage = restyle.damage;
11891189
debug!("Noting restyle for {:?}: {:?}", el, restyle_data);
11901190
}

components/script/dom/document.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ use std::rc::Rc;
131131
use std::time::{Duration, Instant};
132132
use style::attr::AttrValue;
133133
use style::context::{QuirksMode, ReflowGoal};
134-
use style::restyle_hints::{RestyleHint, RESTYLE_SELF, RESTYLE_STYLE_ATTRIBUTE};
134+
use style::restyle_hints::{RestyleHint, RestyleReplacements, RESTYLE_STYLE_ATTRIBUTE};
135135
use style::selector_parser::{RestyleDamage, Snapshot};
136136
use style::shared_lock::SharedRwLock as StyleSharedRwLock;
137137
use style::str::{HTML_SPACE_CHARACTERS, split_html_space_chars, str_join};
@@ -2361,14 +2361,14 @@ impl Document {
23612361
entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document()));
23622362
}
23632363
if attr.local_name() == &local_name!("style") {
2364-
entry.hint |= RESTYLE_STYLE_ATTRIBUTE;
2364+
entry.hint.insert(RestyleHint::for_replacements(RESTYLE_STYLE_ATTRIBUTE));
23652365
}
23662366

23672367
// FIXME(emilio): This should become something like
23682368
// element.is_attribute_mapped(attr.local_name()).
23692369
if attr.local_name() == &local_name!("width") ||
23702370
attr.local_name() == &local_name!("height") {
2371-
entry.hint |= RESTYLE_SELF;
2371+
entry.hint.insert(RestyleHint::for_self());
23722372
}
23732373

23742374
let mut snapshot = entry.snapshot.as_mut().unwrap();

components/script/dom/element.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ use style::context::{QuirksMode, ReflowGoal};
102102
use style::element_state::*;
103103
use style::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
104104
use style::properties::longhands::{self, background_image, border_spacing, font_family, font_size, overflow_x};
105-
use style::restyle_hints::RESTYLE_SELF;
105+
use style::restyle_hints::RestyleHint;
106106
use style::rule_tree::CascadeLevel;
107107
use style::selector_parser::{NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser};
108108
use style::shared_lock::{SharedRwLock, Locked};
@@ -245,7 +245,7 @@ impl Element {
245245

246246
// FIXME(bholley): I think we should probably only do this for
247247
// NodeStyleDamaged, but I'm preserving existing behavior.
248-
restyle.hint |= RESTYLE_SELF;
248+
restyle.hint.insert(RestyleHint::for_self());
249249

250250
if damage == NodeDamage::OtherNodeDamage {
251251
restyle.damage = RestyleDamage::rebuild_and_reflow();

components/style/data.rs

+25-24
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use context::SharedStyleContext;
1010
use dom::TElement;
1111
use properties::ComputedValues;
1212
use properties::longhands::display::computed_value as display;
13-
use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
13+
use restyle_hints::{RestyleReplacements, RestyleHint};
1414
use rule_tree::StrongRuleNode;
1515
use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage};
1616
use shared_lock::StylesheetGuards;
@@ -198,21 +198,18 @@ impl StoredRestyleHint {
198198
// In the middle of an animation only restyle, we don't need to
199199
// propagate any restyle hints, and we need to remove ourselves.
200200
if traversal_flags.for_animation_only() {
201-
self.0.remove(RestyleHint::for_animations());
201+
self.0.remove_animation_hints();
202202
return Self::empty();
203203
}
204204

205-
debug_assert!(!self.0.intersects(RestyleHint::for_animations()),
205+
debug_assert!(!self.0.has_animation_hint(),
206206
"There should not be any animation restyle hints \
207207
during normal traversal");
208208

209209
// Else we should clear ourselves, and return the propagated hint.
210-
let hint = mem::replace(&mut self.0, RestyleHint::empty());
211-
StoredRestyleHint(if hint.contains(RESTYLE_DESCENDANTS) {
212-
RESTYLE_SELF | RESTYLE_DESCENDANTS
213-
} else {
214-
RestyleHint::empty()
215-
})
210+
let new_hint = mem::replace(&mut self.0, RestyleHint::empty())
211+
.propagate_for_non_animation_restyle();
212+
StoredRestyleHint(new_hint)
216213
}
217214

218215
/// Creates an empty `StoredRestyleHint`.
@@ -223,25 +220,25 @@ impl StoredRestyleHint {
223220
/// Creates a restyle hint that forces the whole subtree to be restyled,
224221
/// including the element.
225222
pub fn subtree() -> Self {
226-
StoredRestyleHint(RESTYLE_SELF | RESTYLE_DESCENDANTS)
223+
StoredRestyleHint(RestyleHint::subtree())
227224
}
228225

229226
/// Creates a restyle hint that forces the element and all its later
230227
/// siblings to have their whole subtrees restyled, including the elements
231228
/// themselves.
232229
pub fn subtree_and_later_siblings() -> Self {
233-
StoredRestyleHint(RESTYLE_SELF | RESTYLE_DESCENDANTS | RESTYLE_LATER_SIBLINGS)
230+
StoredRestyleHint(RestyleHint::subtree_and_later_siblings())
234231
}
235232

236233
/// Returns true if the hint indicates that our style may be invalidated.
237234
pub fn has_self_invalidations(&self) -> bool {
238-
self.0.intersects(RestyleHint::for_self())
235+
self.0.affects_self()
239236
}
240237

241238
/// Returns true if the hint indicates that our sibling's style may be
242239
/// invalidated.
243240
pub fn has_sibling_invalidations(&self) -> bool {
244-
self.0.intersects(RESTYLE_LATER_SIBLINGS)
241+
self.0.affects_later_siblings()
245242
}
246243

247244
/// Whether the restyle hint is empty (nothing requires to be restyled).
@@ -250,13 +247,18 @@ impl StoredRestyleHint {
250247
}
251248

252249
/// Insert another restyle hint, effectively resulting in the union of both.
253-
pub fn insert(&mut self, other: &Self) {
254-
self.0 |= other.0
250+
pub fn insert(&mut self, other: Self) {
251+
self.0.insert(other.0)
252+
}
253+
254+
/// Insert another restyle hint, effectively resulting in the union of both.
255+
pub fn insert_from(&mut self, other: &Self) {
256+
self.0.insert_from(&other.0)
255257
}
256258

257259
/// Returns true if the hint has animation-only restyle.
258260
pub fn has_animation_hint(&self) -> bool {
259-
self.0.intersects(RestyleHint::for_animations())
261+
self.0.has_animation_hint()
260262
}
261263
}
262264

@@ -356,7 +358,7 @@ pub enum RestyleKind {
356358
MatchAndCascade,
357359
/// We need to recascade with some replacement rule, such as the style
358360
/// attribute, or animation rules.
359-
CascadeWithReplacements(RestyleHint),
361+
CascadeWithReplacements(RestyleReplacements),
360362
/// We only need to recascade, for example, because only inherited
361363
/// properties in the parent changed.
362364
CascadeOnly,
@@ -381,7 +383,7 @@ impl ElementData {
381383
context.traversal_flags);
382384

383385
let mut hint = match self.get_restyle() {
384-
Some(r) => r.hint.0,
386+
Some(r) => r.hint.0.clone(),
385387
None => RestyleHint::empty(),
386388
};
387389

@@ -393,7 +395,7 @@ impl ElementData {
393395
element.implemented_pseudo_element());
394396

395397
if element.has_snapshot() && !element.handled_snapshot() {
396-
hint |= context.stylist.compute_restyle_hint(&element, context.snapshot_map);
398+
hint.insert(context.stylist.compute_restyle_hint(&element, context.snapshot_map));
397399
unsafe { element.set_handled_snapshot() }
398400
debug_assert!(element.handled_snapshot());
399401
}
@@ -402,8 +404,7 @@ impl ElementData {
402404

403405
// If the hint includes a directive for later siblings, strip it out and
404406
// notify the caller to modify the base hint for future siblings.
405-
let later_siblings = hint.contains(RESTYLE_LATER_SIBLINGS);
406-
hint.remove(RESTYLE_LATER_SIBLINGS);
407+
let later_siblings = hint.remove_later_siblings_hint();
407408

408409
// Insert the hint, overriding the previous hint. This effectively takes
409410
// care of removing the later siblings restyle hint.
@@ -445,13 +446,13 @@ impl ElementData {
445446
debug_assert!(self.restyle.is_some());
446447
let restyle_data = self.restyle.as_ref().unwrap();
447448

448-
let hint = restyle_data.hint.0;
449-
if hint.contains(RESTYLE_SELF) {
449+
let hint = &restyle_data.hint.0;
450+
if hint.match_self() {
450451
return RestyleKind::MatchAndCascade;
451452
}
452453

453454
if !hint.is_empty() {
454-
return RestyleKind::CascadeWithReplacements(hint);
455+
return RestyleKind::CascadeWithReplacements(hint.replacements);
455456
}
456457

457458
debug_assert!(restyle_data.recascade,

components/style/gecko/generated/bindings.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,8 @@ extern "C" {
961961
}
962962
extern "C" {
963963
pub fn Gecko_CalcStyleDifference(oldstyle: *mut nsStyleContext,
964-
newstyle: ServoComputedValuesBorrowed)
964+
newstyle: ServoComputedValuesBorrowed,
965+
any_style_changed: *mut bool)
965966
-> nsChangeHint;
966967
}
967968
extern "C" {

components/style/gecko/restyle_damage.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use gecko_bindings::bindings;
88
use gecko_bindings::structs;
99
use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
1010
use gecko_bindings::sugar::ownership::FFIArcHelpers;
11+
use matching::{StyleChange, StyleDifference};
1112
use properties::ComputedValues;
1213
use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
1314
use stylearc::Arc;
@@ -38,22 +39,27 @@ impl GeckoRestyleDamage {
3839
self.0 == nsChangeHint(0)
3940
}
4041

41-
/// Computes a change hint given an old style (in the form of a
42-
/// `nsStyleContext`, and a new style (in the form of `ComputedValues`).
42+
/// Computes the `StyleDifference` (including the appropriate change hint)
43+
/// given an old style (in the form of a `nsStyleContext`, and a new style
44+
/// (in the form of `ComputedValues`).
4345
///
4446
/// Note that we could in theory just get two `ComputedValues` here and diff
4547
/// them, but Gecko has an interesting optimization when they mark accessed
4648
/// structs, so they effectively only diff structs that have ever been
4749
/// accessed from layout.
48-
pub fn compute(source: &nsStyleContext,
49-
new_style: &Arc<ComputedValues>) -> Self {
50+
pub fn compute_style_difference(source: &nsStyleContext,
51+
new_style: &Arc<ComputedValues>)
52+
-> StyleDifference {
5053
// TODO(emilio): Const-ify this?
5154
let context = source as *const nsStyleContext as *mut nsStyleContext;
55+
let mut any_style_changed: bool = false;
5256
let hint = unsafe {
5357
bindings::Gecko_CalcStyleDifference(context,
54-
new_style.as_borrowed_opt().unwrap())
58+
new_style.as_borrowed_opt().unwrap(),
59+
&mut any_style_changed)
5560
};
56-
GeckoRestyleDamage(hint)
61+
let change = if any_style_changed { StyleChange::Changed } else { StyleChange::Unchanged };
62+
StyleDifference::new(GeckoRestyleDamage(hint), change)
5763
}
5864

5965
/// Returns true if this restyle damage contains all the damage of |other|.

0 commit comments

Comments
 (0)