nsContentUtils::ToWidgetPoint can return incorrect coordinates in content process

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P3
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

unspecified
mozilla64
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 1 obsolete attachment)

`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.
`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.
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.
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.
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.
Priority: -- → P3
Whiteboard: [gfx-noted]
Attachment #9016452 - Attachment is obsolete: true
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.
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/084f6e60778f
Apply root resolution when sending mouse events from child to parent; r=kats
https://hg.mozilla.org/mozilla-central/rev/084f6e60778f
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.