871578 - don't include AccessCheck.h or nsContentUtils.h in bindings unless necessary

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: don't include nsContentUtils.h in bindings unless necessary

I don't particularly like the gory condition that's required to determine
this, but it's what CGClassHasInstanceHook and CGAddPropertyHook are
conditioned on.  I guess we could add slightly nicer predicate functions...

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: don't include AccessCheck.h in bindings unless necessary

This is a little nicer, but possibly not as much of a win, because we need to
include WrapperFactory.h for what looks like a fair amount of stuff.

On the plus side, it looks like AccessCheck.h doesn't need to include
WrapperFactory.h anymore, so maybe we can fix that up in another bug.

I think this patch will become substantially simpler with bug 869195
applied, so if you want to wait for that bug to land and have the patch
redone based on that, that'd be fine too.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 748845 [details] [diff] [review]
don't include nsContentUtils.h in bindings unless necessary

Please add a comment explaining why the things that condition is checking are the ones that need nsContentUtils.

r=me

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 748848 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

Please do this on top of bug 869195, yes.  That way we can land the same thing on trunk and branch for that bug....

Comment 5

[Boris Zbarsky [:bzbarsky]]

That's landed now if you want to rebase....

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: don't include AccessCheck.h in bindings unless necessary

Rebased on bug 869195.  I suppose the requiresQueryInterfaceMethod change
isn't strictly needed now, but I kept it in just because.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 751035 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

This stopped calling allAncestorsAbstract, so presumably doesn't pass tests...

Comment 8

[Nathan Froyd [:froydnj]]

Attachment: don't include AccessCheck.h in bindings unless necessary

Doh, how about this instead?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 751045 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

> +    return (not descriptor.interface.parent and

No.  You do not want this part.  Please compare your new condition to the old one!

Comment 10

[Nathan Froyd [:froydnj]]

Attachment: don't include AccessCheck.h in bindings unless necessary

Gah, I suck.  Sorry about that.  Third time's the charm?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 751068 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

r=me.  Good.  ;)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1f6740440eaa
https://hg.mozilla.org/mozilla-central/rev/ae494c31aab2