[chromium] Move draw time properties out of *LayerChromium to CCLayerImpl

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

[chromium] Move draw time properties out of *LayerChromium to CCLayerImpl

[James Robinson]

Created attachment 83431 [details]
Patch

[James Robinson]

This patch is (in essence) a strict subset of https://bugs.webkit.org/show_bug.cgi?id=54047.  IMO it's big enough and conflict-likely enough to land by itself, but small enough to be a low-risk landing so I'd like to get it in before there is too much more surgery on these files.  Most of the lines touched are changes that switch to use a getter/setter rather than setting member variables directly.

Conceptually this patch doesn't do much except prepare for splitting the draw responsibilities away from what currently exists as the LayerChromium tree.  The patch at https://bugs.webkit.org/show_bug.cgi?id=54047 shows what this looks like after the update and draw steps are divided more cleanly in LayerRendererChromium's internals.

[Kenneth Russell]

Vangelis, could you do the unofficial review of this patch?

[Vangelis Kokkevis]

Comment on attachment 83431 [details]
Patch

Is the plan to also rename RenderSurfaceChromium to something like ccRenderSurface and move it to the cc directory?  
Is there any reason you call the new class CCLayerImpl instead of plain CCLayer ? 

Like you mention in the description, there's still a bunch of code that will need to change to break the connections between CCLayers and LayerChromium's but I think this is a fine first step.

[James Robinson]

(In reply to comment #4)
> (From update of attachment 83431 [details])
> Is the plan to also rename RenderSurfaceChromium to something like ccRenderSurface and move it to the cc directory?

Yup, that will happen as well.  It seemed premature to do that yet as it adds more churn and RenderSurfaceChromium is still being used directly by some of the LayerChromium subclasses.

> Is there any reason you call the new class CCLayerImpl instead of plain CCLayer ? 
> 

Nat pointed out that it makes more sense to name the interface we exposed to the rest of WebKit CCLayer and have CCLayerImpl be the compositor's internal implementation.  IOW GraphicsLayerChromium will have a set of CCLayers and set properties on them, etc, and only the compositor internals will ever have knowledge of CCLayerImpl.

> Like you mention in the description, there's still a bunch of code that will need to change to break the connections between CCLayers and LayerChromium's but I think this is a fine first step.

[Nat Duca]

I could imagine doing a similar change to LayerRendrerChromium too, as a subsequent patch perhaps.

[James Robinson]

So everyone is OK with this?

[Adrienne Walker]

(In reply to comment #7)
> So everyone is OK with this?

No complaints here.  It makes a lot of sense to commit this large but low-risk change first.

[Kenneth Russell]

Comment on attachment 83431 [details]
Patch

Thumbs up in that case.

[James Robinson]

Committed r79659: <http://trac.webkit.org/changeset/79659>

[WebKit Review Bot]

http://trac.webkit.org/changeset/79659 might have broken Leopard Intel Debug (Build)