Last Comment Bug 677101 - Various #include improvements
: Various #include improvements
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla8
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
:
Mentors:
Depends on:
Blocks: includehell
  Show dependency treegraph
 
Reported: 2011-08-07 11:33 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2013-08-13 11:04 PDT (History)
4 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part a: Create nsAutoLayoutPhase.h (9.56 KB, patch)
2011-08-07 11:41 PDT, :Ms2ger (⌚ UTC+1/+2)
tnikkel: review+
Ms2ger: checkin+
Details | Diff | Splinter Review
Part b: Remove nsContentUtils.h includes from headers (82.17 KB, patch)
2011-08-07 11:42 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part c: Reduce nsIDOMText.h inclusions (24.33 KB, patch)
2011-08-07 11:43 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part d: Remove some includes from nsHTMLInputElement.cpp (884 bytes, patch)
2011-08-07 11:44 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-08-07 11:33:40 PDT

    
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2011-08-07 11:41:57 PDT
Created attachment 551327 [details] [diff] [review]
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 (⌚ UTC+1/+2) 2011-08-07 11:42:35 PDT
Created attachment 551328 [details] [diff] [review]
Part b: Remove nsContentUtils.h includes from headers
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-08-07 11:43:16 PDT
Created attachment 551329 [details] [diff] [review]
Part c: Reduce nsIDOMText.h inclusions
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-08-07 11:44:18 PDT
Created attachment 551330 [details] [diff] [review]
Part d: Remove some includes from nsHTMLInputElement.cpp
Comment 5 Timothy Nikkel (:tnikkel) 2011-08-07 12:16:22 PDT
So what is the goal here?
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-08-07 12:24:18 PDT
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) 2011-08-07 21:02:12 PDT
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 (⌚ UTC+1/+2) 2011-08-08 01:42:19 PDT
Will do.
Comment 9 Mounir Lamouri (:mounir) 2011-08-08 15:32:51 PDT
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) 2011-08-08 15:35:05 PDT
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) 2011-08-08 15:36:02 PDT
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 (⌚ UTC+1/+2) 2011-08-09 02:17:48 PDT
Comment on attachment 551327 [details] [diff] [review]
Part a: Create nsAutoLayoutPhase.h

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

Note You need to log in before you can comment on or make changes to this bug.