From 14e945f09aaee4d73c79363a158654c43c481447 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Tue, 29 Mar 2016 08:11:59 -0700 Subject: [PATCH 1/4] Add a unit test for Fragment size --- components/layout/lib.rs | 3 +++ components/servo/Cargo.lock | 8 ++++++++ components/servo/Cargo.toml | 3 +++ tests/unit/layout/Cargo.toml | 12 ++++++++++++ tests/unit/layout/lib.rs | 7 +++++++ tests/unit/layout/size_of.rs | 26 ++++++++++++++++++++++++++ 6 files changed, 59 insertions(+) create mode 100644 tests/unit/layout/Cargo.toml create mode 100644 tests/unit/layout/lib.rs create mode 100644 tests/unit/layout/size_of.rs diff --git a/components/layout/lib.rs b/components/layout/lib.rs index fa8a2231265..4f1d9abdac1 100644 --- a/components/layout/lib.rs +++ b/components/layout/lib.rs @@ -100,3 +100,6 @@ mod text; mod traversal; mod webrender_helpers; mod wrapper; + +// For unit tests: +pub use fragment::Fragment; diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index df4759a0ca3..b2b61d0ee09 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -22,6 +22,7 @@ dependencies = [ "ipc-channel 0.2.1 (git+https://github.com/servo/ipc-channel)", "layers 0.2.4 (git+https://github.com/servo/rust-layers)", "layout 0.0.1", + "layout_tests 0.0.1", "libc 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "msg 0.0.1", @@ -1083,6 +1084,13 @@ dependencies = [ "webrender_traits 0.1.0 (git+https://github.com/servo/webrender_traits)", ] +[[package]] +name = "layout_tests" +version = "0.0.1" +dependencies = [ + "layout 0.0.1", +] + [[package]] name = "layout_traits" version = "0.0.1" diff --git a/components/servo/Cargo.toml b/components/servo/Cargo.toml index c547e09a071..b7cafca6af9 100644 --- a/components/servo/Cargo.toml +++ b/components/servo/Cargo.toml @@ -24,6 +24,9 @@ image = "0.7" [dev-dependencies.gfx_tests] path = "../../tests/unit/gfx" +[dev-dependencies.layout_tests] +path = "../../tests/unit/layout" + [dev-dependencies.net_tests] path = "../../tests/unit/net" diff --git a/tests/unit/layout/Cargo.toml b/tests/unit/layout/Cargo.toml new file mode 100644 index 00000000000..a080eace5a7 --- /dev/null +++ b/tests/unit/layout/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "layout_tests" +version = "0.0.1" +authors = ["The Servo Project Developers"] + +[lib] +name = "layout_tests" +path = "lib.rs" +doctest = false + +[dependencies.layout] +path = "../../../components/layout" diff --git a/tests/unit/layout/lib.rs b/tests/unit/layout/lib.rs new file mode 100644 index 00000000000..59092bf4d9a --- /dev/null +++ b/tests/unit/layout/lib.rs @@ -0,0 +1,7 @@ +/* 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/. */ + +extern crate layout; + +#[cfg(all(test, target_pointer_width = "64"))] mod size_of; diff --git a/tests/unit/layout/size_of.rs b/tests/unit/layout/size_of.rs new file mode 100644 index 00000000000..a404a3f7d81 --- /dev/null +++ b/tests/unit/layout/size_of.rs @@ -0,0 +1,26 @@ +/* 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/. */ + +use layout::Fragment; +use std::mem::size_of; + +#[test] +fn test_size_of_fragment() { + let expected = 184; + let actual = size_of::(); + + if actual < expected { + panic!("Your changes have decreased the stack size of layout::fragment::Fragment \ + from {} to {}. Good work! Please update the size in tests/layout/size_of.rs", + expected, actual); + } + + if actual > expected { + panic!("Your changes have increased the stack size of layout::fragment::Fragment \ + from {} to {}. Please consider choosing a design which avoids this increase. \ + If you feel that the increase is necessary, update the size in \ + tests/layout/size_of.rs.", + expected, actual); + } +} From 31261f045ee908ce14d63bd88283559627bdb227 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Mon, 28 Mar 2016 22:03:25 -0700 Subject: [PATCH 2/4] Put UnscannedTextFragmentInfo in a Box This reduces the size of the SpecificFragmentInfo enum from 48 to 24. --- components/layout/construct.rs | 12 ++++++------ components/layout/fragment.rs | 6 +++--- components/layout/generated_content.rs | 3 ++- components/layout/text.rs | 4 ++-- tests/unit/layout/size_of.rs | 2 +- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index c964501116c..dd6d5ff9930 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -578,7 +578,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> // Add whitespace results. They will be stripped out later on when // between block elements, and retained when between inline elements. let fragment_info = SpecificFragmentInfo::UnscannedText( - UnscannedTextFragmentInfo::new(" ".to_owned(), None)); + box UnscannedTextFragmentInfo::new(" ".to_owned(), None)); properties::modify_style_for_replaced_content(&mut whitespace_style); properties::modify_style_for_text(&mut whitespace_style); let fragment = Fragment::from_opaque_node_and_style(whitespace_node, @@ -715,7 +715,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> match text_content { TextContent::Text(string) => { - let info = UnscannedTextFragmentInfo::new(string, selection); + let info = box UnscannedTextFragmentInfo::new(string, selection); let specific_fragment_info = SpecificFragmentInfo::UnscannedText(info); fragments.fragments.push_back(Fragment::from_opaque_node_and_style( node.opaque(), @@ -728,7 +728,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> for content_item in content_items.into_iter() { let specific_fragment_info = match content_item { ContentItem::String(string) => { - let info = UnscannedTextFragmentInfo::new(string, None); + let info = box UnscannedTextFragmentInfo::new(string, None); SpecificFragmentInfo::UnscannedText(info) } content_item => { @@ -859,7 +859,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> whitespace_damage)) => { // Instantiate the whitespace fragment. let fragment_info = SpecificFragmentInfo::UnscannedText( - UnscannedTextFragmentInfo::new(" ".to_owned(), None)); + box UnscannedTextFragmentInfo::new(" ".to_owned(), None)); properties::modify_style_for_replaced_content(&mut whitespace_style); properties::modify_style_for_text(&mut whitespace_style); let fragment = Fragment::from_opaque_node_and_style(whitespace_node, @@ -1241,7 +1241,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> unscanned_marker_fragments.push_back(Fragment::new( node, SpecificFragmentInfo::UnscannedText( - UnscannedTextFragmentInfo::new(text, None)))); + box UnscannedTextFragmentInfo::new(text, None)))); let marker_fragments = TextRunScanner::new().scan_for_runs( &mut self.layout_context.font_context(), unscanned_marker_fragments); @@ -1848,7 +1848,7 @@ fn control_chars_to_fragment(node: &InlineFragmentNodeInfo, restyle_damage: RestyleDamage) -> Fragment { let info = SpecificFragmentInfo::UnscannedText( - UnscannedTextFragmentInfo::new(String::from(text), None)); + box UnscannedTextFragmentInfo::new(String::from(text), None)); let mut style = node.style.clone(); properties::modify_style_for_text(&mut style); Fragment::from_opaque_node_and_style(node.address, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index ba7ff2f714e..021efa31a08 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -167,7 +167,7 @@ pub enum SpecificFragmentInfo { TableWrapper, Multicol, MulticolColumn, - UnscannedText(UnscannedTextFragmentInfo), + UnscannedText(Box), } impl SpecificFragmentInfo { @@ -896,8 +896,8 @@ impl Fragment { let mut unscanned_ellipsis_fragments = LinkedList::new(); unscanned_ellipsis_fragments.push_back(self.transform( self.border_box.size, - SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::new("…".to_owned(), - None)))); + SpecificFragmentInfo::UnscannedText( + box UnscannedTextFragmentInfo::new("…".to_owned(), None)))); let ellipsis_fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(), unscanned_ellipsis_fragments); debug_assert!(ellipsis_fragments.len() == 1); diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index 7af9c4349d2..9cf40b0711f 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -434,7 +434,8 @@ fn render_text(layout_context: &LayoutContext, string: String) -> Option { let mut fragments = LinkedList::new(); - let info = SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::new(string, None)); + let info = SpecificFragmentInfo::UnscannedText( + box UnscannedTextFragmentInfo::new(string, None)); fragments.push_back(Fragment::from_opaque_node_and_style(node, pseudo, style, diff --git a/components/layout/text.rs b/components/layout/text.rs index 14b0d138113..049aaabee7e 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -476,8 +476,8 @@ fn split_first_fragment_at_newline_if_necessary(fragments: &mut LinkedList(); if actual < expected { From f0f0265139f6ae5b82b2600d961524d2d6cc3f62 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Tue, 29 Mar 2016 08:09:39 -0700 Subject: [PATCH 3/4] Unbox IframeFragmentInfo. --- components/layout/construct.rs | 2 +- components/layout/fragment.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index dd6d5ff9930..f92a4211fcb 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -297,7 +297,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let specific_fragment_info = match node.type_id() { Some(NodeTypeId::Element(ElementTypeId::HTMLElement( HTMLElementTypeId::HTMLIFrameElement))) => { - SpecificFragmentInfo::Iframe(box IframeFragmentInfo::new(node)) + SpecificFragmentInfo::Iframe(IframeFragmentInfo::new(node)) } Some(NodeTypeId::Element(ElementTypeId::HTMLElement( HTMLElementTypeId::HTMLImageElement))) => { diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 021efa31a08..e5e31959607 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -145,7 +145,7 @@ pub enum SpecificFragmentInfo { /// content resolution phase (e.g. an ordered list item marker). GeneratedContent(Box), - Iframe(Box), + Iframe(IframeFragmentInfo), Image(Box), Canvas(Box), From 33b9aa49dbb9323009dc87de45ac378cd8036061 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Tue, 29 Mar 2016 08:16:04 -0700 Subject: [PATCH 4/4] Update path in script_tests::size_of error messages --- tests/unit/script/size_of.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index 1550ca558d1..fc9cf6f09bf 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -23,14 +23,14 @@ macro_rules! sizeof_checker ( let old = $known_size; if new < old { panic!("Your changes have decreased the stack size of commonly used DOM struct {} from {} to {}. \ - Good work! Please update the size in script/tests.rs", + Good work! Please update the size in tests/unit/script/size_of.rs.", stringify!($t), old, new) } else if new > old { panic!("Your changes have increased the stack size of commonly used DOM struct {} from {} to {}. \ These structs are present in large quantities in the DOM, and increasing the size \ may dramatically affect our memory footprint. Please consider choosing a design which \ avoids this increase. If you feel that the increase is necessary, \ - update to the new size in script/tests.rs.", + update to the new size in tests/unit/script/size_of.rs.", stringify!($t), old, new) } });