Bug 37633

Summary:

Spatial Navigation: use a static var in maxDistance method ( SpatialNavigation.h )

Product:

WebKit

Reporter:

Antonio Gomes <tonikitoo>

Component:

Accessibility

Assignee:

Antonio Gomes <tonikitoo>

Status:

RESOLVED INVALID

Severity:

Enhancement

CC:

kenneth, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
patch v1	eric: review-, tonikitoo: commit-queue-

Antonio Gomes

Reported 2010-04-14 20:48:17 PDT

bug title says it all ps: it is a minor, but good to get in patch coming...

Attachments
patch v1 (1.66 KB, patch) 2010-04-14 20:51 PDT, Antonio Gomes	eric: review- tonikitoo: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Antonio Gomes

Comment 1 2010-04-14 20:51:27 PDT

Created attachment 53403 [details] patch v1

Antonio Gomes

Comment 2 2010-05-08 20:33:39 PDT

kenneth, could you sr that minor one?

Antonio Gomes

Comment 3 2010-05-08 20:48:03 PDT

note: I am also ok with not getting this change in at all if we agree compilers are smart enough so that it is unneeded.

Eric Seidel (no email)

Comment 4 2010-05-09 14:01:01 PDT

Comment on attachment 53403 [details] patch v1 Seems like this makes no sense as an inline if you're doing this. Why do we want this? Is this a performance win? numeric_limits<long long>::max() should already be a static lookup, no?

Kenneth Rohde Christiansen

Comment 5 2010-05-09 16:33:16 PDT

(In reply to comment #4) > (From update of attachment 53403 [details]) > Seems like this makes no sense as an inline if you're doing this. > > Why do we want this? Is this a performance win? numeric_limits<long long>::max() should already be a static lookup, no? I agree, this seems quite strange. Without data on performance improvements this will not be considered for inclusion.

Antonio Gomes

Comment 6 2010-05-09 19:29:32 PDT

(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 53403 [details] [details]) > > Seems like this makes no sense as an inline if you're doing this. > > > > Why do we want this? Is this a performance win? numeric_limits<long long>::max() should already be a static lookup, no? > > I agree, this seems quite strange. Without data on performance improvements this will not be considered for inclusion. At the time I made this small patch, I was questioning myself about it, and if it would be something that could get any better for the code, but I also agree it is fine as is, and that the change brings no logical win.

Note You need to log in before you can comment on or make changes to this bug.