[Webkit-unassigned] [Bug 68207] Minor cleanups related to LayerChromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68207





--- Comment #2 from James Robinson <jamesr at chromium.org>  2011-09-15 18:58:32 PST ---
(In reply to comment #0)
> While studying LayerChromium to make unit tests, I came across several points that should be cleaned up.
> 
> 1. fix const correctness of preserves3D and replicaLayer accessors

Yes, please!

> 
> 2. remove default NULL argument in the LayerChromium::create function.  All existing instances of LayerChromium give an instance of the delegate, and it seems error prone to keep this default arg.

Yes, please!

> 
> 3.  re-name border functions to reflect that they are only for debug use

Yes, please!

> 
> 4. remove m_contentsDirty from LayerChromium, which had redundant and error-prone semantics with m_dirtyRect.  In particular, layers were using m_contentsDirty or m_dirtyRect, but not both.

I'm not sure what this means for layers that can have dirtyness vs layers that can only be entirely clean or entirely dirty.  There probably is room for improvement, though.

> 
> 5. NOT included in this first patch:  is it OK if we re-name isRootLayer to isNonCompositedContent ?   The only place the "isRootLayer" flag is being set is in the NonCompositedContentHost.  it seems appropriate to call it isNonCompositedContent() because (1) we gradually move away from having 3 different "root" layers (2) the notion of a root layer becomes more homogeneous with other layers

We have a root layer and we have a nonCompositedContent layer and they aren't quite the same thing.  I think LayerChromium::isRootLayer really does mean isNonCompositedContent, so that's a good rename to make.  In other places, it's not so clear.
> 
> 
> Please let me know what you think.

Most of these cleanups sound great (see above).  I would strongly prefer that you do the orthogonal cleanups in separate patches.  They will be small, but that means that they will be trivial to review + land with minimal chance of regression.  Also, having more patches to your name is a good thing (it'll help you earn committer status sooner, for example).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.