Bug 65396

Summary: Switch RenderBlock to to new layout types
Product: WebKit Reporter: Emil A Eklund <eae>
Component: Layout and RenderingAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, eric, leviw, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 63567    
Attachments:
Description Flags
Patch
none
Patch none

Description Emil A Eklund 2011-07-29 15:01:06 PDT
Convert RenderBlock to new layout abstraction.
Comment 1 Emil A Eklund 2011-07-29 15:11:21 PDT
Created attachment 102403 [details]
Patch
Comment 2 Darin Adler 2011-07-30 11:01:52 PDT
Comment on attachment 102403 [details]
Patch

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

No EWS because the patch doesn’t apply.

I suggest putting in a patch that only adds the new typedef and is entirely mechanical, and leaving the part that introduces new functions as a separate far smaller patch.

> Source/WebCore/platform/graphics/IntRect.cpp:142
> +IntRect enclosingIntRect(const IntRect& rect)
> +{
> +    return IntRect(rect);
> +}

This must be inlined in the header. We do not want to pay a function call to convert an IntRect to an IntRect.

> Source/WebCore/rendering/LayoutTypes.h:89
> +inline bool layoutUnitEquals(LayoutUnit a, LayoutUnit b)

If the idea here is to use a definition that involves epsilon, then I think you need a different name. After all, == does not use epsilon, so the name "equal" is already taken.
Comment 3 Emil A Eklund 2011-08-01 13:12:13 PDT
Created attachment 102546 [details]
Patch
Comment 4 Emil A Eklund 2011-08-01 13:12:37 PDT
Thanks Darin, made the changes you suggested. Please take another look.
Comment 5 Eric Seidel (no email) 2011-08-08 09:28:01 PDT
Comment on attachment 102546 [details]
Patch

LGTM.  You may want to let Darin take another look too.
Comment 6 Emil A Eklund 2011-08-08 13:38:53 PDT
Thanks Eric,

I'll hold of landing this for another day or so to give Darin a chance to look at it.
Comment 7 WebKit Review Bot 2011-08-09 14:52:54 PDT
Comment on attachment 102546 [details]
Patch

Clearing flags on attachment: 102546

Committed r92712: <http://trac.webkit.org/changeset/92712>
Comment 8 WebKit Review Bot 2011-08-09 14:53:03 PDT
All reviewed patches have been landed.  Closing bug.