Closed Bug 1792708 Opened 2 months ago Closed 2 months ago

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

Categories

(Core :: Panning and Zooming, defect)

defect

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox105 --- wontfix
firefox106 --- fixed
firefox107 --- fixed

People

(Reporter: mccr8, Assigned: botond)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.

See Also: → 1792710

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

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

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.

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.

Assignee: nobody → botond
Regressed by: 1779144

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

Attachment #9296535 - Attachment description: Bug 1792708 - Access aHit instead of mTouchBlockHitResult consistently iin AdjustEventPointForDynamicToolbar(). r=dlrobertson → Bug 1792708 - Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fef48bed539e
Access aHit instead of mTouchBlockHitResult consistently in AdjustEventPointForDynamicToolbar(). r=dlrobertson
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

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.

Flags: needinfo?(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
Flags: needinfo?(botond)
Attachment #9296535 - Flags: approval-mozilla-beta?

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

Approved for 106.0b7, thanks.

Attachment #9296535 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.