Bug 89503

Summary: [Chromium] Remove redundant #includes in compositor
Product: WebKit Reporter: zlieber
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, eric.carlson, feature-media-reviews, jamesr, kbr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description zlieber 2012-06-19 13:03:27 PDT
[Chromium] Remove redundant #include's in compositor
Comment 1 zlieber 2012-06-19 13:06:16 PDT
Created attachment 148400 [details]
Patch
Comment 2 Adrienne Walker 2012-06-19 13:56:26 PDT
Comment on attachment 148400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148400&action=review

> Source/WebCore/ChangeLog:10
> +        Removed some redundant #include's to clarify dependency
> +        picture. Some 90 #include's removed, some 50 less disruptive
> +        #include's added. This brings the number of class dependencies to

grammar: s/'//g.  No grocers' apostrophes.  ;)

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.h:39
> +#include "Path.h"

Forward declare this instead.

> Source/WebCore/platform/graphics/chromium/RenderSurfaceChromium.h:32
> +#include "Color.h"

What is this doing?

> Source/WebCore/platform/graphics/chromium/cc/CCRenderer.h:44
> +enum TextureUploaderOption { ThrottledUploader, UnthrottledUploader };
> +

I think this is an LRC detail, not a CCRenderer one.
Comment 3 zlieber 2012-06-19 14:16:21 PDT
Created attachment 148418 [details]
Patch
Comment 4 Adrienne Walker 2012-06-19 14:26:53 PDT
Comment on attachment 148418 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148418&action=review

R=me.  Moving the TextureUploaderOption makes sense given the CCLTHI layer renderer creation method.  Maybe that can get cleaned up after we have a real software renderer implementation.

Thanks for all this cleanup.  It's obviously needed!

> Source/WebCore/ChangeLog:3
> +        [Chromium] Remove redundant #include's in compositor

ಠ_ಠ
Comment 5 zlieber 2012-06-19 14:37:05 PDT
(In reply to comment #4)
> (From update of attachment 148418 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148418&action=review
> 
> R=me.  Moving the TextureUploaderOption makes sense given the CCLTHI layer renderer creation method.  Maybe that can get cleaned up after we have a real software renderer implementation.
> 
> Thanks for all this cleanup.  It's obviously needed!
> 
> > Source/WebCore/ChangeLog:3
> > +        [Chromium] Remove redundant #include's in compositor
> 
> ಠ_ಠ

Arghh!! missed that one
Comment 6 zlieber 2012-06-19 14:38:52 PDT
Created attachment 148430 [details]
Patch
Comment 7 WebKit Review Bot 2012-06-19 16:46:43 PDT
Comment on attachment 148430 [details]
Patch

Clearing flags on attachment: 148430

Committed r120772: <http://trac.webkit.org/changeset/120772>
Comment 8 WebKit Review Bot 2012-06-19 16:46:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Kenneth Russell 2012-06-19 17:07:16 PDT
Reverted r120772 for reason:

Broke build on Chromium Mac

Committed r120775: <http://trac.webkit.org/changeset/120775>
Comment 10 Kenneth Russell 2012-06-19 17:08:47 PDT
Sorry, I had to roll this out as it broke the build. Sample failure:

http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/38407/steps/compile-webkit/logs/stdio

Looks simple to fix -- change of a forward declaration from class to struct.
Comment 11 zlieber 2012-06-19 17:26:58 PDT
Created attachment 148469 [details]
Patch
Comment 12 WebKit Review Bot 2012-06-20 06:53:06 PDT
Comment on attachment 148469 [details]
Patch

Clearing flags on attachment: 148469

Committed r120820: <http://trac.webkit.org/changeset/120820>
Comment 13 WebKit Review Bot 2012-06-20 06:53:12 PDT
All reviewed patches have been landed.  Closing bug.