With these hooks, we can remove the need to expose an arbitrary attribute getter to servo, which avoids the need to convert UTF-16 to UTF-8.

I have no idea why these are virtual, but they're somewhat hot, so we should fix that.

Comment on attachment 8766938 [details] [diff] [review] Part 5 - Implement attribute API for servo. v1 Review of attachment 8766938 [details] [diff] [review]: ----------------------------------------------------------------- I think we can avoid the long (and ugly) macros in favor of some more modern C++-isms. ::: layout/style/ServoBindings.cpp @@ +186,5 @@ > +{ > + return false; > +} > + > +#define IMPL_MATCH(aElement, aNS, aName, args) \ Why not: template <class MatchFn> DoMatch(RawGeckoElement* aElement, nsIAtom* aNS, nsIAtom* aName, MatchFn aMatcher) { ...aMatcher(value)... } Called as (e.g.): bool Gecko_AttrEquals(...) { auto matcher = [ aStr, aIgnoreCase ](nsAttrValue* aValue) { return aValue->Equals(aStr, aIgnoreCase ? eIgnoreCase : eCaseMatters); } return DoMatch(aElement, aNS, aName, matcher); } avoiding long macros and keeping type safety.

Er, DoMatch would obviously return bool.

Nice idea. I initially avoided using lambdas because I knew our static analysis was going to try to make me use strong refs, but I found a workaround. :-)

CC mystor, who wants to remove the FakeRef thing when he fixes the static analysis.

Comment on attachment 8768187 [details] [diff] [review] Part 5 - Implement attribute API for servo. v2 Review of attachment 8768187 [details] [diff] [review]: ----------------------------------------------------------------- Vive bug 1281935 :-) ::: layout/style/ServoBindings.cpp @@ +190,5 @@ > + NS_ENSURE_TRUE(ns != kNameSpaceID_Unknown, false); > + const nsAttrValue* value = aElement->GetParsedAttr(aName, ns); > + return value && aMatch(value); > + } > + /* No namespace means any namespace - we have to check them all. :-( */ This could be a C++-style comment now, if you want.

Pushed by bholley@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b4cb1618f8b Devirtualize a bunch of namespace methods. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/44266a61eeac Add an atom overload of GetNameSpaceID. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/52828db7c9e4 Add a namespaced overload of GetParsedAttr. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/3f2b97db440f Hoist ValueIncludes into nsStyleUtil. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/c23ae65921f0 Implement attribute API for servo. r=mrbkap

Green try runs, for posterity: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b508f5bd407a with a followup fix to appease static analysis: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad11f36abb1d

https://hg.mozilla.org/mozilla-central/rev/3b4cb1618f8b https://hg.mozilla.org/mozilla-central/rev/44266a61eeac https://hg.mozilla.org/mozilla-central/rev/52828db7c9e4 https://hg.mozilla.org/mozilla-central/rev/3f2b97db440f https://hg.mozilla.org/mozilla-central/rev/c23ae65921f0