From 5104d8244f6936bd5b78bb6b4ced86219237d6c8 Mon Sep 17 00:00:00 2001
From: Matt Brubeck <mbrubeck@limpet.net>
Date: Fri, 11 Mar 2016 08:34:30 -0800
Subject: [PATCH] Don't re-resolve already-resolved generated content

This fixes #7846, a failure in the "quotes-036.htm" test. Servo lays out this
test correctly in its initial layout, but then messes it up in any relayout
(whether it's an incremental or full layout).

The problem is that the ResolveGeneratedContent traversal is not safe to run
more than once on the same flow. It mutates some GeneratedContent fragments
into ScannedText fragments, but leaves others unmodified (in particular,
those that generate empty content). The next time layout runs, these remaining
GeneratedContent fragments are processed *again* but with an incorrect correct
quote nesting level (because some of the surrounding GeneratedContent
fragments are gone).

This patch ensures that each GeneratedContent fragment is resolved only once.
---
 components/layout/fragment.rs                        |  5 ++++-
 components/layout/generated_content.rs               | 12 +++++++++---
 components/layout/inline.rs                          |  6 ++----
 components/layout/lib.rs                             |  1 +
 .../metadata-css/css21_dev/html4/quotes-036.htm.ini  |  3 ---
 5 files changed, 16 insertions(+), 11 deletions(-)
 delete mode 100644 tests/wpt/metadata-css/css21_dev/html4/quotes-036.htm.ini

diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
index db3a42a0669..ef7d3008e17 100644
--- a/components/layout/fragment.rs
+++ b/components/layout/fragment.rs
@@ -256,6 +256,8 @@ fn clamp_size(size: Au,
 pub enum GeneratedContentInfo {
     ListItem,
     ContentItem(ContentItem),
+    /// Placeholder for elements with generated content that did not generate any fragments.
+    Empty,
 }
 
 /// A hypothetical box (see CSS 2.1 § 10.3.7) for an absolutely-positioned block that was declared
@@ -1291,8 +1293,9 @@ impl Fragment {
     }
 
     /// Returns true if and only if this fragment is a generated content fragment.
-    pub fn is_generated_content(&self) -> bool {
+    pub fn is_unscanned_generated_content(&self) -> bool {
         match self.specific {
+            SpecificFragmentInfo::GeneratedContent(box GeneratedContentInfo::Empty) => false,
             SpecificFragmentInfo::GeneratedContent(..) => true,
             _ => false,
         }
diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs
index d6a2149237c..b0284c62d7e 100644
--- a/components/layout/generated_content.rs
+++ b/components/layout/generated_content.rs
@@ -180,6 +180,7 @@ impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> {
                                                                list_style_type,
                                                                RenderingMode::Suffix(".\u{00a0}"))
                 }
+                GeneratedContentInfo::Empty |
                 GeneratedContentInfo::ContentItem(ContentItem::String(_)) => {
                     // Nothing to do here.
                 }
@@ -242,9 +243,14 @@ impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> {
             }
         };
 
-        if let Some(new_info) = new_info {
-            fragment.specific = new_info
-        }
+        fragment.specific = match new_info {
+            Some(new_info) => new_info,
+            // If the fragment did not generate any content, replace it with a no-op placeholder
+            // so that it isn't processed again on the next layout.  FIXME (mbrubeck): When
+            // processing an inline flow, this traversal should be allowed to insert or remove
+            // fragments.  Then we can just remove these fragments rather than adding placeholders.
+            None => SpecificFragmentInfo::GeneratedContent(box GeneratedContentInfo::Empty)
+        };
     }
 
     fn reset_and_increment_counters_as_necessary(&mut self, fragment: &mut Fragment) {
diff --git a/components/layout/inline.rs b/components/layout/inline.rs
index 1212f8a3cfc..1045613b934 100644
--- a/components/layout/inline.rs
+++ b/components/layout/inline.rs
@@ -846,10 +846,8 @@ impl InlineFlow {
             first_line_indentation: Au(0),
         };
 
-        for fragment in &flow.fragments.fragments {
-            if fragment.is_generated_content() {
-                flow.base.restyle_damage.insert(RESOLVE_GENERATED_CONTENT)
-            }
+        if flow.fragments.fragments.iter().any(Fragment::is_unscanned_generated_content) {
+            flow.base.restyle_damage.insert(RESOLVE_GENERATED_CONTENT);
         }
 
         flow
diff --git a/components/layout/lib.rs b/components/layout/lib.rs
index 174125c3fd9..fa8a2231265 100644
--- a/components/layout/lib.rs
+++ b/components/layout/lib.rs
@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #![feature(as_unsafe_cell)]
+#![feature(box_patterns)]
 #![feature(box_syntax)]
 #![feature(custom_derive)]
 #![feature(mpsc_select)]
diff --git a/tests/wpt/metadata-css/css21_dev/html4/quotes-036.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/quotes-036.htm.ini
deleted file mode 100644
index f79479fde90..00000000000
--- a/tests/wpt/metadata-css/css21_dev/html4/quotes-036.htm.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[quotes-036.htm]
-  type: reftest
-  expected: FAIL