Andrew, are you doing this? If not, I was planning to.

I hadn't started on it or anything. Either way is fine with me.

This simple testcase: <style></style> <script> try { m = new WeakMap(); var s = document.styleSheets[0]; m.set(s, 5); alert(m.get(s)); } catch (e) { alert(e); } </script> seems to work fine with these patches. Seems like we could have just mutated bug 846666... ;)

Comment on attachment 720232 [details] [diff] [review] part 1. Make nsCSSStyleSheet wrappercached. Review of attachment 720232 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/nsCSSStyleSheet.cpp @@ +1215,2 @@ > NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END > +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsCSSStyleSheet) You could use NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE here. ::: layout/style/nsCSSStyleSheet.h @@ +106,5 @@ > > class nsCSSStyleSheet MOZ_FINAL : public nsIStyleSheet, > public nsIDOMCSSStyleSheet, > + public nsICSSLoaderObserver, > + public nsWrapperCache I guess it was only non-nsISupports things that must inherit from wrappercache first, right?

> You could use NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE here. Done. > I guess it was only non-nsISupports things that must inherit from wrappercache first Correct.

Comment on attachment 720233 [details] [diff] [review] part 2. Add the WebIDL APIs for StyleSheet and CSSStyleSheet. Review of attachment 720233 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/html/content/src/HTMLLinkElement.cpp @@ +89,5 @@ > if (!ss) { > return; > } > > + ss->SetDisabled(aDisabled); I'd just do if (ss) { ss->SetDisabled(aDisabled); } ::: content/html/content/src/HTMLStyleElement.cpp @@ +90,5 @@ > if (!ss) { > return; > } > > + ss->SetDisabled(aDisabled); Same here.

Comment on attachment 720233 [details] [diff] [review] part 2. Add the WebIDL APIs for StyleSheet and CSSStyleSheet. Review of attachment 720233 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/BindingDeclarations.h @@ +399,5 @@ > +{ > + return NULL; > +} > + > +struct ParentObject { { on the next line ::: layout/style/nsCSSStyleSheet.h @@ +257,5 @@ > > + // WebIDL StyleSheet API > + // Our nsIStyleSheet::GetType is a const method, so it ends up > + // ambiguous with with the XPCOM version. Just disambiguate. > + void GetType(nsString& aType) { { on the next line @@ +265,5 @@ > + nsINode* GetOwnerNode() const { return mOwningNode; } > + nsCSSStyleSheet* GetParentStyleSheet() const { return mParent; } > + // Our nsIStyleSheet::GetTitle is a const method, so it ends up > + // ambiguous with with the XPCOM version. Just disambiguate. > + void GetTitle(nsString& aTitle) { . @@ +266,5 @@ > + nsCSSStyleSheet* GetParentStyleSheet() const { return mParent; } > + // Our nsIStyleSheet::GetTitle is a const method, so it ends up > + // ambiguous with with the XPCOM version. Just disambiguate. > + void GetTitle(nsString& aTitle) { > + const_cast<const nsCSSStyleSheet*>(this)->GetTitle(aTitle); :/ @@ +279,5 @@ > + // version. > + nsIDOMCSSRule* GetDOMOwnerRule() const; > + nsIDOMCSSRuleList* GetCssRules(mozilla::ErrorResult& aRv); > + uint32_t InsertRule(const nsAString& aRule, uint32_t aIndex, > + mozilla::ErrorResult& aRv) { . @@ +284,5 @@ > + uint32_t retval; > + aRv = InsertRule(aRule, aIndex, &retval); > + return retval; > + } > + void DeleteRule(uint32_t aIndex, mozilla::ErrorResult& aRv) { . @@ +297,5 @@ > + > + return mozilla::dom::ParentObject(static_cast<nsIStyleSheet*>(mParent), > + mParent); > + } > + Trailing whitespace

(In reply to :Ms2ger from comment #10) > Comment on attachment 720233 [details] [diff] [review] > part 2. Add the WebIDL APIs for StyleSheet and CSSStyleSheet. > ::: layout/style/nsCSSStyleSheet.h > @@ +257,5 @@ > > > > + // WebIDL StyleSheet API > > + // Our nsIStyleSheet::GetType is a const method, so it ends up > > + // ambiguous with with the XPCOM version. Just disambiguate. > > + void GetType(nsString& aType) { > > { on the next line Not when the definition is inside the class declaration.

> I'd just do Done. >{ on the next line As dbaron said, file style is what I did here. > Trailing whitespace Fixed.

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/946419184f99 https://hg.mozilla.org/integration/mozilla-inbound/rev/883db473ec46 https://hg.mozilla.org/integration/mozilla-inbound/rev/dd888bc71adc

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #11) > (In reply to :Ms2ger from comment #10) > > Comment on attachment 720233 [details] [diff] [review] > > part 2. Add the WebIDL APIs for StyleSheet and CSSStyleSheet. > > ::: layout/style/nsCSSStyleSheet.h > > @@ +257,5 @@ > > > > > > + // WebIDL StyleSheet API > > > + // Our nsIStyleSheet::GetType is a const method, so it ends up > > > + // ambiguous with with the XPCOM version. Just disambiguate. > > > + void GetType(nsString& aType) { > > > > { on the next line > > Not when the definition is inside the class declaration. Unless it's void SetScopeElement(mozilla::dom::Element* aScopeElement) { a couple of lines earlier, then.

https://hg.mozilla.org/mozilla-central/rev/946419184f99 https://hg.mozilla.org/mozilla-central/rev/883db473ec46 https://hg.mozilla.org/mozilla-central/rev/dd888bc71adc