1272757 - Fix up target confirmation for drag blocks

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

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

Back when the APZ drag code was first added in bug 1199885, drag blocks were only created if the apz.drag.enabled pref was enabled, and they were always created as unconfirmed [1][2]. The idea was that if they hit a scrollbar, they would be confirmed by the main-thread sending over drag metrics [3], and thus would get processed.

However, in bug 1242690 I made it so that drag blocks get created on mousedown, even if apz.drag.enabled is not set. However, I overlooked this business with every drag block being unconfirmed by default. This means that if apz.drag.enabled is false, every drag block ends up waiting for the full content timeout before it proceeds. In practice this is not really noticeable, but in tests we increase the content timeout to 60s and this behaviour can cause the input queue to get backed up and tests to timeout. The tests I'm adding in bug 1264017 suffer from this problem.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=09c5d774bd83#701
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp?rev=09c5d774bd83#185
[3] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp?rev=09c5d774bd83#643

Comment 1

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

Attachment: Patch

This fixes enough of the problem to take care of the test timeout issue. However I think there's still a piece missing where we need to send the target APZC confirmation from nsBaseWidget. I'll do that in a followup bug because I want to write a test to go with that. This patch is covered by the tests I'm adding in bug 1264017.

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8eb85af0fea3

Comment 3

[Pulsebot]

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

Comment 4

[Phil Ringnalda (:philor)]

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

Comment 5

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

I filed bug 1273137 as the follow-up I mentioned in comment 1.

Comment 6

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

Comment on attachment 8752319 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: APZ (specifically bug 1242690)
[User impact if declined]: mouse-dragging inputs can end up delayed slightly, which can cause strange behaviour. Bug 1269066 in particular seems to have been fixed by this bug.
[Describe test coverage new/current, TreeHerder]: there are tests that cover this code, and more tests added in bug 1264017 that specifically isolate the delay
[Risks and why]: low risk
[String/UUID change made/needed]: none

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8752319 [details] [diff] [review]
Patch

Improve APZ, taking it.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0389431b7aad