Assertion failure: aOwner->Elm() (aOwner->Elm() must be a valid pointer) in [@ mozilla::a11y::DocAccessible::DoARIAOwnsRelocation]

RESOLVED WONTFIX

Status

()

Core
Disability Access APIs
P2
normal
RESOLVED WONTFIX
8 months ago
7 months ago

People

(Reporter: tsmith, Assigned: eeejay)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

52 Branch
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox56 wontfix, firefox57 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 months ago
Created attachment 8909054 [details]
test_case.html

Assertion failure: aOwner->Elm() (aOwner->Elm() must be a valid pointer), at /home/worker/workspace/build/src/accessible/generic/DocAccessible.cpp:2068

This only seems to affect ESR52 (20170916-292ae551b2b3)

The test case takes a few tries to trigger the issue.

0|0|libxul.so|mozilla::a11y::DocAccessible::DoARIAOwnsRelocation|hg:hg.mozilla.org/releases/mozilla-esr52:accessible/generic/DocAccessible.cpp:292ae551b2b3|2068|0x0
0|1|libxul.so|mozilla::a11y::NotificationController::WillRefresh|hg:hg.mozilla.org/releases/mozilla-esr52:accessible/base/NotificationController.cpp:292ae551b2b3|802|0xc
0|2|libxul.so|nsRefreshDriver::Tick|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsRefreshDriver.cpp:292ae551b2b3|1798|0x11
0|3|libxul.so|mozilla::RefreshDriverTimer::TickRefreshDrivers|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsRefreshDriver.cpp:292ae551b2b3|295|0xf
0|4|libxul.so|mozilla::RefreshDriverTimer::Tick|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsRefreshDriver.cpp:292ae551b2b3|317|0x12
0|5|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsRefreshDriver.cpp:292ae551b2b3|663|0x5
0|6|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsRefreshDriver.cpp:292ae551b2b3|501|0xb
0|7|libxul.so|mozilla::layout::VsyncChild::RecvNotify|hg:hg.mozilla.org/releases/mozilla-esr52:layout/ipc/VsyncChild.cpp:292ae551b2b3|64|0x9
0|8|libxul.so|mozilla::layout::PVsyncChild::OnMessageReceived|hg:hg.mozilla.org/releases/mozilla-esr52:obj-firefox/ipc/ipdl/PVsyncChild.cpp:292ae551b2b3|169|0xf
0|9|libxul.so|mozilla::ipc::MessageChannel::DispatchAsyncMessage|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessageChannel.cpp:292ae551b2b3|1743|0x6
0|10|libxul.so|mozilla::ipc::MessageChannel::DispatchMessage|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessageChannel.cpp:292ae551b2b3|1681|0xb
0|11|libxul.so|mozilla::ipc::MessageChannel::RunMessage|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessageChannel.cpp:292ae551b2b3|1572|0xb
0|12|libxul.so|mozilla::ipc::MessageChannel::MessageTask::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessageChannel.cpp:292ae551b2b3|1597|0xc
0|13|libxul.so|nsThread::ProcessNextEvent|hg:hg.mozilla.org/releases/mozilla-esr52:xpcom/threads/nsThread.cpp:292ae551b2b3|1216|0x11
0|14|libxul.so|NS_ProcessNextEvent|hg:hg.mozilla.org/releases/mozilla-esr52:xpcom/glue/nsThreadUtils.cpp:292ae551b2b3|361|0xd
0|15|libxul.so|mozilla::ipc::MessagePump::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessagePump.cpp:292ae551b2b3|96|0xa
0|16|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:292ae551b2b3|232|0x17
0|17|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:292ae551b2b3|225|0x8
0|18|libxul.so|nsBaseAppShell::Run|hg:hg.mozilla.org/releases/mozilla-esr52:widget/nsBaseAppShell.cpp:292ae551b2b3|156|0xd
0|19|libxul.so|XRE_RunAppShell|hg:hg.mozilla.org/releases/mozilla-esr52:toolkit/xre/nsEmbedFunctions.cpp:292ae551b2b3|866|0x6
0|20|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessagePump.cpp:292ae551b2b3|269|0x5
0|21|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:292ae551b2b3|232|0x17
0|22|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:292ae551b2b3|225|0x8
0|23|libxul.so|XRE_InitChildProcess|hg:hg.mozilla.org/releases/mozilla-esr52:toolkit/xre/nsEmbedFunctions.cpp:292ae551b2b3|698|0xf
0|24|plugin-container|content_process_main|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/contentproc/plugin-container.cpp:292ae551b2b3|197|0xe
0|25|libc-2.23.so||||0x20830
0|26|plugin-container|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/releases/mozilla-esr52:mfbt/Assertions.h:292ae551b2b3|170|0x5
Flags: in-testsuite?
(Reporter)

Updated

8 months ago
status-firefox57: --- → unaffected
status-firefox-esr52: --- → affected
(Assignee)

Comment 1

8 months ago
Pretty certain we fixed this in bug 1396267. I'll see if a backported patch helps.
(Assignee)

Comment 2

8 months ago
Created attachment 8909371 [details] [diff] [review]
Check that owner has content before aria-owns relocation. r?surkov
Attachment #8909371 - Flags: review?(surkov.alexander)
Priority: -- → P2

Comment 3

8 months ago
Comment on attachment 8909371 [details] [diff] [review]
Check that owner has content before aria-owns relocation. r?surkov

Review of attachment 8909371 [details] [diff] [review]:
-----------------------------------------------------------------

it makes sense for sure, however shouldn't we request bug 1396267 backported instead?
Attachment #8909371 - Flags: review?(surkov.alexander) → review+
Is there a user impact that even justifies fixing this on ESR52? Beyond making the fuzzers happy, that is.
Assignee: nobody → eitan
Has Regression Range: --- → yes
status-firefox56: --- → wontfix
status-firefox57: unaffected → fixed
Depends on: 1396267
Flags: needinfo?(eitan)

Updated

8 months ago
Blocks: 1406117
(Assignee)

Comment 5

8 months ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
> Is there a user impact that even justifies fixing this on ESR52? Beyond
> making the fuzzers happy, that is.

You can crash Firefox from content when a11y is enabled? If that is not a strong enough case for ESR, I'm fine with that too. I'll ask the gods for approval, we'll see what happens.

As Alex suggests, this is a dup of bug 1396267, but let's keep it a blocker since we already have conversation here.
Flags: needinfo?(eitan)
(Assignee)

Comment 6

8 months ago
Comment on attachment 8909371 [details] [diff] [review]
Check that owner has content before aria-owns relocation. r?surkov

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: There is a potential crash from a content.
User impact if declined: A potential crash when a11y is enabled.
Fix Landed on Version: 57
Risk to taking this patch (and alternatives if risky): This should be very low risk.
String or UUID changes made by this patch: None.



This patch is very low risk. OTOH, I'm not sure if the user benefit is all that worth it, even if this is a crasher. Honestly on the fence if this is needed.
Attachment #8909371 - Flags: approval-mozilla-esr52?
Comment on attachment 8909371 [details] [diff] [review]
Check that owner has content before aria-owns relocation. r?surkov

I get that this would be more correct and might avoid a crash. But I am only a frail mortal and so would like more evidence that the crashes actually happen on ESR before uplifting. Happy to reconsider if anyone feels strongly about it.
Attachment #8909371 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52-
status-firefox-esr52: affected → wontfix
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → WONTFIX
(Assignee)

Comment 8

7 months ago
Thanks for making the hard choices for us.
You need to log in before you can comment on or make changes to this bug.