1902571 - Cleanups/refactoring in APZHandledResult computation codepaths

Status: RESOLVED FIXED

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

Description

[Botond Ballo [:botond]]

While reviewing bug 1895742, I noticed some opportunities for cleanups / refactoring in the APZHandledResult computation codepaths.

In particular, I realized that we can share more code between the "eager" and "delayed" computation codepaths, and in the process address this FIXME (which the patch in bug 1895742 partially does already).

Comment 1

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Make the aOutTouchBehaviors parameter of GetTouchInputBlockAPZC() required. r=hiro

The function only has one call site, and it passes an argument for this parameter.

Comment 2

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Const-correctness improvements. r=hiro

Depends on D213713

Comment 3

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Move the definition of the APZHandledResult constructor to the top of APZInputBridge.cpp. r=hiro

This just makes the order of the definitions in the source file
consistent with the order of the declarations in the header file.

Depends on D213714

Comment 4

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Factor out two static helper methods of APZHandledResult: Initialize and UpdateForTouchEvent. r=hiro

The second one takes the place of APZEventResult::UpdateHandledResult.

Depends on D213715

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Reuse APZHandledResult helpers during the delayed result codepath. r=hiro

Depends on D213716

Comment 6

[Botond Ballo [:botond]]

Attachment: Bug 1902571 - Stop retaining the eager nsEventStatus for the delayed result computation. r=hiro

The delayed result computation now calls ArePointerEventsConsumable()
again to compute a more accurate result, so it doesn't need the
eager result.

Depends on D213717

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71bda9b87cd5
Make the aOutTouchBehaviors parameter of GetTouchInputBlockAPZC() required. r=hiro
https://hg.mozilla.org/integration/autoland/rev/e80ef04c421a
Const-correctness improvements. r=hiro
https://hg.mozilla.org/integration/autoland/rev/eda7b7d4d3a1
Move the definition of the APZHandledResult constructor to the top of APZInputBridge.cpp. r=hiro
https://hg.mozilla.org/integration/autoland/rev/f5acf5b3e363
Factor out two static helper methods of APZHandledResult: Initialize and UpdateForTouchEvent. r=hiro
https://hg.mozilla.org/integration/autoland/rev/d7c28fe9f43e
Reuse APZHandledResult helpers during the delayed result codepath. r=hiro
https://hg.mozilla.org/integration/autoland/rev/07ae591a549b
Stop retaining the eager nsEventStatus for the delayed result computation. r=hiro

Comment 8

[Cristian Tuns]

Backed out for causing build bustages in TestEventResult.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/gfx/layers/apz/test/gtest/TestEventResult.cpp:68:9: error: no matching constructor for initialization of 'InputBlockCallback &&' (aka 'function<void (unsigned long, const mozilla::layers::APZHandledResult &)> &&')

Comment 9

[Botond Ballo [:botond]]

Huh. I wonder how this built locally.

Comment 10

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #9)

Huh. I wonder how this built locally.

Ok, I see now: gtest source files are not built when you run mach build, only when you run mach gtest. (Which is a bit unintuitive because in CI they are built by the build job rather than the gtest job.)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77a3ea498c1d
Make the aOutTouchBehaviors parameter of GetTouchInputBlockAPZC() required. r=hiro
https://hg.mozilla.org/integration/autoland/rev/2215ccc81f14
Const-correctness improvements. r=hiro
https://hg.mozilla.org/integration/autoland/rev/d9abbb164fc2
Move the definition of the APZHandledResult constructor to the top of APZInputBridge.cpp. r=hiro
https://hg.mozilla.org/integration/autoland/rev/d4278aa127c3
Factor out two static helper methods of APZHandledResult: Initialize and UpdateForTouchEvent. r=hiro
https://hg.mozilla.org/integration/autoland/rev/401c09d143d7
Reuse APZHandledResult helpers during the delayed result codepath. r=hiro
https://hg.mozilla.org/integration/autoland/rev/08cba3d1a49d
Stop retaining the eager nsEventStatus for the delayed result computation. r=hiro

Comment 12

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/77a3ea498c1d
https://hg.mozilla.org/mozilla-central/rev/2215ccc81f14
https://hg.mozilla.org/mozilla-central/rev/d9abbb164fc2
https://hg.mozilla.org/mozilla-central/rev/d4278aa127c3
https://hg.mozilla.org/mozilla-central/rev/401c09d143d7
https://hg.mozilla.org/mozilla-central/rev/08cba3d1a49d