1590929 - Crash in [@ mozilla::a11y::TraversalRule::Match]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Chris Peterson [:cpeterson]]

I see these crash reports with this a11y crash signature from GV 71 Nightly in Focus, but not from GV 69 or 70. However, we didn't ship GV 70 to Focus users, so this crash might be a regression in GV 70 or 71.

Could this be a regression from native a11y bug 1564549 in GV 71?

I don't see any reports from Fenix Nightly (GV 71 Nightly) users, so perhaps there is some app code difference in Focus or Fenix doesn't have enough users to hit some rare corner case.

bp-4af22f95-9482-4283-bf26-7e0930191023

Frame   Module  Signature   Source
0   libxul.so   mozilla::a11y::TraversalRule::Match(mozilla::a11y::Accessible*)   accessible/android/TraversalRule.cpp:42
1   libxul.so   mozilla::a11y::Pivot::AdjustStartPosition(mozilla::a11y::Accessible*, mozilla::a11y::PivotRule&, unsigned short*)   accessible/base/Pivot.cpp:35
2   libxul.so   mozilla::a11y::Pivot::SearchForward(mozilla::a11y::Accessible*, mozilla::a11y::PivotRule&, bool)  accessible/base/Pivot.cpp:105
3   libxul.so   mozilla::a11y::AccessibleWrap::Pivot(int, bool, bool)   accessible/android/AccessibleWrap.cpp:277
4   libxul.so   mozilla::a11y::AccessibleWrap::HandleAccEvent(mozilla::a11y::AccEvent*)   accessible/android/AccessibleWrap.cpp:0
5   libxul.so   nsEventShell::FireEvent(mozilla::a11y::AccEvent*)   accessible/base/nsEventShell.cpp:43
...

Comment 1

[James Teh [:Jamie]]

Yes, it's a regression from bug 1564549.

Comment 2

[Marcia Knous [:marcia]]

Discussed during Platform triage. tried to add a ni on :eejay but was unable to - adding back Jamie. https://crash-stats.mozilla.org/signature/?signature=mozilla%3A%3Aa11y%3A%3ATraversalRule%3A%3AMatch shows crashes for Fenix (they are under Fennec Android).

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1590929: a11y::TraversalRule::Match: Don't assume that all Accessibles have a frame.

I don't have a test case for this crash, but the stack suggests the frame is null.
This can certainly happen for display: contents.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11f08e7b4bcf
a11y::TraversalRule::Match: Don't assume that all Accessibles have a frame. r=MarcoZ

Comment 5

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/11f08e7b4bcf

Comment 6

[Chris Peterson [:cpeterson]]

Jamie, once your fix lands in Fenix Nightly and looks good, we should uplift it to GeckoView Beta (71) because this crash was a regression in 71.

Comment 7

[James Teh [:Jamie]]

Chris, I assume this has landed in Fenix nightly by now? Are you able to tell whether this appears to have fixed the crash? Crash-stop doesn't seem to work here for some reason and the crash graphs are impossible to read with a screen reader. Thanks.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to James Teh [:Jamie] from comment #7)

Chris, I assume this has landed in Fenix nightly by now? Are you able to tell whether this appears to have fixed the crash? Crash-stop doesn't seem to work here for some reason and the crash graphs are impossible to read with a screen reader.

Your fix should be in Fenix Nightly now, but it's hard to tell if it fixed the TraversalRule crash. We only have a couple hundred Fenix Nightly users. Before your fix landed on November 4, we only saw about 1-4 TraversalRule crash reports per day. I don't see any TraversalRule crash reports in any Nightly build after November 4 (build ID 20191104094118).

So I think we can assume your fix landed in the November 5 build and fixed the crashes. I think we should your fix to Beta, if you are comfortable with that.

https://crash-stats.mozilla.com/search/?signature=~TraversalRule&product=Fenix&date=%3E%3D2019-10-28T18%3A28%3A00.000Z&date=%3C2019-11-11T18%3A28%3A00.000Z&_facets=signature&_facets=cpu_arch&_facets=version&_facets=platform_pretty_version&_facets=product&_facets=build_id&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-build_id

Comment 9

[James Teh [:Jamie]]

Comment on attachment 9106101 [details]
Bug 1590929: a11y::TraversalRule::Match: Don't assume that all Accessibles have a frame.

Beta/Release Uplift Approval Request

• User impact if declined: Crashes.
• Is this code covered by automated tests?: No
• 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): Straightforward null check.
• String changes made/needed: None.

Comment 10

[Pascal Chevrel:pascalc]

Comment on attachment 9106101 [details]
Bug 1590929: a11y::TraversalRule::Match: Don't assume that all Accessibles have a frame.

Geckoview crash fix, uplift approved for 71 beta 10, thanks.

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4bc5fce6cfca