Closed Bug 833009 Opened 7 years ago Closed 7 years ago

Remove nsContentUtils.h inclusion from header files

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: mounir, Assigned: mounir)

References

(Depends on 1 open bug)

Details

Attachments

(7 files, 2 obsolete files)

Including nsContentUtils.h in a header is not a very good idea. We should prevent that as much as we can. I will attach a few simple patches to clean up nsContentUtils.h inclusion and will open a couple of blocking bugs that do more complex stuff (especially I'm sad that Element.h includes nsContentUtils.h).
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #704570 - Flags: review?(bzbarsky)
Attached patch Cleanup in content/html/ (obsolete) — Splinter Review
Attachment #704572 - Flags: review?(Ms2ger)
Attachment #704573 - Flags: review?(Ms2ger)
Attached patch Cleanup in gfx/Splinter Review
Attachment #704574 - Flags: review?(Ms2ger)
Attachment #704575 - Flags: review?(Ms2ger)
Attachment #704577 - Flags: review?(Ms2ger)
Attached patch Cleanup in content/base/ (obsolete) — Splinter Review
Attachment #704578 - Flags: review?(Ms2ger)
Comment on attachment 704572 [details] [diff] [review]
Cleanup in content/html/

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

::: content/base/public/nsViewportInfo.h
@@ +4,5 @@
>  
>  #ifndef nsViewportInfo_h___
>  #define nsViewportInfo_h___
>  
> +#include <stdint.h>

mozilla/StandardInteger.h, still
Attachment #704572 - Flags: review?(Ms2ger) → review-
Comment on attachment 704573 [details] [diff] [review]
Cleanup in content/events/

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

No ESM reviews for me, I don't want to be eaten by a dragon :)
Attachment #704573 - Flags: review?(Ms2ger) → review?(bugs)
Attachment #704574 - Flags: review?(Ms2ger) → review+
With all those patches applied, looking for "nsContentUtils.h" in header files returns:
content/xslt/src/xpath/txXPathNode.h:#include "nsContentUtils.h" // For NameSpaceManager().
content/xslt/src/base/txStringUtils.h:#include "nsContentUtils.h" // For ASCIIToLower().
content/xbl/src/nsXBLProtoImplMember.h:#include "nsContentUtils.h" // For NS_CONTENT_DELETE_LIST_MEMBER.
content/base/src/mozAutoDocUpdate.h:#include "nsContentUtils.h" // For AddScriptBlocker() and RemoveScriptBlocker().
content/base/public/Element.h:#include "nsContentUtils.h"
dom/bindings/CallbackFunction.h:#include "nsContentUtils.h"

I think the big fish is Element.h because it is included every where and would explain why changing nsContentUtils.h make us rebuild nearly the entire tree.
Comment on attachment 704575 [details] [diff] [review]
Cleanup in content/xbl/

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

::: content/xbl/src/nsXBLResourceLoader.cpp
@@ +56,5 @@
> +    mType = aType;
> +    mSrc = aSrc;
> +  }
> +
> +  ~nsXBLResource() { 

Nit: all three {s on their own line.

@@ +57,5 @@
> +    mSrc = aSrc;
> +  }
> +
> +  ~nsXBLResource() { 
> +    MOZ_COUNT_DTOR(nsXBLResource);  

Please fix the trailing whitespace while you're here.
Attachment #704575 - Flags: review?(Ms2ger) → review+
Attachment #704577 - Flags: review?(Ms2ger) → review+
I attached the wrong patch.
Attachment #704572 - Attachment is obsolete: true
Attachment #704580 - Flags: review?(Ms2ger)
Attachment #704578 - Attachment is obsolete: true
Attachment #704578 - Flags: review?(Ms2ger)
Attachment #704581 - Flags: review?(Ms2ger)
Comment on attachment 704573 [details] [diff] [review]
Cleanup in content/events/

Looks like this isn't used in super-hot code paths, so ok.

And I assume nsViewManager change is needed because ESM.h doesn't include
nsContentUtils.h anymore.
Attachment #704573 - Flags: review?(bugs) → review+
Depends on: 833012
Attachment #704581 - Flags: review?(Ms2ger) → review+
Comment on attachment 704580 [details] [diff] [review]
Cleanup in content/html/

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

r=me modulo SetContentEditable
Attachment #704580 - Flags: review?(Ms2ger) → review+
Comment on attachment 704580 [details] [diff] [review]
Cleanup in content/html/

Why does SetContentEditable need to end up out of line?  I don't see how it's using nsContentUtils.
Comment on attachment 704570 [details] [diff] [review]
Cleanup in layout/

I'd really like to keep WillCauseReflow inline....

Can we just move scriptblockers to a different header containing just them?

r=me on the rest
Attachment #704570 - Flags: review?(bzbarsky) → review+
Depends on: 833928
You need to log in before you can comment on or make changes to this bug.