1793073 - Crash in [@ mozilla::a11y::LocalAccessible::BundleFieldsForCache]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Makoto Kato [:m_kato]]

There is some content process crashes on GeckoView.

Crash report: https://crash-stats.mozilla.org/report/index/950503b7-9a75-4b94-80ac-ecd010220930

Reason: SIGSEGV / SEGV_MAPERR

Top 10 frames of crashing thread:

0 libxul.so mozilla::a11y::LocalAccessible::BundleFieldsForCache accessible/generic/LocalAccessible.cpp:3516
1 libxul.so mozilla::a11y::DocAccessibleChildBase::InsertIntoIpcTree accessible/ipc/DocAccessibleChildBase.cpp:102
2 libxul.so mozilla::a11y::LocalAccessible::HandleAccEvent accessible/generic/LocalAccessible.cpp:868
3 libxul.so mozilla::a11y::AccessibleWrap::HandleAccEvent accessible/android/AccessibleWrap.cpp:120
4 libxul.so nsEventShell::FireEvent accessible/base/nsEventShell.cpp:54
5 libxul.so mozilla::a11y::NotificationController::ProcessMutationEvents accessible/base/NotificationController.cpp:576
5 libxul.so mozilla::a11y::NotificationController::WillRefresh accessible/base/NotificationController.cpp:894
6 libxul.so nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:2510
7 libxul.so mozilla::RefreshDriverTimer::TickDriver layout/base/nsRefreshDriver.cpp:375
7 libxul.so mozilla::RefreshDriverTimer::TickRefreshDrivers layout/base/nsRefreshDriver.cpp:353

Comment 1

[Makoto Kato [:m_kato]]

This is top 1 crash of Fenix 106.0 beta on content process of Fenix. https://crash-stats.mozilla.org/topcrashers/?product=Fenix&version=106.0b&process_type=content

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 10 AArch64 and ARM crashes on nightly
• Top 10 AArch64 and ARM crashes on beta

For more information, please visit auto_nag documentation.

Comment 3

[James Teh [:Jamie]]

We're crashing because IsTable() returns true, but AsTable() returns nullptr. That can happen if an element has role="table" but also display: contents due to bug 1494196.

We should fix bug 1494196 eventually. For now, I think IsTable() should check mGenericTypes directly, rather than calling HasGenericType(), as IsTable() and AsTable() really should match. That said, there's probably no point in calling IsTable() at all; we can just call AsTable().

Comment 4

[James Teh [:Jamie]]

Attachment: Bug 1793073: Ensure Accessible::IsTable can't return true if AsTable will return null.

These should never be out of sync.
Previously, they could be out of sync for an ARIA table with display: contents, since we don't create an ARIAGridAccessible in that case (bug 1494196).
While that incorrect creation is itself a bug that we should fix, I don't want to deal with that here.
Instead, don't check ARIA generic types in Accessible::IsTable, only the class generic types, as we already do for IsTableCell.
This fixes a crash in BundleFieldsForCache.

In addition, while the crash is fixed by the above change, as an optimisation, BundleFieldsForCache now just calls AsTable instead of calling IsTable first, since the IsTable call is redundant.
This second fix alone would be sufficient to fix the crash, but the first will catch other potential instances of this problem.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d420f9190e2f
Ensure Accessible::IsTable can't return true if AsTable will return null. r=eeejay

Comment 6

[Norisz Fay [:noriszfay]]

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

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:Jamie, 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 8

[Pascal Chevrel:pascalc (PTO until April 26)]

James, do you want to uplitft to 106?

Comment 9

[Eitan Isaacson [:eeejay]]

Comment on attachment 9297082 [details]
Bug 1793073: Ensure Accessible::IsTable can't return true if AsTable will return null.

Beta/Release Uplift Approval Request

• User impact if declined: Crash with accessibility enabled
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): This change is essentially a null-check.
• String changes made/needed:
• Is Android affected?: Yes

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9297082 [details]
Bug 1793073: Ensure Accessible::IsTable can't return true if AsTable will return null.

Approved for release, thanks.

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-release/rev/1c6792d74dd9