1498352 - nsContentUtils::ToWidgetPoint can return incorrect coordinates in content process

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement, P3)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

`nsContentUtils::ToWidgetPoint` uses the APZ resolution from `nsLayoutUtils::GetCurrentAPZResolutionScale` when converting to widget coordinates. However, `nsLayoutUtils::GetCurrentAPZResolutionScale` doesn't take into account the resolution of the root pres shell.

In a content process on mobile, the content document resolution _is_ the resolution of the root pres shell. Therefore `nsLayoutUtils::GetCurrentAPZResolutionScale` can return the wrong result if the document resolution is different than 1.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1498352 - Include root pres shell resolution in APZ resolution; r?kats

`nsContentUtils::ToWidgetPoint` uses the current APZ resolution to scale
to layout device coordinates. However, that APZ resolution does not take
into account the root pres-shell's resolution. In a content process, the
root pres-shell's resolution represents the content document resolution.
Therefore, `nsContentUtils::ToWidgetPoint` fails to work correctly in a
content process when the content document has a resolution that's
different than one. This patch changes the APZ resolution to include the
root pres shell resolution to resolve the issue.

Comment 2

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

How was this manifesting? i.e. is there a user-observable issue that this fixes? And is it possible to make a test case for it? Off the top of my head I agree with Botond's comment on the review that this is probably intentional. Getting more understanding for the motivation behind this change will help us understand if this is the right fix.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

We're migrating tests on mobile from non-e10s to e10s. Some existing tests (e.g. "dom/animation/test/mozilla/test_restyles.html") fail when running under e10s on mobile because 1) the tests call `nsContentUtils::ToWidgetPoint` indirectly by synthesizing mouse events from the content process, and 2) unlike desktop, the test documents have a different resolution than one on mobile. So I guess these existing, failing tests are the test cases for this issue.

Comment 4

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

I think that on non-e10s the root prescontext at [1] is the chrome prescontext, but in e10s in the content process it's the content prescontext. That might explain the difference in behaviour. But I'll need to spend more time thinking about this before I'm comfortable r+'ing it.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1498352 - Apply root resolution when sending mouse events from child to parent; r?kats

Because the root resolution is conceptually at the parent/child process
boundary, when sending mouse events from child to the parent, we need to
apply that resolution to the mouse coordinates.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/084f6e60778f
Apply root resolution when sending mouse events from child to parent; r=kats

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/084f6e60778f