677101 - Various #include improvements

Status: RESOLVED FIXED

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

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: Create nsAutoLayoutPhase.h

This is necessary because nsAutoLayoutPhase uses nsContentUtils, which I don't want to include in nsPresContext.h

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: Remove nsContentUtils.h includes from headers

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: Reduce nsIDOMText.h inclusions

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part d: Remove some includes from nsHTMLInputElement.cpp

Comment 5

[Timothy Nikkel (:tnikkel)]

So what is the goal here?

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Not including things unnecessarily, and in particular not including nsContentUtils.h from headers, because it includes a lot of stuff itself.

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 551327 [details] [diff] [review]
Part a: Create nsAutoLayoutPhase.h

>diff --git a/layout/base/nsAutoLayoutPhase.h b/layout/base/nsAutoLayoutPhase.h
>+
>+#ifndef nsAutoLayoutPhase_h
>+#define nsAutoLayoutPhase_h
>+
>+#include "nsPresContext.h"
>+#include "nsContentUtils.h"
>+
>+#ifdef DEBUG

Move the "#ifdef DEBUG" above the other includes?

r+ assuming this is basically a straight copy of code from one place to another.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Will do.

Comment 9

[Mounir Lamouri (:mounir)]

Comment on attachment 551328 [details] [diff] [review]
Part b: Remove nsContentUtils.h includes from headers

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

Do whatever you want with my comments (they are only suggestions).

r=mounir

::: content/svg/content/src/nsSVGLength2.h
@@ +175,5 @@
>      NS_IMETHOD SetValue(float aValue)
>        {
> +        if (!NS_finite(aValue)) {
> +          return NS_ERROR_ILLEGAL_VALUE;
> +        }

Maybe add a comment explaining that you don't use NS_ENSURE_FINITE to prevent including nsContentUtils.h?

@@ +186,5 @@
>      NS_IMETHOD SetValueInSpecifiedUnits(float aValue)
>        {
> +        if (!NS_finite(aValue)) {
> +          return NS_ERROR_ILLEGAL_VALUE;
> +        }

Same here.

::: content/svg/content/src/nsSVGNumber2.h
@@ +112,5 @@
>      NS_IMETHOD SetBaseVal(float aValue)
>        {
> +        if (!NS_finite(aValue)) {
> +          return NS_ERROR_ILLEGAL_VALUE;
> +        }

and here..

::: content/svg/content/src/nsSVGNumberPair.h
@@ +121,5 @@
>      NS_IMETHOD SetBaseVal(float aValue)
>        {
> +        if (!NS_finite(aValue)) {
> +          return NS_ERROR_ILLEGAL_VALUE;
> +        }

oh, here too :)

Comment 10

[Mounir Lamouri (:mounir)]

Comment on attachment 551329 [details] [diff] [review]
Part c: Reduce nsIDOMText.h inclusions

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

I believe you checked that nothing in the includes you removed were actually used?

r=mounir assuming that.

Comment 11

[Mounir Lamouri (:mounir)]

Comment on attachment 551330 [details] [diff] [review]
Part d: Remove some includes from nsHTMLInputElement.cpp

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

r=mounir

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 551327 [details] [diff] [review]
Part a: Create nsAutoLayoutPhase.h

http://hg.mozilla.org/mozilla-central/rev/36989c74b287

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/f4537a268e6f
http://hg.mozilla.org/mozilla-central/rev/eeec7694bf6a
http://hg.mozilla.org/mozilla-central/rev/134ee27f55a7