1792708 - Crash in [@ mozilla::Maybe<T>::operator=] from StickyPositionInfo

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/c338bbae-5926-412d-b853-5e1140220928

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames of crashing thread:

0 XUL mozilla::Maybe<unsigned long long>::operator= mfbt/Maybe.h:349
0 XUL mozilla::layers::APZCTreeManager::StickyPositionInfo::StickyPositionInfo gfx/layers/apz/src/APZCTreeManager.cpp:3710
1 XUL mozilla::layers::APZCTreeManager::StickyPositionInfo::StickyPositionInfo gfx/layers/apz/src/APZCTreeManager.cpp:3709
1 XUL mozilla::layers::APZCTreeManager::SidesStuckToRootContent const gfx/layers/apz/src/APZCTreeManager.cpp:3349
2 XUL mozilla::layers::APZCTreeManager::AdjustEventPointForDynamicToolbar gfx/layers/apz/src/APZCTreeManager.cpp:2078
3 XUL mozilla::layers::APZCTreeManager::ConvertToGecko gfx/layers/apz/src/APZCTreeManager.cpp:3272
4 XUL mozilla::layers::AsyncPanZoomController::ConvertToGecko gfx/layers/apz/src/AsyncPanZoomController.cpp:1873
4 XUL mozilla::layers::AsyncPanZoomController::OnDoubleTap gfx/layers/apz/src/AsyncPanZoomController.cpp:3064
5 XUL mozilla::layers::AsyncPanZoomController::HandleGestureEvent gfx/layers/apz/src/AsyncPanZoomController.cpp:1247
6 XUL mozilla::layers::AsyncPanZoomController::HandleInputEvent gfx/layers/apz/src/AsyncPanZoomController.cpp

The signature isn't great, so maybe this is a dupe of some known issue, but I didn't find other crashes with StickyPositionInfo in the signature, so maybe it is a regression? 7 crashes from 7 installations, OSX only.

Comment 1

[Andrew McCreight [:mccr8]]

This is the set of patches added to the build where it looks like it first showed up. Nothing too obviously APZ related.

Comment 2

[Botond Ballo [:botond]]

Sounds like maybe mTouchBlockHitResult has been cleared by the time we get to OnDoubleTap, causing us to use an invalid or null node here?

Comment 3

[Botond Ballo [:botond]]

That would make it a regression from bug 1779144, which is earlier than the regression range posted in comment 1, but maybe it's just a rare crash and users of previous builds were lucky and haven't run into it.

Comment 4

[Botond Ballo [:botond]]

Oh, never mind, the caller of HandleGestureEvent() is this one, so we're processing tap gesture input, not touch input block, so of course we shouldn't be accessing mTouchBlockHitResult.

This is a bug introduced during the refactor in https://phabricator.services.mozilla.com/D151940, we should be accessing the parameter aHit (which could refer to mTapGestureHitResult) instead of mTouchBlockHitResult.

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1792708 - Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson

Comment 6

[Release mgmt bot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1779144

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fef48bed539e
Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson

Comment 8

[Atila Butkovits]

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

Comment 9

[Release mgmt bot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:botond, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox106 to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Botond Ballo [:botond]]

Comment on attachment 9296535 [details]
Bug 1792708 - Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson

Beta/Release Uplift Approval Request

• User impact if declined: Some users may experience a crash while performing a double-tap on the touchpad over sticky-positioned content
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix is simple and obvious, a previous refactoring missed one use of a variable
• String changes made/needed:
• Is Android affected?: No

Comment 11

[Pascal Chevrel:pascalc]

Comment on attachment 9296535 [details]
Bug 1792708 - Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson

Approved for 106.0b7, thanks.

Comment 12

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/a8eb93456828