Bug 227186 – Remove redundant HitTestLocation(const LayoutPoint& centerPoint, unsigned topPadding, unsigned rightPadding, unsigned bottomPadding, unsigned leftPadding)

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

Let's replace the _one_ caller (Internals) with the LayoutRect version.

[alan]

Created attachment 431807 [details]
Patch

[alan]

Created attachment 431819 [details]
Patch

[Sam Weinig]

Comment on attachment 431819 [details]
Patch

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

> Source/WebCore/testing/Internals.cpp:2323
> +    auto hitTestResult = HitTestResult { };
> +    auto size = LayoutSize { leftPadding + rightPadding, topPadding + bottomPadding };
> +    if (size.isEmpty())
> +        hitTestResult = HitTestResult { point };
> +    else {
> +        auto adjustedPosition = LayoutPoint { flooredIntPoint(point) };
> +        adjustedPosition -= LayoutSize  { leftPadding, topPadding };
> +        hitTestResult = HitTestResult { LayoutRect { adjustedPosition, size } };
> +    }

I would usually do this using a lambda so that hitTestResult is not assigned twice.

Can the two adjustedPosition lines be written as one line? 

auto adjustedPosition = LayoutPoint { flooredIntPoint(point) } - LayoutSize { leftPadding, topPadding };

?

[alan]

(In reply to Sam Weinig from comment #3)
> Comment on attachment 431819 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431819&action=review
> 
> > Source/WebCore/testing/Internals.cpp:2323
> > +    auto hitTestResult = HitTestResult { };
> > +    auto size = LayoutSize { leftPadding + rightPadding, topPadding + bottomPadding };
> > +    if (size.isEmpty())
> > +        hitTestResult = HitTestResult { point };
> > +    else {
> > +        auto adjustedPosition = LayoutPoint { flooredIntPoint(point) };
> > +        adjustedPosition -= LayoutSize  { leftPadding, topPadding };
> > +        hitTestResult = HitTestResult { LayoutRect { adjustedPosition, size } };
> > +    }
> 
> I would usually do this using a lambda so that hitTestResult is not assigned
> twice.
I normally go overboard with lambdas, not sure what happened here. :)

> 
> Can the two adjustedPosition lines be written as one line? 
> 
> auto adjustedPosition = LayoutPoint { flooredIntPoint(point) } - LayoutSize
> { leftPadding, topPadding };
> 
> ?
Yup

Thanks.

[alan]

Created attachment 431830 [details]
Patch

[alan]

Created attachment 431850 [details]
Patch

[EWS]

Committed r279066 (238986@main): <https://commits.webkit.org/238986@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431850 [details].

[Radar WebKit Bug Importer]

<rdar://problem/79562100>