style: Simplify visited-related code in invalidation.

We match with AllLinksVisitedAndUnvisited for style invalidation, and we already
do a subtree restyle because :visited matching doesn't depend on the actual
element state.

So all this stuff is just not needed. The comment points to the attribute tests
in bug 1328509, but those still trivially pass with this change.

I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or
maybe since https://github.com/servo/servo/pull/19520. In any case it doesn't
really matter, and I already had done this cleanup in my WIP patches for
bug 1406622, but I guess this is a slightly more suitable place to land them :)

Differential Revision: https://phabricator.services.mozilla.com/D3305
This commit is contained in:
Emilio Cobos Álvarez 2018-08-14 10:49:21 +02:00
parent 87b1e1cdc9
commit a0cb37d29d
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C

View file

@ -24,12 +24,6 @@ use selectors::matching::matches_selector;
use smallvec::SmallVec;
use stylesheets::origin::{Origin, OriginSet};
#[derive(Debug, PartialEq)]
enum VisitedDependent {
Yes,
No,
}
/// The collector implementation.
struct Collector<'a, 'b: 'a, 'selectors: 'a, E>
where
@ -352,7 +346,7 @@ where
if let Some(ref id) = removed_id {
if let Some(deps) = map.id_to_selector.get(id, quirks_mode) {
for dep in deps {
self.scan_dependency(dep, VisitedDependent::No);
self.scan_dependency(dep);
}
}
}
@ -361,7 +355,7 @@ where
if let Some(ref id) = added_id {
if let Some(deps) = map.id_to_selector.get(id, quirks_mode) {
for dep in deps {
self.scan_dependency(dep, VisitedDependent::No);
self.scan_dependency(dep);
}
}
}
@ -369,7 +363,7 @@ where
for class in self.classes_added.iter().chain(self.classes_removed.iter()) {
if let Some(deps) = map.class_to_selector.get(class, quirks_mode) {
for dep in deps {
self.scan_dependency(dep, VisitedDependent::No);
self.scan_dependency(dep);
}
}
}
@ -395,7 +389,7 @@ where
self.removed_id,
self.classes_removed,
|dependency| {
self.scan_dependency(dependency, VisitedDependent::No);
self.scan_dependency(dependency);
true
},
);
@ -415,98 +409,53 @@ where
if !dependency.state.intersects(state_changes) {
return true;
}
let visited_dependent = if dependency
.state
.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE)
{
VisitedDependent::Yes
} else {
VisitedDependent::No
};
self.scan_dependency(&dependency.dep, visited_dependent);
self.scan_dependency(&dependency.dep);
true
},
);
}
/// Check whether a dependency should be taken into account, using a given
/// visited handling mode.
/// Check whether a dependency should be taken into account.
fn check_dependency(
&mut self,
visited_handling_mode: VisitedHandlingMode,
dependency: &Dependency,
) -> bool {
let element = &self.element;
let wrapper = &self.wrapper;
self.matching_context
.with_visited_handling_mode(visited_handling_mode, |mut context| {
let matches_now = matches_selector(
&dependency.selector,
dependency.selector_offset,
None,
element,
&mut context,
&mut |_, _| {},
);
let matches_now = matches_selector(
&dependency.selector,
dependency.selector_offset,
None,
element,
&mut self.matching_context,
&mut |_, _| {},
);
let matched_then = matches_selector(
&dependency.selector,
dependency.selector_offset,
None,
wrapper,
&mut context,
&mut |_, _| {},
);
let matched_then = matches_selector(
&dependency.selector,
dependency.selector_offset,
None,
wrapper,
&mut self.matching_context,
&mut |_, _| {},
);
matched_then != matches_now
})
matched_then != matches_now
}
fn scan_dependency(
&mut self,
dependency: &'selectors Dependency,
is_visited_dependent: VisitedDependent,
) {
fn scan_dependency(&mut self, dependency: &'selectors Dependency) {
debug!(
"TreeStyleInvalidator::scan_dependency({:?}, {:?}, {:?})",
self.element, dependency, is_visited_dependent,
"TreeStyleInvalidator::scan_dependency({:?}, {:?})",
self.element, dependency
);
if !self.dependency_may_be_relevant(dependency) {
return;
}
let should_account_for_dependency =
self.check_dependency(VisitedHandlingMode::AllLinksVisitedAndUnvisited, dependency);
if should_account_for_dependency {
if self.check_dependency(dependency) {
return self.note_dependency(dependency);
}
// If there is a relevant link, then we also matched in visited
// mode.
//
// Match again in this mode to ensure this also matches.
//
// Note that we never actually match directly against the element's true
// visited state at all, since that would expose us to timing attacks.
//
// The matching process only considers the relevant link state and
// visited handling mode when deciding if visited matches. Instead, we
// are rematching here in case there is some :visited selector whose
// matching result changed for some other state or attribute change of
// this element (for example, for things like [foo]:visited).
//
// NOTE: This thing is actually untested because testing it is flaky,
// see the tests that were added and then backed out in bug 1328509.
if is_visited_dependent == VisitedDependent::Yes && self.element.is_link() {
let should_account_for_dependency =
self.check_dependency(VisitedHandlingMode::RelevantLinkVisited, dependency);
if should_account_for_dependency {
return self.note_dependency(dependency);
}
}
}
fn note_dependency(&mut self, dependency: &'selectors Dependency) {