Bug 76637 - Update FloatRect::uniteIfNonZero to use isZero()
Summary: Update FloatRect::uniteIfNonZero to use isZero()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-19 08:13 PST by Philip Rogers
Modified: 2012-01-23 14:38 PST (History)
4 users (show)

See Also:


Attachments
Update uniteIfNonZero to use isZero (1.88 KB, patch)
2012-01-19 08:15 PST, Philip Rogers
no flags Details | Formatted Diff | Diff
Update uniteIfNonZero to use isZero (2.03 KB, patch)
2012-01-19 08:36 PST, Philip Rogers
no flags Details | Formatted Diff | Diff
Update uniteIfNonZero to use isZero (2.16 KB, patch)
2012-01-19 12:22 PST, Philip Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2012-01-19 08:13:04 PST
We should update uniteIfNonZero() to use isZero() instead of a !width() && !height().

See: https://bugs.webkit.org/show_bug.cgi?id=76177#c3
Comment 1 Philip Rogers 2012-01-19 08:15:47 PST
Created attachment 123133 [details]
Update uniteIfNonZero to use isZero
Comment 2 Philip Rogers 2012-01-19 08:36:07 PST
Created attachment 123135 [details]
Update uniteIfNonZero to use isZero

A note on why I added no tests: I did a little research on what this change will affect and I came up empty handed. I instrumented the code and ran all our tests and visited various popular/complex pages, neither of which registered a difference between the old behavior and the new. The primary use of uniteIfNonZero in regular code is in RenderInline.cpp which, as far as I can tell in my testing, only passes integer values through to this method and should not be affected by this change. I still think this is change will be clearly useful in preventing future bugs.
Comment 3 Alexey Proskuryakov 2012-01-19 12:09:01 PST
Comment on attachment 123135 [details]
Update uniteIfNonZero to use isZero

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

> Source/WebCore/ChangeLog:15
> +        * rendering/svg/SVGRenderSupport.cpp:
> +        (WebCore::SVGRenderSupport::computeContainerBoundingBoxes):

It would be helpful to explain this change in ChangeLog. Readers needn't spend effort trying to figure out how it's related to bug title when it's not!
Comment 4 Philip Rogers 2012-01-19 12:22:01 PST
Created attachment 123171 [details]
Update uniteIfNonZero to use isZero

Adding a comment in the ChangeLog so that readers are not confused by the unrelated change in SVGRenderSupport.cpp.
Comment 5 Darin Adler 2012-01-23 10:43:48 PST
Comment on attachment 123171 [details]
Update uniteIfNonZero to use isZero

OK
Comment 6 WebKit Review Bot 2012-01-23 14:38:09 PST
Comment on attachment 123171 [details]
Update uniteIfNonZero to use isZero

Clearing flags on attachment: 123171

Committed r105643: <http://trac.webkit.org/changeset/105643>
Comment 7 WebKit Review Bot 2012-01-23 14:38:13 PST
All reviewed patches have been landed.  Closing bug.