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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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.