Bug 77914

Summary: Update LayoutUnit usage in ColumnInfo and RenderFrameSet
Product: WebKit Reporter: Levi Weintraub <leviw@chromium.org>
Component: Layout and RenderingAssignee: Levi Weintraub <leviw@chromium.org>
Status: RESOLVED FIXED    
Severity: Normal CC: darin@apple.com, eae@chromium.org, eric@webkit.org
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60318    
Attachments:
Description Flags
Patch
none
Patch none

Description From 2012-02-06 16:58:43 PST
There are a handfull of functions and variables that should be switched to LayoutUnits from integers.
------- Comment #1 From 2012-02-06 17:03:19 PST -------
Created an attachment (id=125732) [details]
Patch
------- Comment #2 From 2012-02-06 18:19:40 PST -------
(From update of attachment 125732 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=125732&action=review

> Source/WebCore/rendering/ColumnInfo.h:76
> +    LayoutUnit forcedBreaks() const { return m_forcedBreaks; }

Is this a count?
------- Comment #3 From 2012-02-07 10:13:31 PST -------
Created an attachment (id=125869) [details]
Patch
------- Comment #4 From 2012-02-07 10:14:50 PST -------
(In reply to comment #2)
> (From update of attachment 125732 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=125732&action=review
> 
> > Source/WebCore/rendering/ColumnInfo.h:76
> > +    LayoutUnit forcedBreaks() const { return m_forcedBreaks; }
> 
> Is this a count?

Good catch! I had the functions swapped.
------- Comment #5 From 2012-02-07 14:16:04 PST -------
(From update of attachment 125869 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=125869&action=review

> Source/WebCore/ChangeLog:8
> +        Updating usage of LayoutUnits in ColumnInfo and RenderFrameSet.

You should clarify in your Changelog (or code) why these are different from table columns.

All these changes are fine, but I think you should spend a bit more effort on the *why* explanations. :)
------- Comment #6 From 2012-02-07 14:17:03 PST -------
(From update of attachment 125869 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=125869&action=review

>> Source/WebCore/ChangeLog:8
>> +        Updating usage of LayoutUnits in ColumnInfo and RenderFrameSet.
> 
> You should clarify in your Changelog (or code) why these are different from table columns.
> 
> All these changes are fine, but I think you should spend a bit more effort on the *why* explanations. :)

Will do. Thanks for the review!
------- Comment #7 From 2012-02-07 16:52:44 PST -------
Committed r107009: <http://trac.webkit.org/changeset/107009>
------- Comment #8 From 2012-02-07 16:56:27 PST -------
(From update of attachment 125869 [details])
Clearing Flags