author | L. David Baron <dbaron@dbaron.org> |
Mon, 02 May 2011 18:43:44 -0700 | |
changeset 68894 | 064d7c5425a6b70e0e7d4f3c6aebdf812334b65e |
parent 68893 | e089a54230bf71965c9c74bb0e3aa9771fd40247 |
child 68895 | 86248f7209b70f8a381320075fd2ea7aa858d201 |
push id | 19788 |
push user | dbaron@mozilla.com |
push date | Tue, 03 May 2011 01:45:09 +0000 |
treeherder | mozilla-central@86248f7209b7 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | bzbarsky |
bugs | 634373 |
milestone | 6.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/layout/style/StyleRule.cpp +++ b/layout/style/StyleRule.cpp @@ -1373,20 +1373,23 @@ StyleRule::DeclarationChanged(Declaratio StyleRule* clone = new StyleRule(*this, aDecl); if (!clone) { return nsnull; } NS_ADDREF(clone); // for return if (aHandleContainer) { - NS_ASSERTION(mSheet, "rule must be in a sheet"); if (mParentRule) { - mSheet->ReplaceRuleInGroup(mParentRule, this, clone); - } else { + if (mSheet) { + mSheet->ReplaceRuleInGroup(mParentRule, this, clone); + } else { + mParentRule->ReplaceStyleRule(this, clone); + } + } else if (mSheet) { mSheet->ReplaceStyleRule(this, clone); } } return clone; } /* virtual */ void
--- a/layout/style/nsCSSRules.cpp +++ b/layout/style/nsCSSRules.cpp @@ -539,16 +539,17 @@ GroupRule::GroupRule(const GroupRule& aC : Rule(aCopy) { const_cast<GroupRule&>(aCopy).mRules.EnumerateForwards(CloneRuleInto, &mRules); mRules.EnumerateForwards(SetParentRuleReference, this); } GroupRule::~GroupRule() { + NS_ABORT_IF_FALSE(!mSheet, "SetStyleSheet should have been called"); mRules.EnumerateForwards(SetParentRuleReference, nsnull); if (mRuleCollection) { mRuleCollection->DropReference(); } } IMPL_STYLE_RULE_INHERIT_MAP_RULE_INFO_INTO(GroupRule, Rule) @@ -1903,17 +1904,19 @@ nsCSSKeyframeRule::SetKeyText(const nsAS nsTArray<float> newSelectors; // FIXME: pass filename and line number if (parser.ParseKeyframeSelectorString(aKeyText, nsnull, 0, newSelectors)) { newSelectors.SwapElements(mKeys); } else { // for now, we don't do anything if the parse fails } - mSheet->SetModifiedByChildRule(); + if (mSheet) { + mSheet->SetModifiedByChildRule(); + } return NS_OK; } NS_IMETHODIMP nsCSSKeyframeRule::GetStyle(nsIDOMCSSStyleDeclaration** aStyle) { if (!mDOMDeclaration) { @@ -1923,17 +1926,19 @@ nsCSSKeyframeRule::GetStyle(nsIDOMCSSSty return NS_OK; } void nsCSSKeyframeRule::ChangeDeclaration(mozilla::css::Declaration* aDeclaration) { mDeclaration = aDeclaration; - mSheet->SetModifiedByChildRule(); + if (mSheet) { + mSheet->SetModifiedByChildRule(); + } } // ------------------------------------------- // nsCSSKeyframesRule // nsCSSKeyframesRule::nsCSSKeyframesRule(const nsCSSKeyframesRule& aCopy) // copy everything except our reference count. GroupRule's copy @@ -2038,17 +2043,19 @@ nsCSSKeyframesRule::GetName(nsAString& a return NS_OK; } NS_IMETHODIMP nsCSSKeyframesRule::SetName(const nsAString& aName) { mName = aName; - mSheet->SetModifiedByChildRule(); + if (mSheet) { + mSheet->SetModifiedByChildRule(); + } return NS_OK; } NS_IMETHODIMP nsCSSKeyframesRule::GetCssRules(nsIDOMCSSRuleList* *aRuleList) { NS_ADDREF(*aRuleList = GroupRule::GetCssRules()); @@ -2063,17 +2070,19 @@ nsCSSKeyframesRule::InsertRule(const nsA // http://lists.w3.org/Archives/Public/www-style/2011Apr/0034.html nsCSSParser parser; // FIXME: pass filename and line number nsRefPtr<nsCSSKeyframeRule> rule = parser.ParseKeyframeRule(aRule, nsnull, 0); if (rule) { mRules.AppendObject(rule); - mSheet->SetModifiedByChildRule(); + if (mSheet) { + mSheet->SetModifiedByChildRule(); + } } return NS_OK; } static const PRUint32 RULE_NOT_FOUND = PRUint32(-1); PRUint32 @@ -2100,17 +2109,19 @@ nsCSSKeyframesRule::FindRuleIndexForKey( } NS_IMETHODIMP nsCSSKeyframesRule::DeleteRule(const nsAString& aKey) { PRUint32 index = FindRuleIndexForKey(aKey); if (index != RULE_NOT_FOUND) { mRules.RemoveObjectAt(index); - mSheet->SetModifiedByChildRule(); + if (mSheet) { + mSheet->SetModifiedByChildRule(); + } } return NS_OK; } NS_IMETHODIMP nsCSSKeyframesRule::FindRule(const nsAString& aKey, nsIDOMMozCSSKeyframeRule** aResult) {
--- a/layout/style/test/Makefile.in +++ b/layout/style/test/Makefile.in @@ -163,16 +163,17 @@ GARBAGE += css_properties.js test_parse_rule.html \ test_parse_url.html \ test_pixel_lengths.html \ test_pointer-events.html \ test_property_database.html \ test_priority_preservation.html \ test_property_syntax_errors.html \ test_rem_unit.html \ + test_rules_out_of_sheets.html \ test_selectors.html \ test_selectors_on_anonymous_content.html \ test_shorthand_property_getters.html \ test_style_struct_copy_constructors.html \ test_system_font_serialization.html \ test_transitions_and_zoom.html \ test_transitions_cancel_near_end.html \ test_transitions_computed_values.html \
new file mode 100644 --- /dev/null +++ b/layout/style/test/test_rules_out_of_sheets.html @@ -0,0 +1,116 @@ +<!DOCTYPE HTML> +<html> +<!-- +https://bugzilla.mozilla.org/show_bug.cgi?id=634373 +--> +<head> + <title>Test for Bug 634373</title> + <script type="application/javascript" src="/MochiKit/packed.js"></script> + <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script> + <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> +</head> +<body> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=634373">Mozilla Bug 634373</a> +<p id="display"></p> +<div id="content" style="display: none"> + +</div> +<pre id="test"> +<script type="application/javascript"> + +/** Test for Bug 634373 **/ + +function make_rule_and_remove_sheet(text, getter) { + var style = document.createElement("style"); + style.setAttribute("type", "text/css"); + style.appendChild(document.createTextNode(text)); + document.head.appendChild(style); + var result = style.sheet.cssRules[0]; + if (getter) { + result = getter(result); + } + document.head.removeChild(style); + style = null; + SpecialPowers.DOMWindowUtils.garbageCollect(); + return result; +} + +var gDisplayCS = getComputedStyle(document.getElementById("display"), ""); + +function keep_rule_alive_by_matching(rule) { + // It's the caller's job to guarantee that the rule matches a p. + // This just causes a style flush, which in turn keeps the rule alive + // until the next style flush. + var color = gDisplayCS.color; + return rule; +} + +function get_rule_and_child(rule) { + return [rule, rule.cssRules[0]]; +} + +function get_only_child(rule) { + return rule.cssRules[0]; +} + +var rule; + +// In this case, the rule goes away immediately, so we're testing +// the DOM wrapper's handling of a null rule, rather than the rule's +// handling of a null sheet. +rule = make_rule_and_remove_sheet("p { color: blue }"); +rule.style.color = ""; +try { + rule.style.color = "fuchsia"; +} catch(ex) {} + +rule = make_rule_and_remove_sheet("p { color: blue }", + keep_rule_alive_by_matching); +try { + rule.style.color = ""; +} catch(ex) {} +try { + rule.style.color = "fuchsia"; +} catch(ex) {} + +rule = make_rule_and_remove_sheet("@media screen { p { color: blue } }", + get_rule_and_child); +rule[1].style.color = ""; +try { + rule[1].style.color = "fuchsia"; +} catch(ex) {} + +// In this case, the rule goes away immediately, so we're testing +// the DOM wrapper's handling of a null rule, rather than the rule's +// handling of a null sheet. +rule = make_rule_and_remove_sheet("@media screen { p { color: blue } }", + get_only_child); +rule.style.color = ""; +try { + rule.style.color = "fuchsia"; +} catch(ex) {} + +rule = make_rule_and_remove_sheet("@media screen { p { color: blue } }", + function(rule) { + return keep_rule_alive_by_matching( + get_only_child(rule)); + }); +try { + rule.style.color = ""; +} catch(ex) {} +try { + rule.style.color = "fuchsia"; +} catch(ex) {} + +rule = make_rule_and_remove_sheet("@-moz-keyframes a { from { color: blue } }"); +rule.insertRule("from { color: fuchsia}"); +rule.deleteRule("from"); +rule.name = "b"; +rule.cssRules[0].keyText = "50%"; + +ok(true, "didn't crash"); + +</script> +</pre> +</body> +</html>