891468 - Convert nsEventStateManager::GetChildProcessOffset to return a LayoutDeviceIntPoint instead of an nsIntPoint

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

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

nsEventStateManager::GetChildProcessOffset returns a generic nsIntPoint that is really in the LayoutDevice coordinate space. The nsIntPoint should be changed to a LayoutDeviceIntPoint and propagated as much as is reasonable.

As part of this, we'll probably want to add a helper function to the LayoutDevicePixel struct in layout/base/Units.h with the signature "static LayoutDeviceIntPoint FromAppUnitsToNearest(const nsPoint& aPoint, int32_t appUnitsPerDevPixel)" and use it when converting from app units to the LayoutDevice coordinate space.

It probably also makes sense to convert the first argument to nsEventStateManager::MapEventCoordinatesForChildProcess in this bug as well, but leave nsEvent::refPoint as an nsIntPoint for now.

Comment 1

[Botond Ballo [:botond]]

Attachment: patch

Comment 2

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

Comment on attachment 772841 [details] [diff] [review]
patch

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

Looks fine to me, requesting smaug to review this since I'm not actually a peer for this code.

::: content/events/src/nsEventStateManager.cpp
@@ +1539,5 @@
>  
>    // Find out how far we're offset from the nearest widget.
>    nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(&aEvent,
>                                                              targetFrame);
> +  return LayoutDevicePixel::FromAppUnitsToNearest(pt, presContext->AppUnitsPerDevPixel());

This works fine but for consistency with other pieces of similar code I've written, I would use "LayoutDeviceIntPoint::FromAppUnitsToNearest" here instead of "LayoutDevicePixel::FromAppUnitsToNearest".

Comment 3

[Botond Ballo [:botond]]

Attachment: updated patch

Comment 4

[Botond Ballo [:botond]]

> ::: content/events/src/nsEventStateManager.cpp
> @@ +1539,5 @@
> >  
> >    // Find out how far we're offset from the nearest widget.
> >    nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(&aEvent,
> >                                                              targetFrame);
> > +  return LayoutDevicePixel::FromAppUnitsToNearest(pt, presContext->AppUnitsPerDevPixel());
> 
> This works fine but for consistency with other pieces of similar code I've
> written, I would use "LayoutDeviceIntPoint::FromAppUnitsToNearest" here
> instead of "LayoutDevicePixel::FromAppUnitsToNearest".

Changed in updated patch,

Comment 5

[Botond Ballo [:botond]]

Attachment: updated patch

Updated patch to fix a compiler error.

Comment 6

[Botond Ballo [:botond]]

Try results: https://tbpl.mozilla.org/?tree=Try&rev=4264d54cc63e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a14404dc5c68

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a14404dc5c68