Closed Bug 911292 Opened 6 years ago Closed 6 years ago

Minimize the #includes in layout/mathml

Categories

(Core :: Layout, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Blocks: includehell
Comment on attachment 797955 [details] [diff] [review]
Patch (v1)

>layout/mathml/nsMathMLChar.cpp
>-#include "nsFrame.h"

I think you should change it to "nsIFrame.h" instead, there's lot's of
nsIFrame usage in this file and we should make that dependency explicit
(and this is not a frame subclass file).


>layout/mathml/nsMathMLChar.h
>-#include "nsMathMLFrame.h"

I suspect you should add "class nsIFrame;" here, but because
nsMathMLChar.cpp includes its header last I guess it compiles.
We should move that include first to make sure nsMathMLChar.h
is self-contained.


>layout/mathml/nsMathMLContainerFrame.cpp
> #include "nsFrame.h"

Is there a reason to keep this?  We can count on nsMathMLContainerFrame.h
having included all our base-class headers.

> #include "nsMathMLContainerFrame.h"

This should be the first include.


>layout/mathml/nsMathMLFrame.cpp
> #include "nsMathMLFrame.h"

This should be the first include.

> #include "nsAutoPtr.h"

not used?


>layout/mathml/nsMathMLOperators.cpp
> #include "nsMathMLOperators.h"

This should be the first include.

>layout/mathml/nsMathMLOperators.h

This file should include "nsString.h" becaues it needs nsDependentSubstring.


>layout/mathml/nsMathMLTokenFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLTokenFrame.h"

move first in the include list

> #include "nsTextFrame.h"

is this needed?


>layout/mathml/nsMathMLmactionFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmactionFrame.h"

move first


>layout/mathml/nsMathMLmencloseFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmencloseFrame.h"

move first


>layout/mathml/nsMathMLmfencedFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmfencedFrame.h"

move first


>layout/mathml/nsMathMLmfracFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmfracFrame.h"

move first


>layout/mathml/nsMathMLmmultiscriptsFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmmultiscriptsFrame.h"

move first


>layout/mathml/nsMathMLmoFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmoFrame.h"

move first


>layout/mathml/nsMathMLmpaddedFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmpaddedFrame.h"

move first


>layout/mathml/nsMathMLmphantomFrame.cpp
>+#include "nsIFrame.h"

Unnecessary since this a frame class impl file.


>layout/mathml/nsMathMLmrootFrame.cpp
> #include "nsFrame.h"

remove 

> #include "nsMathMLmrootFrame.h"

move first


>layout/mathml/nsMathMLmrowFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmrowFrame.h"

move first


>layout/mathml/nsMathMLmspaceFrame.cpp
> #include "nsFrame.h"

remove


>layout/mathml/nsMathMLmstyleFrame.cpp
> #include "nsFrame.h"

remove


>layout/mathml/nsMathMLmtableFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsBlockFrame.h"

move to nsMathMLmtableFrame.h

> #include "nsTableRowFrame.h"

move to nsMathMLmtableFrame.h

> #include "nsTableOuterFrame.h"

move to nsMathMLmtableFrame.h

> #include "nsTableFrame.h"

move to nsMathMLmtableFrame.h

> #include "nsTableCellFrame.h"

move to nsMathMLmtableFrame.h

> #include "nsMathMLmtableFrame.h"

move first


>layout/mathml/nsMathMLmunderoverFrame.cpp
> #include "nsFrame.h"

remove

> #include "nsMathMLmunderoverFrame.h"

move first
Attachment #797955 - Flags: review?(matspal) → review+
(In reply to Mats Palmgren (:mats) from comment #1)
> >layout/mathml/nsMathMLChar.h
> >-#include "nsMathMLFrame.h"
> 
> I suspect you should add "class nsIFrame;" here, but because
> nsMathMLChar.cpp includes its header last I guess it compiles.
> We should move that include first to make sure nsMathMLChar.h
> is self-contained.

OK, I had to do some surgery to make that work...

> >layout/mathml/nsMathMLFrame.cpp
> > #include "nsMathMLFrame.h"
> 
> This should be the first include.
> 
> > #include "nsAutoPtr.h"
> 
> not used?

nsRefPtr is used in this file.

> >layout/mathml/nsMathMLOperators.h
> 
> This file should include "nsString.h" becaues it needs nsDependentSubstring.

nsStringFwd.h is enough here actually.

> > #include "nsTextFrame.h"
> 
> is this needed?

Yes, for TEXT_FORCE_TRIM_WHITESPACE.

Addressed the rest of your comments!
https://hg.mozilla.org/mozilla-central/rev/faaee438ce6d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.