Bug 1357583: Avoid unconditionally popping under RulesIterator. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 27 May 2017 00:55:52 +0200
changeset 585485 b16f24d0edcab91158239c29f098a59907fe909b
parent 585484 644e4fb2e5c8fd17834e54ac8eee621fd95d976c
child 630733 4923c00d873bb70fb24fb3dc01391246b961da58
push id61125
push userbmo:emilio+bugs@crisal.io
push dateFri, 26 May 2017 23:01:10 +0000
reviewersheycam
bugs1357583
milestone55.0a1
Bug 1357583: Avoid unconditionally popping under RulesIterator. r?heycam Not sure it's worth it, really, but seemed like it could be. Feel free to r- if it doesn't feel like it. MozReview-Commit-ID: HTsBNfEqQy4
servo/components/style/stylesheets.rs
--- a/servo/components/style/stylesheets.rs
+++ b/servo/components/style/stylesheets.rs
@@ -972,66 +972,79 @@ impl<'a, 'b, C> RulesIterator<'a, 'b, C>
 
 impl<'a, 'b, C> Iterator for RulesIterator<'a, 'b, C>
     where 'b: 'a,
           C: NestedRuleIterationCondition + 'static,
 {
     type Item = &'a CssRule;
 
     fn next(&mut self) -> Option<Self::Item> {
-        while let Some(mut nested_iter) = self.stack.pop() {
-             let rule = match nested_iter.next() {
-                Some(r) => r,
-                None => continue,
-            };
-
-            let sub_iter = match *rule {
-                CssRule::Import(ref import_rule) => {
-                    let import_rule = import_rule.read_with(self.guard);
+        let mut pop = false;
+        while !self.stack.is_empty() {
+            if pop {
+                self.stack.pop();
+                pop = false;
+                continue;
+            }
 
-                    if C::process_import(self.guard, self.device, self.quirks_mode, import_rule) {
-                        Some(import_rule.stylesheet.rules.read_with(self.guard).0.iter())
-                    } else {
-                        None
+            let rule;
+            let sub_iter;
+            {
+                let mut nested_iter = self.stack.last_mut().unwrap();
+                rule = match nested_iter.next() {
+                    Some(r) => r,
+                    None => {
+                        pop = true;
+                        continue
                     }
-                }
-                CssRule::Document(ref doc_rule) => {
-                    let doc_rule = doc_rule.read_with(self.guard);
-                    if C::process_document(self.guard, self.device, self.quirks_mode, doc_rule) {
-                        Some(doc_rule.rules.read_with(self.guard).0.iter())
-                    } else {
-                        None
+                };
+
+                sub_iter = match *rule {
+                    CssRule::Import(ref import_rule) => {
+                        let import_rule = import_rule.read_with(self.guard);
+
+                        if C::process_import(self.guard, self.device, self.quirks_mode, import_rule) {
+                            Some(import_rule.stylesheet.rules.read_with(self.guard).0.iter())
+                        } else {
+                            None
+                        }
                     }
-                }
-                CssRule::Media(ref lock) => {
-                    let media_rule = lock.read_with(self.guard);
-                    if C::process_media(self.guard, self.device, self.quirks_mode, media_rule) {
-                        Some(media_rule.rules.read_with(self.guard).0.iter())
-                    } else {
-                        None
+                    CssRule::Document(ref doc_rule) => {
+                        let doc_rule = doc_rule.read_with(self.guard);
+                        if C::process_document(self.guard, self.device, self.quirks_mode, doc_rule) {
+                            Some(doc_rule.rules.read_with(self.guard).0.iter())
+                        } else {
+                            None
+                        }
+                    }
+                    CssRule::Media(ref lock) => {
+                        let media_rule = lock.read_with(self.guard);
+                        if C::process_media(self.guard, self.device, self.quirks_mode, media_rule) {
+                            Some(media_rule.rules.read_with(self.guard).0.iter())
+                        } else {
+                            None
+                        }
                     }
-                }
-                CssRule::Supports(ref lock) => {
-                    let supports_rule = lock.read_with(self.guard);
-                    if C::process_supports(self.guard, self.device, self.quirks_mode, supports_rule) {
-                        Some(supports_rule.rules.read_with(self.guard).0.iter())
-                    } else {
-                        None
+                    CssRule::Supports(ref lock) => {
+                        let supports_rule = lock.read_with(self.guard);
+                        if C::process_supports(self.guard, self.device, self.quirks_mode, supports_rule) {
+                            Some(supports_rule.rules.read_with(self.guard).0.iter())
+                        } else {
+                            None
+                        }
                     }
-                }
-                CssRule::Namespace(_) |
-                CssRule::Style(_) |
-                CssRule::FontFace(_) |
-                CssRule::CounterStyle(_) |
-                CssRule::Viewport(_) |
-                CssRule::Keyframes(_) |
-                CssRule::Page(_) => None,
-            };
-
-            self.stack.push(nested_iter);
+                    CssRule::Namespace(_) |
+                    CssRule::Style(_) |
+                    CssRule::FontFace(_) |
+                    CssRule::CounterStyle(_) |
+                    CssRule::Viewport(_) |
+                    CssRule::Keyframes(_) |
+                    CssRule::Page(_) => None,
+                };
+            }
 
             if let Some(sub_iter) = sub_iter {
                 self.stack.push(sub_iter);
             }
 
             return Some(rule);
         }