[webkit-reviews] review granted: [Bug 11844] Code Cleanup for more of the rendering code : [Attachment 11887] updated patch

[bugzilla-request-daemon at macosforge.org]

mitz at webkit.org has granted mitz at webkit.org's request for review:
Bug 11844: Code Cleanup for more of the rendering code
http://bugs.webkit.org/show_bug.cgi?id=11844

Attachment 11887: updated patch
http://bugs.webkit.org/attachment.cgi?id=11887&action=edit

------- Additional Comments from mitz at webkit.org
r=me, although I'd prefer if you addressed the following comments before
landing:

Use the class name 'RenderText' instead of 'rendertext':

+    // The text runs point to parts of the rendertext's m_str

I'm fine with you just adding these comments for now, but it's wrong to call
these values 'random'. Use 'arbitrary' instead.

+    // FIXME: we should not use a random value like this.  Perhaps we should
use INT_MAX.

+    // FIXME: we should not use a random value like this.  Perhaps we should
use INT_MAX.

Here I think it's best to leave 'sizes' since otherwise it's not obvious that
the parameter is pointing to an array:

-    void setControlSize(NSCell*, const IntSize* sizes, const IntSize&
minSize);
-    void setSizeFromFont(RenderStyle*, const IntSize* sizes) const;
-    IntSize sizeForFont(RenderStyle*, const IntSize* sizes) const;
-    IntSize sizeForSystemFont(RenderStyle*, const IntSize* sizes) const;
+    void setControlSize(NSCell*, const IntSize*, const IntSize& minSize);
+    void setSizeFromFont(RenderStyle*, const IntSize*) const;
+    IntSize sizeForFont(RenderStyle*, const IntSize*) const;
+    IntSize sizeForSystemFont(RenderStyle*, const IntSize*) const;