Skip to content

Commit 1778c77

Browse files
committed
Bug 1789474 - Allow detached documents to be overwritten in accessible mapping. r=Jamie,geckoview-reviewers,owlish
Also assure that unregistering an accessible removes the right one and doesn't confuse the attached document with the detached one. Add stderr output to Accessible::DebugPrint in Android as well. Differential Revision: https://phabricator.services.mozilla.com/D157528
1 parent dc387b5 commit 1778c77

File tree

3 files changed

+57
-4
lines changed

3 files changed

+57
-4
lines changed

accessible/android/SessionAccessibility.cpp

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,17 @@ Accessible* SessionAccessibility::GetAccessibleByID(int32_t aID) const {
10081008
return accessible;
10091009
}
10101010

1011+
#ifdef DEBUG
1012+
static bool IsDetachedDoc(Accessible* aAccessible) {
1013+
if (!aAccessible->IsRemote() || !aAccessible->AsRemote()->IsDoc()) {
1014+
return false;
1015+
}
1016+
1017+
return !aAccessible->Parent() ||
1018+
aAccessible->Parent()->FirstChild() != aAccessible;
1019+
}
1020+
#endif
1021+
10111022
void SessionAccessibility::RegisterAccessible(Accessible* aAccessible) {
10121023
if (IPCAccessibilityActive()) {
10131024
// Don't register accessible in content process.
@@ -1044,8 +1055,14 @@ void SessionAccessibility::RegisterAccessible(Accessible* aAccessible) {
10441055
}
10451056
AccessibleWrap::SetVirtualViewID(aAccessible, virtualViewID);
10461057

1047-
MOZ_ASSERT(!sessionAcc->mIDToAccessibleMap.Contains(virtualViewID),
1048-
"ID already registered");
1058+
Accessible* oldAcc = sessionAcc->mIDToAccessibleMap.Get(virtualViewID);
1059+
if (oldAcc) {
1060+
// About to overwrite mapping of registered accessible. This should
1061+
// only happen when the registered accessible is a detached document.
1062+
MOZ_ASSERT(IsDetachedDoc(oldAcc),
1063+
"ID already registered to non-detached document");
1064+
}
1065+
10491066
sessionAcc->mIDToAccessibleMap.InsertOrUpdate(virtualViewID, aAccessible);
10501067
}
10511068

@@ -1064,8 +1081,21 @@ void SessionAccessibility::UnregisterAccessible(Accessible* aAccessible) {
10641081
RefPtr<SessionAccessibility> sessionAcc = GetInstanceFor(aAccessible);
10651082
MOZ_ASSERT(sessionAcc, "Need SessionAccessibility to unregister Accessible!");
10661083
if (sessionAcc) {
1067-
MOZ_ASSERT(sessionAcc->mIDToAccessibleMap.Contains(virtualViewID),
1068-
"Unregistering unregistered accessible");
1084+
Accessible* registeredAcc =
1085+
sessionAcc->mIDToAccessibleMap.Get(virtualViewID);
1086+
if (registeredAcc != aAccessible) {
1087+
// Attempting to unregister an accessible that is not mapped to
1088+
// its virtual view ID. This probably means it is a detached document
1089+
// and a more recent document overwrote its '-1' mapping.
1090+
// We set its own virtual view ID to `kUnsetID` and return early.
1091+
MOZ_ASSERT(!registeredAcc || IsDetachedDoc(aAccessible),
1092+
"Accessible is detached document");
1093+
AccessibleWrap::SetVirtualViewID(aAccessible, kUnsetID);
1094+
return;
1095+
}
1096+
1097+
MOZ_ASSERT(registeredAcc, "Unregistering unregistered accessible");
1098+
MOZ_ASSERT(registeredAcc == aAccessible, "Unregistering wrong accessible");
10691099
sessionAcc->mIDToAccessibleMap.Remove(virtualViewID);
10701100
}
10711101

accessible/basetypes/Accessible.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,11 @@ void Accessible::DebugPrint(const char* aPrefix,
396396
const Accessible* aAccessible) {
397397
nsAutoCString desc;
398398
aAccessible->DebugDescription(desc);
399+
# if defined(ANDROID)
400+
printf_stderr("%s %s\n", aPrefix, desc.get());
401+
# else
399402
printf("%s %s\n", aPrefix, desc.get());
403+
# endif
400404
}
401405

402406
#endif

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,25 @@ class AccessibilityTest : BaseSessionTest() {
13481348
assertThat("Button has correct text", buttonNode.text.toString(), equalTo("Submit"))
13491349
}
13501350

1351+
@Test fun testLoadUnloadIframeDoc() {
1352+
mainSession.loadTestPath(REMOTE_IFRAME)
1353+
waitForInitialFocus()
1354+
1355+
loadTestPage("test-tree")
1356+
waitForInitialFocus()
1357+
1358+
mainSession.loadTestPath(REMOTE_IFRAME)
1359+
waitForInitialFocus()
1360+
1361+
loadTestPage("test-tree")
1362+
waitForInitialFocus()
1363+
1364+
mainSession.loadTestPath(REMOTE_IFRAME)
1365+
waitForInitialFocus()
1366+
1367+
loadTestPage("test-tree")
1368+
waitForInitialFocus()
1369+
}
13511370

13521371
private fun testAccessibilityFocusIframe(page: String) {
13531372
var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID

0 commit comments

Comments
 (0)