Bug 978833 patch 12 - Use the css::Declaration instead of the css::StyleRule as the matching rule. r=heycam

This is the key change in this patch series; it changes the object we
use for style data (currently nsIStyleRule) identity.  It allows
removing some hacks we have to deal with that for StyleRule, and avoids
having to write similar hacks for nsCSSKeyframeRule and nsCSSPageRule
(which are broken without this).

I confirmed locally that it is this patch that fixes both of the todo_is
mochitests, by building and testing with the patch queue through patch
11, and again through patch 12.
This commit is contained in:
L. David Baron 2015-11-05 16:44:10 +08:00
parent 9a63529ce5
commit c7d6d8dcfa
10 changed files with 80 additions and 68 deletions

View File

@ -2135,8 +2135,8 @@ CanvasRenderingContext2D::SetShadowColor(const nsAString& shadowColor)
// filters
//
static already_AddRefed<StyleRule>
CreateStyleRule(nsINode* aNode,
static already_AddRefed<Declaration>
CreateDeclaration(nsINode* aNode,
const nsCSSProperty aProp1, const nsAString& aValue1, bool* aChanged1,
const nsCSSProperty aProp2, const nsAString& aValue2, bool* aChanged2,
ErrorResult& error)
@ -2171,17 +2171,18 @@ CreateStyleRule(nsINode* aNode,
rule->RuleMatched();
return rule.forget();
RefPtr<Declaration> declaration = rule->GetDeclaration();
return declaration.forget();
}
static already_AddRefed<StyleRule>
CreateFontStyleRule(const nsAString& aFont,
nsINode* aNode,
bool* aOutFontChanged,
ErrorResult& error)
static already_AddRefed<Declaration>
CreateFontDeclaration(const nsAString& aFont,
nsINode* aNode,
bool* aOutFontChanged,
ErrorResult& error)
{
bool lineHeightChanged;
return CreateStyleRule(aNode,
return CreateDeclaration(aNode,
eCSSProperty_font, aFont, aOutFontChanged,
eCSSProperty_line_height, NS_LITERAL_STRING("normal"), &lineHeightChanged,
error);
@ -2205,9 +2206,9 @@ GetFontParentStyleContext(Element* aElement, nsIPresShell* presShell,
// otherwise inherit from default (10px sans-serif)
bool changed;
RefPtr<css::StyleRule> parentRule =
CreateFontStyleRule(NS_LITERAL_STRING("10px sans-serif"),
presShell->GetDocument(), &changed, error);
RefPtr<css::Declaration> parentRule =
CreateFontDeclaration(NS_LITERAL_STRING("10px sans-serif"),
presShell->GetDocument(), &changed, error);
if (error.Failed()) {
return nullptr;
@ -2226,13 +2227,12 @@ GetFontParentStyleContext(Element* aElement, nsIPresShell* presShell,
}
static bool
PropertyIsInheritOrInitial(StyleRule* aRule, const nsCSSProperty aProperty)
PropertyIsInheritOrInitial(Declaration* aDeclaration, const nsCSSProperty aProperty)
{
css::Declaration* declaration = aRule->GetDeclaration();
// We know the declaration is not !important, so we can use
// GetNormalBlock().
const nsCSSValue* filterVal =
declaration->GetNormalBlock()->ValueFor(aProperty);
aDeclaration->GetNormalBlock()->ValueFor(aProperty);
return (!filterVal || (filterVal->GetUnit() == eCSSUnit_Unset ||
filterVal->GetUnit() == eCSSUnit_Inherit ||
filterVal->GetUnit() == eCSSUnit_Initial));
@ -2245,9 +2245,9 @@ GetFontStyleContext(Element* aElement, const nsAString& aFont,
ErrorResult& error)
{
bool fontParsedSuccessfully = false;
RefPtr<css::StyleRule> rule =
CreateFontStyleRule(aFont, presShell->GetDocument(),
&fontParsedSuccessfully, error);
RefPtr<css::Declaration> decl =
CreateFontDeclaration(aFont, presShell->GetDocument(),
&fontParsedSuccessfully, error);
if (error.Failed()) {
return nullptr;
@ -2262,7 +2262,7 @@ GetFontStyleContext(Element* aElement, const nsAString& aFont,
// 'inherit' and 'initial'. The easiest way to check for this is to look
// at font-size-adjust, which the font shorthand resets to either 'none' or
// '-moz-system-font'.
if (PropertyIsInheritOrInitial(rule, eCSSProperty_font_size_adjust)) {
if (PropertyIsInheritOrInitial(decl, eCSSProperty_font_size_adjust)) {
return nullptr;
}
@ -2283,7 +2283,7 @@ GetFontStyleContext(Element* aElement, const nsAString& aFont,
"GetFontParentStyleContext should have returned an error if the presshell is being destroyed.");
nsTArray<nsCOMPtr<nsIStyleRule>> rules;
rules.AppendElement(rule);
rules.AppendElement(decl);
// add a rule to prevent text zoom from affecting the style
rules.AppendElement(new nsDisableTextZoomStyleRule);
@ -2295,34 +2295,34 @@ GetFontStyleContext(Element* aElement, const nsAString& aFont,
// parsed (including having line-height removed). (Older drafts of
// the spec required font sizes be converted to pixels, but that no
// longer seems to be required.)
rule->GetDeclaration()->GetValue(eCSSProperty_font, aOutUsedFont);
decl->GetValue(eCSSProperty_font, aOutUsedFont);
return sc.forget();
}
static already_AddRefed<StyleRule>
CreateFilterStyleRule(const nsAString& aFilter,
nsINode* aNode,
bool* aOutFilterChanged,
ErrorResult& error)
static already_AddRefed<Declaration>
CreateFilterDeclaration(const nsAString& aFilter,
nsINode* aNode,
bool* aOutFilterChanged,
ErrorResult& error)
{
bool dummy;
return CreateStyleRule(aNode,
return CreateDeclaration(aNode,
eCSSProperty_filter, aFilter, aOutFilterChanged,
eCSSProperty_UNKNOWN, EmptyString(), &dummy,
error);
}
static already_AddRefed<nsStyleContext>
ResolveStyleForFilterRule(const nsAString& aFilterString,
nsIPresShell* aPresShell,
nsStyleContext* aParentContext,
ErrorResult& error)
ResolveStyleForFilter(const nsAString& aFilterString,
nsIPresShell* aPresShell,
nsStyleContext* aParentContext,
ErrorResult& error)
{
nsIDocument* document = aPresShell->GetDocument();
bool filterChanged = false;
RefPtr<css::StyleRule> rule =
CreateFilterStyleRule(aFilterString, document, &filterChanged, error);
RefPtr<css::Declaration> decl =
CreateFilterDeclaration(aFilterString, document, &filterChanged, error);
if (error.Failed()) {
return nullptr;
@ -2335,12 +2335,12 @@ ResolveStyleForFilterRule(const nsAString& aFilterString,
// In addition to unparseable values, the spec says we need to reject
// 'inherit' and 'initial'.
if (PropertyIsInheritOrInitial(rule, eCSSProperty_filter)) {
if (PropertyIsInheritOrInitial(decl, eCSSProperty_filter)) {
return nullptr;
}
nsTArray<nsCOMPtr<nsIStyleRule>> rules;
rules.AppendElement(rule);
rules.AppendElement(decl);
RefPtr<nsStyleContext> sc =
aPresShell->StyleSet()->ResolveStyleForRules(aParentContext, rules);
@ -2375,7 +2375,7 @@ CanvasRenderingContext2D::ParseFilter(const nsAString& aString,
}
RefPtr<nsStyleContext> sc =
ResolveStyleForFilterRule(aString, presShell, parentContext, error);
ResolveStyleForFilter(aString, presShell, parentContext, error);
if (!sc) {
return false;

View File

@ -904,7 +904,7 @@ nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
if (mContentStyleRule) {
mContentStyleRule->RuleMatched();
aRuleWalker->Forward(mContentStyleRule);
aRuleWalker->Forward(mContentStyleRule->GetDeclaration());
}
return NS_OK;
@ -928,7 +928,7 @@ nsSVGElement::WalkAnimatedContentStyleRules(nsRuleWalker* aRuleWalker)
}
if (animContentStyleRule) {
animContentStyleRule->RuleMatched();
aRuleWalker->Forward(animContentStyleRule);
aRuleWalker->Forward(animContentStyleRule->GetDeclaration());
}
}
}

View File

@ -238,13 +238,17 @@ inDOMUtils::GetCSSStyleRules(nsIDOMElement *aElement,
NS_NewISupportsArray(getter_AddRefs(rules));
if (!rules) return NS_ERROR_OUT_OF_MEMORY;
RefPtr<mozilla::css::StyleRule> cssRule;
for ( ; !ruleNode->IsRoot(); ruleNode = ruleNode->GetParent()) {
cssRule = do_QueryObject(ruleNode->GetRule());
if (cssRule) {
nsCOMPtr<nsIDOMCSSRule> domRule = cssRule->GetDOMRule();
if (domRule)
rules->InsertElementAt(domRule, 0);
RefPtr<Declaration> decl = do_QueryObject(ruleNode->GetRule());
if (decl) {
RefPtr<mozilla::css::StyleRule> styleRule =
do_QueryObject(decl->GetOwningRule());
if (styleRule) {
nsCOMPtr<nsIDOMCSSRule> domRule = styleRule->GetDOMRule();
if (domRule) {
rules->InsertElementAt(domRule, 0);
}
}
}
}

View File

@ -2631,7 +2631,7 @@ StyleAnimationValue::ComputeValues(
nsCOMArray<nsIStyleRule> ruleArray;
ruleArray.AppendObject(styleSet->InitialStyleRule());
ruleArray.AppendObject(aStyleRule);
ruleArray.AppendObject(aStyleRule->GetDeclaration());
aStyleRule->RuleMatched();
tmpStyleContext =
styleSet->ResolveStyleByAddingRules(styleContext, ruleArray);
@ -2658,7 +2658,7 @@ StyleAnimationValue::ComputeValues(
// value may have been biased by the 'initial' values supplied.
if (!aIsContextSensitive || *aIsContextSensitive) {
nsCOMArray<nsIStyleRule> ruleArray;
ruleArray.AppendObject(aStyleRule);
ruleArray.AppendObject(aStyleRule->GetDeclaration());
aStyleRule->RuleMatched();
tmpStyleContext =
styleSet->ResolveStyleByAddingRules(styleContext, ruleArray);

View File

@ -632,7 +632,7 @@ ResolvedStyleCache::Get(nsPresContext *aPresContext,
"Keyframe rule has !important data");
nsCOMArray<nsIStyleRule> rules;
rules.AppendObject(aKeyframe);
rules.AppendObject(declaration);
RefPtr<nsStyleContext> resultStrong = aPresContext->StyleSet()->
ResolveStyleByAddingRules(aParentStyleContext, rules);
mCache.Put(aKeyframe, resultStrong);

View File

@ -2620,7 +2620,7 @@ void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector,
eLookForRelevantLink)) {
css::StyleRule *rule = value.mRule;
rule->RuleMatched();
data->mRuleWalker->Forward(rule);
data->mRuleWalker->Forward(rule->GetDeclaration());
// nsStyleSet will deal with the !important rule
}
}
@ -2666,7 +2666,7 @@ nsCSSRuleProcessor::RulesMatching(AnonBoxRuleProcessorData* aData)
for (RuleValue *value = rules.Elements(), *end = value + rules.Length();
value != end; ++value) {
value->mRule->RuleMatched();
aData->mRuleWalker->Forward(value->mRule);
aData->mRuleWalker->Forward(value->mRule->GetDeclaration());
}
}
}

View File

@ -68,7 +68,7 @@ nsHTMLCSSStyleSheet::ElementRulesMatching(nsPresContext* aPresContext,
css::StyleRule* rule = aElement->GetInlineStyleRule();
if (rule) {
rule->RuleMatched();
aRuleWalker->Forward(rule);
aRuleWalker->Forward(rule->GetDeclaration());
}
rule = aElement->GetSMILOverrideStyleRule();
@ -78,7 +78,7 @@ nsHTMLCSSStyleSheet::ElementRulesMatching(nsPresContext* aPresContext,
// Animation restyle (or non-restyle traversal of rules)
// Now we can walk SMIL overrride style, without triggering transitions.
rule->RuleMatched();
aRuleWalker->Forward(rule);
aRuleWalker->Forward(rule->GetDeclaration());
}
}
}
@ -97,7 +97,7 @@ nsHTMLCSSStyleSheet::PseudoElementRulesMatching(Element* aPseudoElement,
css::StyleRule* rule = aPseudoElement->GetInlineStyleRule();
if (rule) {
rule->RuleMatched();
aRuleWalker->Forward(rule);
aRuleWalker->Forward(rule->GetDeclaration());
}
}

View File

@ -13,7 +13,7 @@
#include "nsRuleNode.h"
#include "nsIStyleRule.h"
#include "StyleRule.h"
#include "Declaration.h"
#include "nsQueryObject.h"
class nsRuleWalker {
@ -34,14 +34,14 @@ protected:
public:
void Forward(nsIStyleRule* aRule) {
NS_PRECONDITION(!RefPtr<mozilla::css::StyleRule>(do_QueryObject(aRule)),
NS_PRECONDITION(!RefPtr<mozilla::css::Declaration>(do_QueryObject(aRule)),
"Calling the wrong Forward() overload");
DoForward(aRule);
}
void Forward(mozilla::css::StyleRule* aRule) {
void Forward(mozilla::css::Declaration* aRule) {
DoForward(aRule);
mCheckForImportantRules =
mCheckForImportantRules && !aRule->GetImportantRule();
mCheckForImportantRules && !aRule->HasImportantData();
}
// ForwardOnPossiblyCSSRule should only be used by callers that have
// an explicit list of rules they need to walk, with the list

View File

@ -1039,11 +1039,11 @@ nsStyleSet::AddImportantRules(nsRuleNode* aCurrLevelNode,
node = node->GetParent()) {
// We guarantee that we never walk the root node here, so no need
// to null-check GetRule(). Furthermore, it must be a CSS rule.
NS_ASSERTION(RefPtr<css::StyleRule>(do_QueryObject(node->GetRule())),
NS_ASSERTION(RefPtr<css::Declaration>(do_QueryObject(node->GetRule())),
"Unexpected non-CSS rule");
nsIStyleRule* impRule =
static_cast<css::StyleRule*>(node->GetRule())->GetImportantRule();
static_cast<css::Declaration*>(node->GetRule())->GetImportantStyleData();
if (impRule)
importantRules.AppendElement(impRule);
}
@ -1066,10 +1066,11 @@ nsStyleSet::AssertNoImportantRules(nsRuleNode* aCurrLevelNode,
for (nsRuleNode *node = aCurrLevelNode; node != aLastPrevLevelNode;
node = node->GetParent()) {
RefPtr<css::StyleRule> rule(do_QueryObject(node->GetRule()));
NS_ASSERTION(rule, "Unexpected non-CSS rule");
RefPtr<css::Declaration> declaration(do_QueryObject(node->GetRule()));
NS_ASSERTION(declaration, "Unexpected non-CSS rule");
NS_ASSERTION(!rule->GetImportantRule(), "Unexpected important rule");
NS_ASSERTION(!declaration->GetImportantStyleData(),
"Unexpected important style source");
}
}
@ -1083,8 +1084,13 @@ nsStyleSet::AssertNoCSSRules(nsRuleNode* aCurrLevelNode,
for (nsRuleNode *node = aCurrLevelNode; node != aLastPrevLevelNode;
node = node->GetParent()) {
nsIStyleRule *rule = node->GetRule();
RefPtr<css::StyleRule> cssRule(do_QueryObject(rule));
NS_ASSERTION(!cssRule || !cssRule->Selector(), "Unexpected CSS rule");
RefPtr<css::Declaration> declaration(do_QueryObject(rule));
if (declaration) {
RefPtr<css::StyleRule> cssRule =
do_QueryObject(declaration->GetOwningRule());
NS_ASSERTION(!cssRule || !cssRule->Selector(),
"Unexpected CSS rule");
}
}
}
#endif
@ -1963,9 +1969,11 @@ nsStyleSet::ResolveAnonymousBoxStyle(nsIAtom* aPseudoTag,
nsTArray<css::ImportantStyleData*> importantRules;
PresContext()->StyleSet()->AppendPageRules(rules);
for (uint32_t i = 0, i_end = rules.Length(); i != i_end; ++i) {
rules[i]->Declaration()->SetImmutable();
ruleWalker.Forward(rules[i]);
css::ImportantStyleData* importantRule = rules[i]->GetImportantRule();
css::Declaration* declaration = rules[i]->Declaration();
declaration->SetImmutable();
ruleWalker.Forward(declaration);
css::ImportantStyleData* importantRule =
declaration->GetImportantStyleData();
if (importantRule) {
importantRules.AppendElement(importantRule);
}

View File

@ -46,14 +46,14 @@ function test_bug978833() {
p.classList.add("alwaysa");
kf.style.marginLeft = "100px";
todo_is(cs.marginLeft, "100px", "p margin-left should be 100px after change");
is(cs.marginLeft, "100px", "p margin-left should be 100px after change");
// Try the same thing a second time, just to make sure it works again.
p.classList.remove("alwaysa");
is(cs.marginLeft, "0px", "p margin-left should be 0px without animation");
p.classList.add("alwaysa");
kf.style.marginLeft = "150px";
todo_is(cs.marginLeft, "150px", "p margin-left should be 150px after second change");
is(cs.marginLeft, "150px", "p margin-left should be 150px after second change");
p.style.animation = "";
}