Avoid stack overflow by not using recursion to add the important rules. (Bug 439184.) r=fantasai sr=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Sun, 13 Jul 2008 13:57:38 -0700
changeset 15902 a00c120d4a6e06d98392980a30a4754435b1b1bf
parent 15901 c6ecab56cb4ff1e76f9b2bef455b6eac07ae6cbb
child 15903 5b6fa5bcaccd17c95bafe39667f1b0e90817e9d4
push id592
push userdbaron@mozilla.com
push dateSun, 13 Jul 2008 20:58:08 +0000
treeherdermozilla-central@6a513bc88338 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfantasai, bzbarsky
bugs439184
milestone1.9.1a1pre
Avoid stack overflow by not using recursion to add the important rules. (Bug 439184.) r=fantasai sr=bzbarsky
layout/style/nsStyleSet.cpp
layout/style/test/Makefile.in
layout/style/test/test_cascade.html
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -423,60 +423,68 @@ nsStyleSet::GetContext(nsPresContext* aP
 
   return result;
 }
 
 void
 nsStyleSet::AddImportantRules(nsRuleNode* aCurrLevelNode,
                               nsRuleNode* aLastPrevLevelNode)
 {
-  if (!aCurrLevelNode || aCurrLevelNode == aLastPrevLevelNode)
+  if (!aCurrLevelNode)
     return;
 
-  AddImportantRules(aCurrLevelNode->GetParent(), aLastPrevLevelNode);
+  nsAutoTArray<nsCOMPtr<nsIStyleRule>, 16> importantRules;
+  for (nsRuleNode *node = aCurrLevelNode; node != aLastPrevLevelNode;
+       node = node->GetParent()) {
+    nsIStyleRule *rule = node->GetRule();
+    nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
+    if (cssRule) {
+      nsCOMPtr<nsIStyleRule> impRule = cssRule->GetImportantRule();
+      if (impRule)
+        importantRules.AppendElement(impRule);
+    }
+  }
 
-  nsIStyleRule *rule = aCurrLevelNode->GetRule();
-  nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
-  if (cssRule) {
-    nsCOMPtr<nsIStyleRule> impRule = cssRule->GetImportantRule();
-    if (impRule)
-      mRuleWalker->Forward(impRule);
+  for (PRUint32 i = importantRules.Length(); i-- != 0; ) {
+    mRuleWalker->Forward(importantRules[i]);
   }
 }
 
 #ifdef DEBUG
 void
 nsStyleSet::AssertNoImportantRules(nsRuleNode* aCurrLevelNode,
                                    nsRuleNode* aLastPrevLevelNode)
 {
-  if (!aCurrLevelNode || aCurrLevelNode == aLastPrevLevelNode)
+  if (!aCurrLevelNode)
     return;
 
-  AssertNoImportantRules(aCurrLevelNode->GetParent(), aLastPrevLevelNode);
-
-  nsIStyleRule *rule = aCurrLevelNode->GetRule();
-  nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
-  if (cssRule) {
-    nsCOMPtr<nsIStyleRule> impRule = cssRule->GetImportantRule();
-    NS_ASSERTION(!impRule, "Unexpected important rule");
+  for (nsRuleNode *node = aCurrLevelNode; node != aLastPrevLevelNode;
+       node = node->GetParent()) {
+    nsIStyleRule *rule = node->GetRule();
+    nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
+    if (cssRule) {
+      nsCOMPtr<nsIStyleRule> impRule = cssRule->GetImportantRule();
+      NS_ASSERTION(!impRule, "Unexpected important rule");
+    }
   }
 }
 
 void
 nsStyleSet::AssertNoCSSRules(nsRuleNode* aCurrLevelNode,
                              nsRuleNode* aLastPrevLevelNode)
 {
-  if (!aCurrLevelNode || aCurrLevelNode == aLastPrevLevelNode)
+  if (!aCurrLevelNode)
     return;
 
-  AssertNoCSSRules(aCurrLevelNode->GetParent(), aLastPrevLevelNode);
-
-  nsIStyleRule *rule = aCurrLevelNode->GetRule();
-  nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
-  NS_ASSERTION(!cssRule || !cssRule->Selector(), "Unexpected CSS rule");
+  for (nsRuleNode *node = aCurrLevelNode; node != aLastPrevLevelNode;
+       node = node->GetParent()) {
+    nsIStyleRule *rule = node->GetRule();
+    nsCOMPtr<nsICSSStyleRule> cssRule(do_QueryInterface(rule));
+    NS_ASSERTION(!cssRule || !cssRule->Selector(), "Unexpected CSS rule");
+  }
 }
 #endif
 
 // Enumerate the rules in a way that cares about the order of the rules.
 void
 nsStyleSet::FileRules(nsIStyleRuleProcessor::EnumFunc aCollectorFunc, 
                       RuleProcessorData* aData)
 {
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -89,16 +89,17 @@ GARBAGE += css_properties.js
 		test_bug389464.html \
 		test_bug391034.html \
 		test_bug391221.html \
 		test_bug397427.html \
 		test_bug401046.html \
 		test_bug405818.html \
 		test_bug412901.html \
 		test_bug437915.html \
+		test_cascade.html \
 		test_compute_data_with_start_struct.html \
 		test_dont_use_document_colors.html \
 		test_inherit_storage.html \
 		test_inherit_computation.html \
 		test_initial_storage.html \
 		test_initial_computation.html \
 		test_of_type_selectors.xhtml \
 		test_parse_rule.html \
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_cascade.html
@@ -0,0 +1,106 @@
+<!DOCTYPE HTML>
+<!-- vim: set shiftwidth=4 tabstop=8 autoindent expandtab: -->
+<!-- ***** BEGIN LICENSE BLOCK *****
+   - Version: MPL 1.1/GPL 2.0/LGPL 2.1
+   -
+   - The contents of this file are subject to the Mozilla Public License Version
+   - 1.1 (the "License"); you may not use this file except in compliance with
+   - the License. You may obtain a copy of the License at
+   - http://www.mozilla.org/MPL/
+   -
+   - Software distributed under the License is distributed on an "AS IS" basis,
+   - WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+   - for the specific language governing rights and limitations under the
+   - License.
+   -
+   - The Original Code is test_cascade.html
+   -
+   - The Initial Developer of the Original Code is the Mozilla Foundation.
+   - Portions created by the Initial Developer are Copyright (C) 2008
+   - the Initial Developer. All Rights Reserved.
+   -
+   - Contributor(s):
+   -   L. David Baron <dbaron@dbaron.org>, Mozilla Corporation (original author)
+   -
+   - Alternatively, the contents of this file may be used under the terms of
+   - either the GNU General Public License Version 2 or later (the "GPL"), or
+   - the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+   - in which case the provisions of the GPL or the LGPL are applicable instead
+   - of those above. If you wish to allow use of your version of this file only
+   - under the terms of either the GPL or the LGPL, and not to allow others to
+   - use your version of this file under the terms of the MPL, indicate your
+   - decision by deleting the provisions above and replace them with the notice
+   - and other provisions required by the LGPL or the GPL. If you do not delete
+   - the provisions above, a recipient may use your version of this file under
+   - the terms of any one of the MPL, the GPL or the LGPL.
+   -
+   - ***** END LICENSE BLOCK ***** -->
+<html>
+<head>
+  <title>Test for Author style sheet aspects of CSS cascading</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <style type="text/css">
+
+  </style>
+</head>
+<body id="thebody">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
+<div class="content_class" id="content" style="position:relative"></div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Author style sheet aspects of CSS cascading **/
+
+var style_element = document.createElement("style");
+var style_contents = document.createTextNode("");
+style_element.appendChild(style_contents);
+document.getElementsByTagName("head")[0].appendChild(style_element);
+
+var div = document.getElementById("content");
+var cs = window.getComputedStyle(div, "");
+var zindex = 0;
+
+/**
+ * Given the selectors |sel1| and |sel2|, in that order (the "order"
+ * aspect of the cascade), with declarations that are !important if
+ * |imp1|/|imp2| are true, assert that the one that wins in the
+ * cascading order is given by |winning| (which must be either 1 or 2).
+ */
+function do_test(sel1, imp1, sel2, imp2, winning) {
+  var ind1 = ++zindex;
+  var ind2 = ++zindex;
+  style_contents.data =
+    sel1 + " { z-index: " + ind1 + (imp1 ? "!important" :"") + " } " +
+    sel2 + " { z-index: " + ind2 + (imp2 ? "!important" :"") + " } ";
+  var result = cs.zIndex;
+  is(result, (winning == 1) ? ind1 : ind2,
+     "cascading of " + style_element.data);
+}
+
+// Test order, and order combined with !important
+do_test("div", false, "div", false, 2);
+do_test("div", false, "div", true, 2);
+do_test("div", true, "div", false, 1);
+do_test("div", true, "div", true, 2);
+
+// Test specificity on a single element
+do_test("div", false, "div.content_class", false, 2);
+do_test("div.content_class", false, "div", false, 1);
+
+// Test specificity across elements
+do_test("body#thebody div", false, "body div.content_class", false, 1);
+do_test("body div.content_class", false, "body#thebody div", false, 2);
+
+// Test specificity combined with !important
+do_test("div.content_class", false, "div", false, 1);
+do_test("div.content_class", true, "div", false, 1);
+do_test("div.content_class", false, "div", true, 2);
+do_test("div.content_class", true, "div", true, 1);
+
+</script>
+</pre>
+</body>
+</html>
+