Audit for places in style rule code that need to check for a null sheet. (Bug 634373) r=bzbarsky

This commit is contained in:
L. David Baron 2011-05-02 18:43:44 -07:00
parent 8c9dd3c89f
commit 7eaaea7cc7
4 changed files with 139 additions and 8 deletions

View File

@ -1378,10 +1378,13 @@ StyleRule::DeclarationChanged(Declaration* aDecl,
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);
}
}

View File

@ -544,6 +544,7 @@ GroupRule::GroupRule(const GroupRule& aCopy)
GroupRule::~GroupRule()
{
NS_ABORT_IF_FALSE(!mSheet, "SetStyleSheet should have been called");
mRules.EnumerateForwards(SetParentRuleReference, nsnull);
if (mRuleCollection) {
mRuleCollection->DropReference();
@ -1908,7 +1909,9 @@ nsCSSKeyframeRule::SetKeyText(const nsAString& aKeyText)
// for now, we don't do anything if the parse fails
}
mSheet->SetModifiedByChildRule();
if (mSheet) {
mSheet->SetModifiedByChildRule();
}
return NS_OK;
}
@ -1928,7 +1931,9 @@ nsCSSKeyframeRule::ChangeDeclaration(mozilla::css::Declaration* aDeclaration)
{
mDeclaration = aDeclaration;
mSheet->SetModifiedByChildRule();
if (mSheet) {
mSheet->SetModifiedByChildRule();
}
}
// -------------------------------------------
@ -2043,7 +2048,9 @@ nsCSSKeyframesRule::SetName(const nsAString& aName)
{
mName = aName;
mSheet->SetModifiedByChildRule();
if (mSheet) {
mSheet->SetModifiedByChildRule();
}
return NS_OK;
}
@ -2068,7 +2075,9 @@ nsCSSKeyframesRule::InsertRule(const nsAString& aRule)
parser.ParseKeyframeRule(aRule, nsnull, 0);
if (rule) {
mRules.AppendObject(rule);
mSheet->SetModifiedByChildRule();
if (mSheet) {
mSheet->SetModifiedByChildRule();
}
}
return NS_OK;
@ -2105,7 +2114,9 @@ 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;
}

View File

@ -168,6 +168,7 @@ _TEST_FILES = test_acid3_test46.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 \

View File

@ -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>