servo: Merge #11039 - Remove redundant code in whitespace stripping (from mbrubeck:strip-trailing); r=pcwalton
authorMatt Brubeck <mbrubeck@limpet.net>
Fri, 06 May 2016 21:53:27 -0700
changeset 338735 dedce28d65cdc7637e315beb82c821d5d70fa394
parent 338734 fe163b3c5571892d915049189f7e40073d3916ee
child 338736 2a005f0d9bca51286b7988b88c8bad63d8e6e822
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspcwalton
servo: Merge #11039 - Remove redundant code in whitespace stripping (from mbrubeck:strip-trailing); r=pcwalton LineBreaker calls Fragment::strip_trailing_whitespace_if_necessary and then recalculates the fragment's inline size. But this isn't necessary because strip_trailing_whitespace_if_necessary already recalculates the size. r? @pcwalton Source-Repo: https://github.com/servo/servo Source-Revision: f80641e7a16b832ce755cf936ed9ea6a9289fda9
servo/components/layout/fragment.rs
servo/components/layout/inline.rs
--- a/servo/components/layout/fragment.rs
+++ b/servo/components/layout/fragment.rs
@@ -2398,26 +2398,21 @@ impl Fragment {
                         trailing_whitespace_start_byte = i + c.len_utf8();
                         break;
                     }
                 }
                 let whitespace_start = ByteIndex(trailing_whitespace_start_byte as isize);
                 let whitespace_len = scanned_text_fragment_info.range.length() - whitespace_start;
                 let whitespace_range = Range::new(whitespace_start, whitespace_len);
 
-                // FIXME: This may be unnecessary because these metrics will be recomputed in
-                // LineBreaker::strip_trailing_whitespace_from_pending_line_if_necessary
                 let text_bounds = scanned_text_fragment_info.run
                                                         .metrics_for_range(&whitespace_range)
                                                         .bounding_box;
-                self.border_box.size.inline = self.border_box.size.inline -
-                    text_bounds.size.width;
-
-                scanned_text_fragment_info.content_size.inline =
-                    scanned_text_fragment_info.content_size.inline - text_bounds.size.width;
+                self.border_box.size.inline -= text_bounds.size.width;
+                scanned_text_fragment_info.content_size.inline -= text_bounds.size.width;
 
                 scanned_text_fragment_info.range.extend_by(-whitespace_len);
                 WhitespaceStrippingResult::RetainFragment
             }
             SpecificFragmentInfo::UnscannedText(ref mut unscanned_text_fragment_info) => {
                 let mut trailing_bidi_control_characters_to_retain = Vec::new();
                 let (mut modified, mut last_character_index) = (true, 0);
                 for (i, character) in unscanned_text_fragment_info.text.char_indices().rev() {
--- a/servo/components/layout/inline.rs
+++ b/servo/components/layout/inline.rs
@@ -387,37 +387,22 @@ impl LineBreaker {
     /// flushing it.
     fn strip_trailing_whitespace_from_pending_line_if_necessary(&mut self) {
         if self.pending_line.range.is_empty() {
             return
         }
         let last_fragment_index = self.pending_line.range.end() - FragmentIndex(1);
         let mut fragment = &mut self.new_fragments[last_fragment_index.get() as usize];
 
-        let mut old_fragment_inline_size = None;
-        if let SpecificFragmentInfo::ScannedText(_) = fragment.specific {
-            old_fragment_inline_size = Some(fragment.border_box.size.inline +
-                                            fragment.margin.inline_start_end());
-        }
+        let old_fragment_inline_size = fragment.border_box.size.inline;
 
         fragment.strip_trailing_whitespace_if_necessary();
 
-        if let SpecificFragmentInfo::ScannedText(ref mut scanned_text_fragment_info) =
-                fragment.specific {
-            let scanned_text_fragment_info = &mut **scanned_text_fragment_info;
-            let range = &mut scanned_text_fragment_info.range;
-
-            scanned_text_fragment_info.content_size.inline =
-                scanned_text_fragment_info.run.metrics_for_range(range).advance_width;
-            fragment.border_box.size.inline = scanned_text_fragment_info.content_size.inline +
-                fragment.border_padding.inline_start_end();
-            self.pending_line.bounds.size.inline = self.pending_line.bounds.size.inline -
-                (old_fragment_inline_size.unwrap() -
-                 (fragment.border_box.size.inline + fragment.margin.inline_start_end()));
-        }
+        self.pending_line.bounds.size.inline +=
+            fragment.border_box.size.inline - old_fragment_inline_size;
     }
 
     // FIXME(eatkinson): this assumes that the tallest fragment in the line determines the line
     // block-size. This might not be the case with some weird text fonts.
     fn new_inline_metrics_for_line(&self, new_fragment: &Fragment, layout_context: &LayoutContext)
                                    -> InlineMetrics {
         if !new_fragment.is_vertically_aligned_to_top_or_bottom() {
             let fragment_inline_metrics = new_fragment.inline_metrics(layout_context);