Closed Bug 1273137 Opened 3 years ago Closed 3 years ago

Fix up target confirmation for drag blocks (part 2)

Categories

(Core :: Panning and Zooming, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- wontfix
firefox50 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files)

+++ This bug was initially created as a clone of Bug #1272757 +++

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.

I fixed part of this issue in bug 1272757, for the cases where the drag block lands on a confirmed hit area. For the dispatch-to-content hit areas, we still need to make the base widget send a APZ target confirmation, somewhere in [4]. This bug is tracking that issue.

[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
[4] http://mxr.mozilla.org/mozilla-central/source/widget/nsBaseWidget.cpp?rev=114ca1fc9c51#1121
Although the tests are all passing on this one, I see that test_group_touchevents is taking ~40s longer on android and that's a sign something is getting stuck. All other platforms seem ok.
This patch also cleans up some inconsistencies in the conditions under which the
main thread would respond to wheel and mouse events. With this patch applied, the
main-thread notifications are only sent if the input block id is nonzero, which
indicates the APZ actually processed the input event and added it to an input
block.

Review commit: https://reviewboard.mozilla.org/r/58250/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58250/
Comment on attachment 8760790 [details]
Bug 1273137 - Add a mochitest that creates a drag block on a dispatch-to-content-region.

https://reviewboard.mozilla.org/r/58246/#review55088
Attachment #8760790 - Flags: review?(rbarker) → review+
Comment on attachment 8760791 [details]
Bug 1273137 - Add the mHandledByAPZ flag to MouseInput as well.

https://reviewboard.mozilla.org/r/58248/#review55090
Attachment #8760791 - Flags: review?(rbarker) → review+
Comment on attachment 8760792 [details]
Bug 1273137 - Add the missing main-thread target confirmation for drag blocks.

https://reviewboard.mozilla.org/r/58250/#review55092
Attachment #8760792 - Flags: review?(rbarker) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cd13955b729
Add a mochitest that creates a drag block on a dispatch-to-content-region. r=rbarker
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bc047b037a0
Add the mHandledByAPZ flag to MouseInput as well. r=rbarker
https://hg.mozilla.org/integration/mozilla-inbound/rev/b72ab182dd55
Add the missing main-thread target confirmation for drag blocks. r=rbarker
Depends on: 1346632
Depends on: 1378247
You need to log in before you can comment on or make changes to this bug.