Closed Bug 1415667 Opened 2 years ago Closed 5 months ago

Assertion failure: target->IsApplication() || target->IsOuterDoc() || target->IsXULTree() (Only app or outerdoc accessible reorder events are in the queue)

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: tsmith, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, intermittent-failure, testcase, Whiteboard: [fuzzblocker])

Attachments

(2 files)

Attached file testcase.html
Assertion failure: target->IsApplication() || target->IsOuterDoc() || target->IsXULTree() (Only app or outerdoc accessible reorder events are in the queue), at /src/accessible/base/EventQueue.cpp:98

#0 mozilla::a11y::EventQueue::CoalesceEvents() /src/accessible/base/EventQueue.cpp:95:7
#1 mozilla::a11y::EventQueue::PushEvent(mozilla::a11y::AccEvent*) /src/accessible/base/EventQueue.cpp:40:3
#2 mozilla::a11y::NotificationController::QueueEvent(mozilla::a11y::AccEvent*) /src/accessible/base/NotificationController.h:112:9
#3 mozilla::a11y::DocAccessible::DoInitialUpdate() /src/accessible/generic/DocAccessible.cpp:1534:23
#4 mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) /src/accessible/base/NotificationController.cpp:633:16
#5 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1843:12
#6 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /src/layout/base/nsRefreshDriver.cpp:306:7
#7 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:328:5
#8 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:769:5
#9 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:682:35
#10 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:583:9
#11 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /src/layout/ipc/VsyncChild.cpp:68:16
#12 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20
#13 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1815:28
#14 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2119:25
#15 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2049:17
#16 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1895:5
#17 mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1928:15
#18 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1037:14
#19 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:513:10
#20 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#21 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#22 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#23 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#24 XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:877:22
#25 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:269:9
#26 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#27 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#28 XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:703:34
#29 content_process_main(mozilla::Bootstrap*, int, char**) /src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
#30 main /src/browser/app/nsBrowserApp.cpp:280:18
#31 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#32 _start (firefox+0x41ebe4)
Flags: in-testsuite?
This testcase asserts back further than a year, though the specific assertion has changed since:
Assertion failure: !aRoot->IsDoc() (documents shouldn't be serialized), at /home/worker/workspace/build/src/accessible/ipc/DocAccessibleChildBase.cpp:67
Has Regression Range: --- → no
Version: 58 Branch → unspecified
Yura, do you know anything about content process serialization (DocAccessibleChildBase::SerializeTree). I'm trying to figure out how bad the things the assertions indicates at.
Flags: needinfo?(yzenevich)
I am not familiar with that code, no, sorry.
Flags: needinfo?(yzenevich)
let's set P2 for now, until we investigate the issue further, to keep it on radar. cc'ing Aaron
Priority: -- → P2
+Jamie
Duplicate of this bug: 1576945

This issue is hit frequently while fuzzing (with debug builds) and this can limit the effectiveness of the fuzzers.

A Pernosco session can be found here: https://pernos.co/debug/cP1CFdCKfijlgCPV1f1rIA/index.html

Whiteboard: [fuzzblocker]
Priority: P2 → P1

(In reply to Tyson Smith [:tsmith] from comment #12)

This issue is hit frequently while fuzzing (with debug builds) and this can limit the effectiveness of the fuzzers.

Which assertion is causing fuzzing trouble here? Comment 1 suggests the assertion is now this:

Assertion failure: !aRoot->IsDoc() (documents shouldn't be serialized), at /home/worker/workspace/build/src/accessible/ipc/DocAccessibleChildBase.cpp:67

But the summary still says this:

Assertion failure: target->IsApplication() || target->IsOuterDoc() || target->IsXULTree() (Only app or outerdoc accessible reorder events are in the queue)

Flags: needinfo?(twsmith)

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

Which assertion is causing fuzzing trouble here?

The assertion mentioned in the summary is causing the problem. The attached test case triggers that issue. Comment 1 seems to be referring to builds that at this point are over 3 years old, so that can likely be ignored for now.

Flags: needinfo?(twsmith)

Okay. Morgan, want to take a look at this one too?

This is really odd. That assertion relates to reorder events with a rule of eCoalesceReorder, which get created by AccReorderEvent. That probably happens here when we fire a reorder event on an OuterDocAccessible (iframe or browser) after its child document is created:
https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/accessible/generic/DocAccessible.cpp#1595
The iframe has been given role="row", so that does affect the accessible role. However, the assertion is checking IsOuterDoc(), which checks the type of the accessible (set in the OuterDocAccessible constructor), not the role. That suggests that this isn't in fact an OuterDocAccessible, but I don't understand why. role="row" creates an ARIARowAccessible instead of falling back to other creation (like OuterDocAccessible), but only if the parent is a table:
https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/accessible/base/nsAccessibilityService.cpp#1047
I don't see how that code could be running... but if it isn't, why aren't we getting an OuterDocAccessible here?

Assignee: nobody → mreschenberg

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

I don't think this is a regression, at least not recent enough to be worth tracking down the source.

Keywords: regression

(In reply to Tyson Smith [:tsmith] from comment #12)

This issue is hit frequently while fuzzing (with debug builds) and this can limit the effectiveness of the fuzzers.

A Pernosco session can be found here: https://pernos.co/debug/cP1CFdCKfijlgCPV1f1rIA/index.html

hello! can I get authorisation to view this? github: mreschenberg

Flags: needinfo?(twsmith)

Note: The Pernosco session will be active for the next 7 days.

Flags: needinfo?(twsmith)
Assignee: mreschenberg → jteh

I have a patch for this. However, I wanted to make sure that the test case actually fails without the fix. Curiously, my try run (with the test but without the fix) succeeded:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b66914f0ca87efc046ad41d8db625c08a78ef87
Yura, do you have any idea what I'm doing wrong here? This crashtest really should fail... unless they don't catch assertions? But I thought they were supposed to...

Flags: needinfo?(yzenevich)

OuterDocAccessible has some special behaviour.
We really shouldn't try to use some other class for iframes.
Anyway, making an iframe part of an ARIA table won't work for other reasons, plus I'm not sure it makes much sense.

I have no idea why this crashtest succeeds on try. It definitely crashes locally, though strangely, I don't see the assertion I expect to see. With the fix, no crash.

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

I have a patch for this. However, I wanted to make sure that the test case actually fails without the fix. Curiously, my try run (with the test but without the fix) succeeded:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b66914f0ca87efc046ad41d8db625c08a78ef87
Yura, do you have any idea what I'm doing wrong here? This crashtest really should fail... unless they don't catch assertions? But I thought they were supposed to...

Looks like the assertion is only a warning in the crash test log from the try run without the fix:
REFTEST TEST-START | file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests/1415667.html
[task 2019-10-25T06:42:48.628Z] 06:42:48 INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests/1415667.html | 14 / 16 (87%)
[task 2019-10-25T06:42:48.631Z] 06:42:48 INFO - ++DOMWINDOW == 28 (0x7fd8a612bc00) [pid = 1198] [serial = 34] [outer = 0x7fd8a8b10f20]
[task 2019-10-25T06:42:48.639Z] 06:42:48 INFO - [Child 1198, Main Thread] WARNING: NS_ENSURE_TRUE(frame) failed: file /builds/worker/workspace/build/src/layout/base/nsPresContext.cpp, line 821
[task 2019-10-25T06:42:48.642Z] 06:42:48 INFO - ++DOCSHELL 0x7fd8a906a000 == 2 [pid = 1198] [id = {5f808bdc-03e8-4807-b70f-4a89a80c3768}]
[task 2019-10-25T06:42:48.643Z] 06:42:48 INFO - ++DOMWINDOW == 29 (0x7fd8a9153d40) [pid = 1198] [serial = 35] [outer = (nil)]
[task 2019-10-25T06:42:48.646Z] 06:42:48 INFO - ++DOMWINDOW == 30 (0x7fd8a8c3f800) [pid = 1198] [serial = 36] [outer = 0x7fd8a9153d40]
[task 2019-10-25T06:42:48.686Z] 06:42:48 INFO - REFTEST TEST-PASS | file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests/1415667.html | (LOAD ONLY)
[task 2019-10-25T06:42:48.686Z] 06:42:48 INFO - REFTEST TEST-END | file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests/1415667.html

Flags: needinfo?(yzenevich)
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8d297ad5ac5
Always use OuterDocAccessible for iframes, even if an ARIA table role is specified. r=yzen
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

(In reply to Yura Zenevich [:yzen] from comment #29)

Looks like the assertion is only a warning in the crash test log from the try run without the fix:
...
[task 2019-10-25T06:42:48.639Z] 06:42:48 INFO - [Child 1198, Main Thread] WARNING: NS_ENSURE_TRUE(frame) failed: file /builds/worker/workspace/build/src/layout/base/nsPresContext.cpp, line 821

That's a different warning. The assertion in question is: target->IsApplication() || target->IsOuterDoc() || target->IsXULTree() (Only app or outerdoc accessible reorder events are in the queue).

Anyway, I was definitely able to confirm this fix locally, so I think that'll have to do.

Is there a user-facing issue fixed by this which would make us want to consider uplifting to Beta?

Flags: needinfo?(jteh)
Flags: in-testsuite?
Flags: in-testsuite+

In theory, this could cause some strange problems. However, this is super obscure, arguably author error and I haven't been able to come up with a test case that causes a crash. So, I think we can just let this ride the trains.

Flags: needinfo?(jteh)
See Also: → 1593396
You need to log in before you can comment on or make changes to this bug.