Reduce the number of places where we #include gfx/2d.h

RESOLVED FIXED in mozilla27

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

Trunk
mozilla27
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

This will hopefully help minimize the impact of this header across our code base.
Comment on attachment 811577 [details] [diff] [review]
Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h

Review of attachment 811577 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsIFrame.h
@@ +2931,5 @@
> +  uint8_t GetDisplay() const;
> +  bool IsFloating() const;
> +  bool IsPositioned() const;
> +  bool IsRelativelyPositioned() const;
> +  bool IsAbsolutelyPositioned() const;

I don't think it's wise to uninline all these!
Attachment #811577 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> Comment on attachment 811577 [details] [diff] [review]
> Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h
> 
> Review of attachment 811577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsIFrame.h
> @@ +2931,5 @@
> > +  uint8_t GetDisplay() const;
> > +  bool IsFloating() const;
> > +  bool IsPositioned() const;
> > +  bool IsRelativelyPositioned() const;
> > +  bool IsAbsolutelyPositioned() const;
> 
> I don't think it's wise to uninline all these!

OK, the reason I did that was to get rid of nsStyleStructInlines.h.  One option is to keep them inline, and move their implementation to a new file (nsIFrameInlines.h) which the few files which require the definitions of these methods would be able to #include.  What do you think?
Attachment #811577 - Attachment is obsolete: true
Attachment #812609 - Flags: review?(roc)
I think this breaks build with --disable-webgl.
Depends on: 923168
Depends on: 923170
Depends on: 923188
(In reply to Honza Bambas (:mayhemer) from comment #14)
> I think this breaks build with --disable-webgl.

Filed bug 923188.
Part 2 of this patch is wrong; it switches to forward declaring gfxPattern in gfxDrawable.h, but gfxPatternDrawable both includes an nsRefPtr<gfxPattern> as a member and implements its destructor directly in the header. This causes a compilation failure if someone includes gfxDrawable.h without including gfxPattern.h first, since the destructor of nsRefPtr<gfxPattern> needs the definition of gfxPattern to be visible.

Ehsan, could you please push a followup patch to fix this?
Normally I'd ask for a follow-up bug to be filed, but that would be kind of rude here.  ;-)
Attachment #817450 - Flags: review?(seth)
Comment on attachment 817450 [details] [diff] [review]
Fix comment 16

Review of attachment 817450 [details] [diff] [review]:
-----------------------------------------------------------------

Super duper. Thanks!
Attachment #817450 - Flags: review?(seth) → review+
You need to log in before you can comment on or make changes to this bug.