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

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: 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)

Comment 1

[Ryan VanderMeulen [:RyanVM]]

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

Comment 2

[alexander :surkov (:asurkov)]

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.

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

I am not familiar with that code, no, sorry.

Comment 4

[alexander :surkov (:asurkov)]

let's set P2 for now, until we investigate the issue further, to keep it on radar. cc'ing Aaron

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

+Jamie

Comment 6

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 7

[Intermittent Failures Robot]

1 failures in 4666 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-inbound: 1

Platform breakdown:

• linux64-qr: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-08-05&endday=2019-08-11&tree=all

Comment 8

[Intermittent Failures Robot]

2 failures in 5073 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-inbound: 1
• autoland: 1

Platform breakdown:

• linux64-qr: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-08-19&endday=2019-08-25&tree=all

Comment 10

[Intermittent Failures Robot]

8 failures in 4350 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 2
• autoland: 5
• mozilla-central: 1

Platform breakdown:

• linux64-qr: 8

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-09-16&endday=2019-09-22&tree=all

Comment 11

[Intermittent Failures Robot]

2 failures in 4530 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1
• mozilla-beta: 1

Platform breakdown:

• linux64-qr: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-09-23&endday=2019-09-29&tree=all

Comment 12

[Tyson Smith [:tsmith]]

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

Comment 13

[Intermittent Failures Robot]

3 failures in 4292 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 3

Platform breakdown:

• linux64-qr: 3

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-09-30&endday=2019-10-06&tree=all

Comment 14

[Intermittent Failures Robot]

5 failures in 3801 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 2
• mozilla-central: 3

Platform breakdown:

• linux64-qr: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-10-07&endday=2019-10-13&tree=all

Comment 15

[James Teh [:Jamie]]

(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)

Comment 16

[Tyson Smith [:tsmith]]

(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.

Comment 17

[James Teh [:Jamie]]

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?

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 19

[James Teh [:Jamie]]

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

Comment 20

[Morgan Reschenberg [:morgan]]

(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

Comment 21

[Tyson Smith [:tsmith]]

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

Comment 22

[Intermittent Failures Robot]

4 failures in 3964 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-inbound: 1
• autoland: 2
• try: 1

Platform breakdown:

• linux64-qr: 4

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-10-14&endday=2019-10-20&tree=all

Comment 23

[James Teh [:Jamie]]

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...

Comment 24

[James Teh [:Jamie]]

Try build with the fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=64428f6dd94f3272c58a07dc7055fb45d4128d22

Comment 25

[James Teh [:Jamie]]

Attachment: Bug 1415667: Always use OuterDocAccessible for iframes, even if an ARIA table role is specified.

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.

Comment 26

[James Teh [:Jamie]]

Windows try with the test but without the fix:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3576a7618747e418b430e2722f4918880fa4f71e

Comment 27

[James Teh [:Jamie]]

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.

Comment 28

[Intermittent Failures Robot]

2 failures in 4182 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 1
• autoland: 1

Platform breakdown:

• linux64-qr: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1415667&startday=2019-10-21&endday=2019-10-27&tree=all

Comment 29

[Yura Zenevich [:yzen] (please use "needinfo")]

(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

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8d297ad5ac5
Always use OuterDocAccessible for iframes, even if an ARIA table role is specified. r=yzen

Comment 31

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a8d297ad5ac5

Comment 32

[James Teh [:Jamie]]

(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.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

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

Comment 34

[James Teh [:Jamie]]

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.