888318 - Use CSSIntPoint for nsGlobalWindow::ScrollTo

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 768975 [details] [diff] [review]
Patch v1

Review of attachment 768975 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok to me. Again I would prefer to also get a r+ from somebody who is a peer or owner of this code.

::: dom/base/nsGlobalWindow.cpp
@@ +6032,5 @@
> +}
> +
> +void
> +nsGlobalWindow::ScrollTo(const CSSIntPoint& aScroll)
> +{

I think the compiler would be able to optimize this better if you changed the signature to ScrollTo(CSSIntPoint aScroll) and didn't copy it to a local "scroll" variable inside the body. That way it should be able to get away without copying it at all at call sites, whereas here it must make the copy in this compilation unit regardless.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 768975 [details] [diff] [review]
Patch v1

Review of attachment 768975 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsGlobalWindow.cpp
@@ +6039,5 @@
>  
>    if (sf) {
>      // Here we calculate what the max pixel value is that we can
>      // scroll to, we do this by dividing maxint with the pixel to
>      // twips conversion factor, and substracting 4, the 4 comes from

While you're here, want to fix substracting -> subtracting?

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/6a1a37e55104