Closed Bug 1272757 Opened 4 years ago Closed 4 years ago

Fix up target confirmation for drag blocks

Categories

(Core :: Panning and Zooming, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(1 file)

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
Attached patch PatchSplinter Review
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.
Attachment #8752319 - Flags: review?(rbarker)
Attachment #8752319 - Flags: review?(rbarker) → review+
https://hg.mozilla.org/mozilla-central/rev/db473770c2eb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
I filed bug 1273137 as the follow-up I mentioned in comment 1.
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
Attachment #8752319 - Flags: approval-mozilla-aurora?
Comment on attachment 8752319 [details] [diff] [review]
Patch

Improve APZ, taking it.
Attachment #8752319 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.