Closed Bug 1597043 Opened 3 months ago Closed 3 months ago

Crash in [@ mozilla::a11y::RootAccessible::ProcessDOMEvent]

Categories

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

68 Branch
x86
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
thunderbird_esr68 ? affected
firefox-esr68 --- fixed
firefox70 --- wontfix
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: wsmwk, Assigned: Jamie)

Details

(Keywords: crash, topcrash-thunderbird)

Crash Data

Attachments

(1 file)

#122 for Firefox 70.0.1. #27 crash for Thunderbird 68.2.2. The crash rate is trivial before Thunderbird version 68, which makes one wonder whether Bug 1502061 - Crash in mozilla::a11y::RootAccessible::ProcessDOMEvent, fixed for v65, made this better or worse for Thunderbird. Perhaps we have a regression? (the author of that patch isn't around)

bp-b5285f48-9aaa-46cb-99d7-a02670191116 Thunderbird
0 xul.dll mozilla::a11y::RootAccessible::ProcessDOMEvent accessible/generic/RootAccessible.cpp:351
1 xul.dll mozilla::a11y::TNotification<mozilla::a11y::RootAccessible, mozilla::dom::Event, nsINode>::Process accessible/base/NotificationController.h:72
2 xul.dll mozilla::a11y::NotificationController::WillRefresh accessible/base/NotificationController.cpp:837
3 xul.dll nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:1892
4 xul.dll mozilla::RefreshDriverTimer::TickRefreshDrivers layout/base/nsRefreshDriver.cpp:325
5 xul.dll mozilla::RefreshDriverTimer::Tick layout/base/nsRefreshDriver.cpp:342
6 xul.dll mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver layout/base/nsRefreshDriver.cpp:708
7 xul.dll mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run layout/base/nsRefreshDriver.cpp:508
8 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1175
9 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486

bp-81ed8710-566c-4128-ad1c-8a0fe0191116 Firefox 70.0.1
0 xul.dll mozilla::a11y::RootAccessible::ProcessDOMEvent(mozilla::dom::Event*, nsINode*) accessible/generic/RootAccessible.cpp:351
1 xul.dll mozilla::a11y::TNotification<mozilla::a11y::RootAccessible, mozilla::dom::Event, nsINode>::Process() accessible/base/NotificationController.h:72
2 xul.dll mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) accessible/base/NotificationController.cpp:837
3 xul.dll nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:1892
4 xul.dll mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) layout/base/nsRefreshDriver.cpp:325
5 xul.dll mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:342
6 xul.dll mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:708

Flags: needinfo?(jteh)
Flags: needinfo?(jteh)
Priority: -- → P1

(In reply to Wayne Mery (:wsmwk) from comment #0)

makes one wonder whether Bug 1502061 - Crash in mozilla::a11y::RootAccessible::ProcessDOMEvent, fixed for v65, made this better or worse for Thunderbird. Perhaps we have a regression?

Looking at that patch, I don't see how this could be a regression from it. That patch just introduces a null check and early return to prevent a crash earlier in the function. I agree it's an odd coincidence, though.

The crash dumps are useless; everything I need is optimised out. From what I can tell from the stack, it looks like calling AsXULMultiSelectControl() on the tree element returns null. I don't understand why that would happen. All XUL trees should implement nsIDOMXULMultiSelectControlElement. The tree has DOM focus (since this code only runs if that is the case), so the element shouldn't be dying.

The best I can do is wallpaper over this in release builds and throw in an assert so we might hopefully one day catch this in a debug build.

Assignee: nobody → jteh

This really shouldn't be possible.
All XUL trees should have nsIDOMXULMultiSelectControlElement, and the tree is focused at this point, so it shouldn't be dying.
Nevertheless, this sometimes happens in the wild and was causing crashes.

Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cbe5a080a2ad
RootAccessible::ProcessDOMEvent: Return early if AsXULMultiSelectControl() on a XUL tree element fails. r=MarcoZ
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Please nominate this for Beta and ESR68 approval when you get a chance.

Flags: needinfo?(jteh)

Comment on attachment 9109331 [details]
Bug 1597043: RootAccessible::ProcessDOMEvent: Return early if AsXULMultiSelectControl() on a XUL tree element fails.

Beta/Release Uplift Approval Request

  • User impact if declined: Rare crashes. I don't know how to reproduce, so I can't provide a test, nor can I verify the fix. However, the crash stack very strongly suggests that this patch should fix the crash.
  • 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): Straightforward null check.
  • String changes made/needed: None.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Crashes.
  • User impact if declined: Rare crashes. I don't know how to reproduce, so I can't provide a test, nor can I verify the fix. However, the crash stack very strongly suggests that this patch should fix the crash.
  • Fix Landed on Version: 72, also uplifting to 71
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Straightforward null check.
  • String or UUID changes made by this patch: None.
Flags: needinfo?(jteh)
Attachment #9109331 - Flags: approval-mozilla-esr68?
Attachment #9109331 - Flags: approval-mozilla-beta?

Comment on attachment 9109331 [details]
Bug 1597043: RootAccessible::ProcessDOMEvent: Return early if AsXULMultiSelectControl() on a XUL tree element fails.

Low risk fix for rare crashes, uplift approved for both beta and esr, thanks.

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