Closed Bug 1270165 Opened 4 years ago Closed 4 years ago

APZC::ResetTouchInputState assumes CurrentTouchBlock can return null, but it can't

Categories

(Core :: Panning and Zooming, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

The code at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=f15a963b5d4e#3763 has a typo. It's an if condition, meaning it allows for the possibility that CurrentTouchBlock() returns null. However, that function is never supposed to be called in such a case; internally it asserts that there the current block exists and is a touch block.

This call site does, however, need to deal with being called without a touch block being active, and so needs to do CurrentBlock()->AsTouchBlock() which will return null if the current block is not a touch block. The code should be safe to assume that there is a CurrentBlock().
Assignee: nobody → bugmail.mozilla
Whiteboard: [gfx-noted]
Version: unspecified → Trunk
Comment on attachment 8748710 [details]
MozReview Request: Bug 1270165 - Allow ResetTouchInputState to be called while a non-touch block is the current block. r?botond

https://reviewboard.mozilla.org/r/50489/#review47255
Attachment #8748710 - Flags: review?(botond) → review+
https://hg.mozilla.org/mozilla-central/rev/4c87fb10e5f0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8748710 [details]
MozReview Request: Bug 1270165 - Allow ResetTouchInputState to be called while a non-touch block is the current block. r?botond

Approval Request Comment
[Feature/regressing bug #]: bug 1240202
[User impact if declined]: none; this code triggers a debug-only assertion in some cases but in production there is no bad effect. would like to uplift this as a prerequisite for uplifting bug 1231570.
[Describe test coverage new/current, TreeHerder]: covered by the tests in bug 1231570
[Risks and why]: zero risk, it's just a small typo
[String/UUID change made/needed]: none
Attachment #8748710 - Flags: approval-mozilla-aurora?
Comment on attachment 8748710 [details]
MozReview Request: Bug 1270165 - Allow ResetTouchInputState to be called while a non-touch block is the current block. r?botond

APZ fix, has test coverage, let's bring it to aurora.
Attachment #8748710 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.