Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Stop generating flows under display: none.
Because this is a bottom-up traversal it can generates flows and throw them away. To prevent that, this cascades an internal `-servo-under-display-none` property and then checks that during flow construction.  Fixes #1536.
  • Loading branch information
mbrubeck committed May 25, 2016
1 parent 116faa7 commit 5c09e26
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 10 deletions.
19 changes: 10 additions & 9 deletions components/layout/construct.rs
Expand Up @@ -1501,7 +1501,15 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS
//
// TODO: This should actually consult the table in that section to get the
// final computed value for 'display'.
fn process(&mut self, node: &ConcreteThreadSafeLayoutNode) -> bool {
fn process(&mut self, node: &ConcreteThreadSafeLayoutNode) {
node.insert_flags(HAS_NEWLY_CONSTRUCTED_FLOW);

// Bail out if this node has an ancestor with display: none.
if node.style(self.style_context()).get_inheritedbox()._servo_under_display_none.0 {
self.set_flow_construction_result(node, ConstructionResult::None);
return;
}

// Get the `display` property for this node, and determine whether this node is floated.
let (display, float, positioning) = match node.type_id() {
None => {
Expand Down Expand Up @@ -1542,12 +1550,8 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS

// Switch on display and floatedness.
match (display, float, positioning) {
// `display: none` contributes no flow construction result. Nuke the flow construction
// results of children.
// `display: none` contributes no flow construction result.
(display::T::none, _, _) => {
for child in node.children() {
self.set_flow_construction_result(&child, ConstructionResult::None);
}
self.set_flow_construction_result(node, ConstructionResult::None);
}

Expand Down Expand Up @@ -1654,9 +1658,6 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS
self.set_flow_construction_result(node, construction_result)
}
}

node.insert_flags(HAS_NEWLY_CONSTRUCTED_FLOW);
true
}
}

Expand Down
1 change: 1 addition & 0 deletions components/layout/incremental.rs
Expand Up @@ -178,6 +178,7 @@ pub fn compute_damage(old: Option<&Arc<ServoComputedValues>>, new: &ServoCompute
], [
get_box.float, get_box.display, get_box.position, get_counters.content,
get_counters.counter_reset, get_counters.counter_increment,
get_inheritedbox._servo_under_display_none,
get_list.quotes, get_list.list_style_type,

// If these text or font properties change, we need to reconstruct the flow so that
Expand Down
2 changes: 1 addition & 1 deletion components/layout/traversal.rs
Expand Up @@ -72,7 +72,7 @@ impl<'lc, 'ln> DomTraversalContext<ServoLayoutNode<'ln>> for RecalcStyleAndConst
/// A bottom-up, parallelizable traversal.
pub trait PostorderNodeMutTraversal<ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> {
/// The operation to perform. Return true to continue or false to stop.
fn process(&mut self, node: &ConcreteThreadSafeLayoutNode) -> bool;
fn process(&mut self, node: &ConcreteThreadSafeLayoutNode);
}

/// The flow construction traversal, which builds flows for styled nodes.
Expand Down
1 change: 1 addition & 0 deletions components/style/properties/longhand/box.mako.rs
Expand Up @@ -78,6 +78,7 @@
_error_reporter: &mut StdBox<ParseErrorReporter + Send>) {
longhands::_servo_display_for_hypothetical_box::derive_from_display(context);
longhands::_servo_text_decorations_in_effect::derive_from_display(context);
longhands::_servo_under_display_none::derive_from_display(context);
}
% endif
Expand Down
37 changes: 37 additions & 0 deletions components/style/properties/longhand/inherited_box.mako.rs
Expand Up @@ -88,3 +88,40 @@ ${helpers.single_keyword("color-adjust", "economy exact", products="gecko")}
}
}
</%helpers:longhand>

// Used in the bottom-up flow construction traversal to avoid constructing flows for
// descendants of nodes with `display: none`.
<%helpers:longhand name="-servo-under-display-none" derived_from="display" products="servo">
use cssparser::ToCss;
use std::fmt;
use values::computed::ComputedValueAsSpecified;

#[derive(Copy, Clone, Debug, Eq, PartialEq, HeapSizeOf, Serialize, Deserialize)]
pub struct SpecifiedValue(pub bool);

pub mod computed_value {
pub type T = super::SpecifiedValue;
}
impl ComputedValueAsSpecified for SpecifiedValue {}

pub fn get_initial_value() -> computed_value::T {
SpecifiedValue(false)
}

impl ToCss for SpecifiedValue {
fn to_css<W>(&self, _: &mut W) -> fmt::Result where W: fmt::Write {
Ok(()) // Internal property
}
}

#[inline]
pub fn derive_from_display<Cx: TContext>(context: &mut Cx) {
use properties::style_struct_traits::Box;
use super::display::computed_value::T as Display;

if context.style().get_box().clone_display() == Display::none {
context.mutate_style().mutate_inheritedbox()
.set__servo_under_display_none(SpecifiedValue(true));
}
}
</%helpers:longhand>

0 comments on commit 5c09e26

Please sign in to comment.