Closed Bug 1400617 Opened 3 years ago Closed 3 years ago

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

Categories

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

52 Branch
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- fixed

People

(Reporter: tsmith, Assigned: eeejay)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file 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?
Pretty certain we fixed this in bug 1396267. I'll see if a backported patch helps.
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
Depends on: 1396267
Flags: needinfo?(eitan)
(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)
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: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Thanks for making the hard choices for us.
You need to log in before you can comment on or make changes to this bug.