Closed
Bug 833009
Opened 10 years ago
Closed 10 years ago
Remove nsContentUtils.h inclusion from header files
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: mounir, Assigned: mounir)
References
(Depends on 1 open bug)
Details
Attachments
(7 files, 2 obsolete files)
12.69 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
3.46 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
713 bytes,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
3.50 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
4.09 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
6.40 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
6.17 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #704572 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #704573 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #704574 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #704575 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #704577 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #704578 -
Flags: review?(Ms2ger)
Comment 8•10 years ago
|
||
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 9•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #704574 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 10•10 years ago
|
||
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 11•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #704577 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 12•10 years ago
|
||
I attached the wrong patch.
Attachment #704572 -
Attachment is obsolete: true
Attachment #704580 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #704578 -
Attachment is obsolete: true
Attachment #704578 -
Flags: review?(Ms2ger)
Attachment #704581 -
Flags: review?(Ms2ger)
Comment 14•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #704581 -
Flags: review?(Ms2ger) → review+
Comment 15•10 years ago
|
||
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 16•10 years ago
|
||
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 17•10 years ago
|
||
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+
Assignee | ||
Comment 18•10 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ae479d35c6 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/00a33fee3c87 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/cd11feb55873 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/02e8dcde27b7 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/32831201b9ef remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7c676b4c0d53 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f28e2f533c56 A big chunk of the nsContentUtils.h inclusions have been removed. Let's do the remaining parts in follow-ups.
Flags: in-testsuite-
Target Milestone: --- → mozilla21
Comment 19•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a2ae479d35c6 https://hg.mozilla.org/mozilla-central/rev/00a33fee3c87 https://hg.mozilla.org/mozilla-central/rev/cd11feb55873 https://hg.mozilla.org/mozilla-central/rev/02e8dcde27b7 https://hg.mozilla.org/mozilla-central/rev/32831201b9ef https://hg.mozilla.org/mozilla-central/rev/7c676b4c0d53 https://hg.mozilla.org/mozilla-central/rev/f28e2f533c56
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•