Closed Bug 1713680 Opened 2 months ago Closed 1 month ago

Crash in [@ mozilla::a11y::DocAccessibleParent::AddChildDoc] MOZ_DIAGNOSTIC_ASSERT(false) (Binding to parent that isn't a valid OuterDoc!)

Categories

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

defect

Tracking

()

RESOLVED FIXED
91 Branch
Fission Milestone M7a
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- wontfix
firefox91 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

Reported in bug 1669748 comment 19. Same signature, but slightly different crash: this hits an assertion I added in bug 1712210.

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/7624dc19-bf06-411f-8e29-3e7c70210528

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (Binding to parent that isn't a valid OuterDoc!)

Top 10 frames of crashing thread:

0 xul.dll mozilla::a11y::DocAccessibleParent::AddChildDoc accessible/ipc/DocAccessibleParent.cpp:608
1 xul.dll mozilla::a11y::DocAccessibleParent::AddSubtree accessible/ipc/DocAccessibleParent.cpp:151
2 xul.dll mozilla::a11y::DocAccessibleParent::RecvShowEvent accessible/ipc/DocAccessibleParent.cpp:78
3 xul.dll mozilla::a11y::PDocAccessibleParent::OnMessageReceived ipc/ipdl/PDocAccessibleParent.cpp:302
4 xul.dll mozilla::dom::PContentParent::OnMessageReceived ipc/ipdl/PContentParent.cpp:6677
5 xul.dll mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2079
6 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:766
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1159
8 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:85
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:328
Fission Milestone: --- → ?
Blocks: a11y-fission

Previously, we only deferred SendSetEmbedderAccessible calls for in-process iframes if the DocAccessibleChild hadn't sent its constructor yet.
This meant that top level content process documents (tab documents or OOP iframes) called SendSetEmbedderAccessible even if a11y insertions/removals were still being deferred.

Fission Milestone: ? → M7a
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3bb5bfa18cf9
When deferring a11y IPC calls, always defer calls to BrowserBridgeChild::SendSetEmbedderAccessible too. r=eeejay
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(jteh)

Still crashing. 😩
Windows: bp-95cc4ca0-e601-437f-a133-b482a0210607
Also, we have some crashes with the same assertion on Linux: bp-0a2adbf1-4ab1-41d1-9c2e-5fd660210603
I originally thought this was Windows specific due to the deferring of IPC calls on Windows and my patch attempted to fix that. However, it seems this isn't the only way we can hit this problem.

Status: RESOLVED → REOPENED
Flags: needinfo?(jteh)
Resolution: FIXED → ---
OS: Windows 10 → All
Hardware: Unspecified → All

New hypothesis: it looks like a new PBrowserBridge gets created when you change the src of an iframe:
data:text/html,<iframe id="iframe" src="https://google.com/nf"></iframe><button onclick="iframe.src = 'https://example.com/';">change
That means that BrowserBridgeParent::RecvSetEmbedderAccessible wouldn't clean up a stale pending addition for the old iframe document, since it was a different BrowserBridgeParent. I'm not quite sure how this results in this assertion, since the OuterDoc should still be the same and thus have the same id, but perhaps this can combine with layout frame reconstruction or similar to create strange results.

An OuterDocAccessible can be recreated, causing the embedder accessible for a BrowserBridgeParent (OOP iframe) to change.
However, changing the src of an iframe can also cause a new BrowserBridgeParent to be created.
Previously, if addition of the child document was still pending when this occurred (because the OuterDocAccessible hadn't been sent to the parent yet), this pending addition could remain, causing problems if the id was reused later.

To fix this (and to hopefully make this more robust given the continued problems we're seeing in the wild with this area of the code), I've completely refactored the way we handle these pending child doc additions.
Rather than tracking the pending additions by their accessible id and child doc, we track them by their BrowserBridgeParent.
This way, we're closest to a single source of truth.
We also remove a pending addition when an associated BrowserBridgeParent is destroyed.

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/238ccbc3793f
Correctly handle the case where the BrowserBridgeParent for an OOP iframe changes while addition of its DocAccessibleParent is still pending. r=eeejay

Backed out changeset 238ccbc3793f (Bug 1713680) for causing bustages on BrowserBridgeParent.cpp
Backout link
Push with failures
Failure Log

Flags: needinfo?(jteh)
Flags: needinfo?(jteh)
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0a20cd5b703
Correctly handle the case where the BrowserBridgeParent for an OOP iframe changes while addition of its DocAccessibleParent is still pending. r=eeejay
Status: REOPENED → RESOLVED
Closed: 2 months ago1 month ago
Resolution: --- → FIXED
See Also: → 1721666
You need to log in before you can comment on or make changes to this bug.