nsICSSGroupRule is only implemented by nsCSSGroupRule. This should be collapsed and everyone should just use nsCSSGroupRule. Then we should deCOM and devirtualize the methods.

Also converted a member in nsCSSGroupRule to nsRefPtr to remove manual reference counting.

Updated due to patch in another bug

Patches 3 and 4 appear to have their order reversed.

Oops(In reply to comment #8) > Patches 3 and 4 appear to have their order reversed. I think you're right. Sorry.

Also, I get: /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/style/nsCSSRules.cpp:91: error: incomplete type ‘mozilla::css::GroupRuleRuleList’ not allowed (repeated about 10 times) I think the DOMCI_DATA needs to move down below the class declaration.

Interesting. I'll have to check when I get home, but I'm pretty sure I was able to build with it where it is. Though I can see why it wouldn't work. What OS are you compiling on?

64-bit Ubuntu 10.10, gcc 4.4.5 One other idea: you might be able to avoid the namespace hacks around the uses of DOMCI_DATA if you put a :: inside the contents of the DOMCI_DATA macro.

Didn't work. Compiler does like declaring a variable with :: in front of it. Got errors like this: dom/base/nsDOMException.cpp:156: error: explicit qualification in declaration of ‘kDOMClassInfo_DOMException_interfaces’

Comment on attachment 523944 [details] [diff] [review] Part 1: Rename CSSGroupRuleListImpl r=me

Comment on attachment 523945 [details] [diff] [review] Part 2: Remove nsICSSGroupRule r=me

Comment on attachment 523946 [details] [diff] [review] Part 3: Rename nsCSSDocumentRule and nsCSSMediaRule r=me

Comment on attachment 523947 [details] [diff] [review] Part 4: DeCOM and de-virtualize methods in GroupRule >+++ b/layout/style/GroupRule.h >+ PRInt32 StyleRuleCount() const { return mRules.Count(); } >+ nsresult GetStyleRuleAt(PRInt32 aIndex, nsICSSRule*& aRule) const; Why not just: nsICSSRule* GetStyleRuleAt(PRInt32 aIndex) const; returning null on out-of-bounds? Seems like that would work better. >+ already_AddRefed<nsIDOMCSSStyleSheet> GetParentStyleSheet(); Why can't this return nsIDOMCSSStyleSheet* instead? > nsresult GetParentRule(nsIDOMCSSRule** aParentRule); This will presumably get deCOMed when we deCOM mParentRule, right? >+ already_AddRefed<nsIDOMCSSRuleList> GetCssRules(); Why can't this return nsIDOMCSSRuleList* instead? r- for now; I'd really prefer we return weak pointers instead of manually refcounting smart pointers. But if there's a good reason to do things the way this patch does, I'm open to being convinced!

Comment on attachment 524122 [details] [diff] [review] Part 4: DeCOM and de-virtualize methods in GroupRule This addresses everything but GetParentRule. That needs to be cleaned up on all rule classes which I will deal with in another bug.

Comment on attachment 524132 [details] [diff] [review] Part 4: DeCOM and de-virtualize methods in GroupRule > +GroupRule::GetStyleRuleAt(PRInt32 aIndex) const This should just do: return mRules.SafeObjectAt(aIndex); instead of reimplementing it. r=me with that nit.

Pushed: http://hg.mozilla.org/mozilla-central/rev/45d658bb4be1 http://hg.mozilla.org/mozilla-central/rev/f100d70f483a http://hg.mozilla.org/mozilla-central/rev/8bd0c6330318 http://hg.mozilla.org/mozilla-central/rev/7e05a50ed66b