1542020 - APZ can stamp input events with the wrong layers id

Status: RESOLVED DUPLICATE of bug 1541589

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

Description

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

This is a defect in the implementation in bug 1528725 due to an oversight on my part. This code added in bug 1528725 assumes that the hit-test result returned from WebRender is always 100% unambiguously correct. In fact, in some cases (e.g. OOP iframes intersecting SVG or clip-path'd elements), the hitInfo field returned from WR might have the eDispatchToContent flag set, in which case the layers id might not actually be correct. In such a case, APZ might stamp the input event with the wrong layers id.

To correct this, we need to update the WR hit-test to not just take the topmost result but iterate through all the results. If all the results have the same pipeline id, then that's good and we can stamp the input events with that. If the results have multiple pipeline ids, and any of the results have an eDispatchToContent flag set, then the result is ambiguous, and we need to do a main-thread fallback (which is covered by bug 1541589).

To minimize the perf impact of the above, we should also make sure that the eDispatchToContent flag is only ever set to indicate ambiguous hit-test areas. Currently it also used to indicate areas with APZ-aware event listeners. I've filed bug 1542019 to track this work.

Comment 1

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

I wrote a testcase to demonstrate this behaviour, and it seems to fail even without WR/Fission. I filed bug 1543482 for it; the testcase is on that bug.

Comment 2

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

The testcase on bug 1543482 has been broken pretty much since the original implementation, but it only affected async-scrolling the right scrollframe. In case of incorrect result, the user probably could work around it.

For this bug we probably have stricter requirements because it affects all events (e.g. mousemove events) and they could get delivered to the wrong content process, which could have harder-to-work-around consequences.

Comment 4

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

This will need bug 1541589 in order to be fixed completely, adding a dependency.

Comment 5

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

The patches that landed in bug 1566599 are a code dependency here, as they move the looping over WR results from Rust to C++ code, which allows doing the iteration I described in comment 0.

Comment 6

[Chris Peterson [:cpeterson]]

btw, we plan to allow Fission on macOS and Linux without WR (and may need to support Fission on Windows without WR), so we will want to test APZ bugs both with and without WR.

Comment 7

[Botond Ballo [:botond]]

I'm going to dupe this over to bug 1541589 because it doesn't serve any additional purpose.