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

RESOLVED FIXED in mozilla24

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Unassigned)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Comment 1

5 years ago
Created attachment 748845 [details] [diff] [review]
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...
Attachment #748845 - Flags: review?(bzbarsky)
(Reporter)

Comment 2

5 years ago
Created attachment 748848 [details] [diff] [review]
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.
Attachment #748848 - Flags: review?(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
Attachment #748845 - Flags: review?(bzbarsky) → review+
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....
Attachment #748848 - Flags: review?(bzbarsky) → review-
That's landed now if you want to rebase....
(Reporter)

Comment 6

5 years ago
Created attachment 751035 [details] [diff] [review]
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.
Attachment #748848 - Attachment is obsolete: true
Attachment #751035 - Flags: review?(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...
Attachment #751035 - Flags: review?(bzbarsky) → review-
(Reporter)

Comment 8

5 years ago
Created attachment 751045 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

Doh, how about this instead?
Attachment #751035 - Attachment is obsolete: true
Attachment #751045 - Flags: review?(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!
Attachment #751045 - Flags: review?(bzbarsky) → review-
(Reporter)

Comment 10

5 years ago
Created attachment 751068 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

Gah, I suck.  Sorry about that.  Third time's the charm?
Attachment #751045 - Attachment is obsolete: true
Attachment #751068 - Flags: review?(bzbarsky)
Comment on attachment 751068 [details] [diff] [review]
don't include AccessCheck.h in bindings unless necessary

r=me.  Good.  ;)
Attachment #751068 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/1f6740440eaa
https://hg.mozilla.org/mozilla-central/rev/ae494c31aab2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.