Bug 1710975: Fix child/sibling functions regarding top level remote documents. r=eeejay

1. OuterDocAccessible::ChildCount was previously Windows only. Implement it for all platforms.
2. OuterDocAccessible::ChildAt included  code for Windows which isn't necessary because it calls AccessibleWrap::LocalChildAt, not OuterDocAccessible::LocalChildAt (where the Windows specific RemoteAccessibleWrap stuff is implemented).
3. LocalAccessible::ChildAtPoint previously expected LocalChildAt() to succeed if ChildCount > 1. As per 1), this is no longer true for OuterDocAccessibles containing a remote document. Adjust accordingly.
4. IndexInParent on a top level remote document was previously returning -1. Implement DocAccessibleParent::IndexInParent to fix this.
5. Doing 4) means that RemoteNext/PrevSibling broke for top level documents because they use IndexInParent, but there is no remote parent. Add a specific early return for documents there.

Differential Revision: https://phabricator.services.mozilla.com/D115045

accessible/generic/LocalAccessible.cpp

@@ -590,6 +590,11 @@ LocalAccessible* LocalAccessible::LocalChildAtPoint(
   // sub documents (XXX: subdocuments should be handled by methods of
   // OuterDocAccessibles).
   uint32_t childCount = accessible->ChildCount();
+  if (childCount == 1 && accessible->IsOuterDoc() &&
+      accessible->FirstChild()->IsRemote()) {
+    // No local children.
+    return accessible;
+  }
   for (uint32_t childIdx = 0; childIdx < childCount; childIdx++) {
     LocalAccessible* child = accessible->LocalChildAt(childIdx);
 

accessible/generic/OuterDocAccessible.cpp

@@ -214,18 +214,10 @@ LocalAccessible* OuterDocAccessible::RemoteChildDocAccessible() const {
   return nullptr;
 }
 
-// On Windows e10s, since we don't cache in the chrome process, these next two
-// functions must be implemented so that we properly cross the chrome-to-content
+// On Windows e10s, since we don't cache in the chrome process, LocalChildAt
+// must be implemented so that we properly cross the chrome-to-content
 // boundary when traversing.
 
-uint32_t OuterDocAccessible::ChildCount() const {
-  uint32_t result = mChildren.Length();
-  if (!result && RemoteChildDocAccessible()) {
-    result = 1;
-  }
-  return result;
-}
-
 LocalAccessible* OuterDocAccessible::LocalChildAt(uint32_t aIndex) const {
   LocalAccessible* result = AccessibleWrap::LocalChildAt(aIndex);
   if (result || aIndex) {
@@ -240,19 +232,29 @@ LocalAccessible* OuterDocAccessible::LocalChildAt(uint32_t aIndex) const {
 
 // Accessible
 
+uint32_t OuterDocAccessible::ChildCount() const {
+  uint32_t result = mChildren.Length();
+  if (!result &&
+#if defined(XP_WIN)
+      // On Windows, as well as returning 1 for a remote document in the parent
+      // process, we also need to return 1 in a content process for an OOP
+      // iframe.
+      RemoteChildDocAccessible()
+#else
+      RemoteChildDoc()
+#endif
+  ) {
+    result = 1;
+  }
+  return result;
+}
+
 Accessible* OuterDocAccessible::ChildAt(uint32_t aIndex) const {
+  // We deliberately bypass OuterDocAccessible::LocalChildAt on Windows because
+  // it will return a RemoteAccessibleWrap for a remote document.
   LocalAccessible* result = AccessibleWrap::LocalChildAt(aIndex);
   if (result || aIndex) {
-#if defined(XP_WIN)
-    // On Windows, AccessibleWrap::LocalChildAt can return a proxy wrapper
-    // for a remote document. These aren't real Accessibles so we skip this
-    // block and retrieve the remote child doc.
-    if (!result || !result->IsProxy()) {
-      return result;
-    }
-#else
     return result;
-#endif  // defined(XP_WIN)
   }
 
   return RemoteChildDoc();

accessible/generic/OuterDocAccessible.h

@@ -57,8 +57,8 @@ class OuterDocAccessible final : public AccessibleWrap {
   virtual bool RemoveChild(LocalAccessible* aAccessible) override;
   virtual bool IsAcceptableChild(nsIContent* aEl) const override;
 
-#if defined(XP_WIN)
   virtual uint32_t ChildCount() const override;
+#if defined(XP_WIN)
   virtual LocalAccessible* LocalChildAt(uint32_t aIndex) const override;
 #endif  // defined(XP_WIN)
 

accessible/ipc/DocAccessibleParent.h

@@ -262,6 +262,15 @@ class DocAccessibleParent : public RemoteAccessible,
    */
   Tuple<DocAccessibleParent*, uint64_t> GetRemoteEmbedder();
 
+  // Accessible
+  virtual int32_t IndexInParent() const override {
+    if (IsTopLevel() && OuterDocOfRemoteBrowser()) {
+      // An OuterDoc can only have 1 child.
+      return 0;
+    }
+    return RemoteAccessible::IndexInParent();
+  }
+
  private:
   ~DocAccessibleParent() {
     LiveDocs().Remove(mActorID);

accessible/ipc/RemoteAccessibleBase.h

@@ -46,6 +46,12 @@ class RemoteAccessibleBase : public Accessible {
     return mChildren.Length() ? mChildren[mChildren.Length() - 1] : nullptr;
   }
   Derived* RemotePrevSibling() const {
+    if (IsDoc()) {
+      // The normal code path doesn't work for documents because the parent
+      // might be a local OuterDoc, but IndexInParent() will return 1.
+      // A document is always a single child of an OuterDoc anyway.
+      return nullptr;
+    }
     int32_t idx = IndexInParent();
     if (idx == -1) {
       return nullptr;  // No parent.
@@ -53,6 +59,12 @@ class RemoteAccessibleBase : public Accessible {
     return idx > 0 ? RemoteParent()->mChildren[idx - 1] : nullptr;
   }
   Derived* RemoteNextSibling() const {
+    if (IsDoc()) {
+      // The normal code path doesn't work for documents because the parent
+      // might be a local OuterDoc, but IndexInParent() will return 1.
+      // A document is always a single child of an OuterDoc anyway.
+      return nullptr;
+    }
     int32_t idx = IndexInParent();
     if (idx == -1) {
       return nullptr;  // No parent.

accessible/tests/browser/tree/browser.ini

@@ -7,6 +7,7 @@ support-files =
 
 [browser_aria_owns.js]
 skip-if = true || (verify && !debug && (os == 'linux')) #Bug 1445513
+[browser_browser_element.js]
 [browser_lazy_tabs.js]
 [browser_searchbar.js]
 [browser_shadowdom.js]

accessible/tests/browser/tree/browser_browser_element.js

@@ -0,0 +1,16 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/* import-globals-from ../../mochitest/role.js */
+loadScripts({ name: "role.js", dir: MOCHITESTS_DIR });
+
+// Test that the tree is correct for browser elements containing remote
+// documents.
+addAccessibleTask(`test`, async function(browser, docAcc) {
+  // testAccessibleTree also verifies childCount, indexInParent and parent.
+  testAccessibleTree(browser, {
+    INTERNAL_FRAME: [{ DOCUMENT: [{ TEXT_LEAF: [] }] }],
+  });
+});