Closed Bug 847110 Opened 8 years ago Closed 8 years ago

Fix up includes in layout/style


(Core :: CSS Parsing and Computation, defect)

Not set





(Reporter: dzbarsky, Assigned: dzbarsky)



(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
No description provided.
Attachment #720337 - Flags: review?(dbaron)
Attached patch PatchSplinter Review
Attachment #720337 - Attachment is obsolete: true
Attachment #720337 - Flags: review?(dbaron)
Attachment #720338 - Flags: review?(dbaron)
Could you explain what led you to make this set of changes?  (Was the goal to reduce a particular set of #includes?  What set?)
I remembered having to rebuild seemingly random parts of the tree when changing code under layout/style so I figured if less headers include each other it would make rebuilding easier.  This patch is the output of running include-what-you-use, which I fixed up.
So the pattern of using:
  class Foo;
  friend class Foo;
  class Foo {
for nested classes is needed in C++98 (where nested classes don't automatically have access to private/protected members of their outer class) but not in C++11.  I'd prefer to leave all of those rather than removing them; some ports might still be depending on those.
Comment on attachment 720338 [details] [diff] [review]

>diff --git a/layout/style/nsCSSValue.h b/layout/style/nsCSSValue.h
>--- a/layout/style/nsCSSValue.h
>+++ b/layout/style/nsCSSValue.h
>@@ -23,18 +23,16 @@
> #include "nsStyleConsts.h"
> #include "mozilla/FloatingPoint.h"
> class imgRequestProxy;
> class nsIDocument;
> class nsIPrincipal;
> class nsPresContext;
> class nsIURI;
>-template <class T>
>-class nsPtrHashKey;

Seems like you ought to leave this as well.

(also see previous comment)

Otherwise the patch looks fine.

So r=dbaron with those parts left.  (Note that there are quite a few nested classes where you removed the friend declarations and the declarations needed before the friend declarations.)
Attachment #720338 - Flags: review?(dbaron) → review+
Landed with all the friend declarations put back.
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.