Bug 1553706: Fix IAccessible::accChild in the parent process for out-of-process iframes. r=eeejay
authorJames Teh <jteh@mozilla.com>
Mon, 03 Jun 2019 06:03:10 +0000
changeset 476692 38c2478a4825044a8c33bb72539b42fbdda833fc
parent 476691 379dc9cb2ae9fa7ceea647a35fcedb86fafb3bba
child 476702 c909c105f914f69054b9a7c6b189ee39fa1cad44
child 476703 ba1f75c56457051f5ea51cf9a2c5c51f9d7f3fa9
push id36104
push usercbrindusan@mozilla.com
push dateTue, 04 Jun 2019 03:45:41 +0000
treeherdermozilla-central@38c2478a4825 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseeejay
bugs1553706
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1553706: Fix IAccessible::accChild in the parent process for out-of-process iframes. r=eeejay It must be possible to retrieve any accessible by calling IAccessible::accChild on the RootAccessible (in the parent process) with the unique id of the desired accessible. Among other things, this is the way accessibility events are targeted on Windows. Previously, this code only searched remote documents at the top level of the tree. In order to support out-of-process iframes, it must now also search embedded documents at the top level of their content process. As part of this, the MSAA id must be set for out-of-process iframe documents, just as it is for top level documents. This was already being sent from the content process, but previously, we didn't store this in the parent process. Differential Revision: https://phabricator.services.mozilla.com/D32417
accessible/windows/msaa/AccessibleWrap.cpp
dom/ipc/BrowserParent.cpp
--- a/accessible/windows/msaa/AccessibleWrap.cpp
+++ b/accessible/windows/msaa/AccessibleWrap.cpp
@@ -30,16 +30,17 @@
 #  include "Logging.h"
 #endif
 
 #include "nsIMutableArray.h"
 #include "nsIFrame.h"
 #include "nsIScrollableFrame.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/dom/NodeInfo.h"
+#include "mozilla/dom/BrowserBridgeParent.h"
 #include "mozilla/dom/BrowserParent.h"
 #include "nsIServiceManager.h"
 #include "nsNameSpaceManager.h"
 #include "nsTextFormatter.h"
 #include "nsView.h"
 #include "nsViewManager.h"
 #include "nsEventMap.h"
 #include "nsArrayUtils.h"
@@ -1473,49 +1474,98 @@ already_AddRefed<IAccessible> Accessible
         disp->QueryInterface(IID_IAccessible, getter_AddRefs(result));
     MOZ_ASSERT(SUCCEEDED(hr));
     return result.forget();
   }
 
   return nullptr;
 }
 
+/**
+ * Visit DocAccessibleParent descendants of `aBrowser` that are at the top
+ * level of their content process.
+ * That is, IsTopLevelInContentProcess() will be true for each visited actor.
+ * Each visited actor will be an embedded document in a different content
+ * process to its embedder.
+ * The DocAccessibleParent for `aBrowser` itself is excluded.
+ * `aCallback` will be called for each DocAccessibleParent.
+ * The callback should return true to continue traversal, false to cease.
+ */
+template <typename Callback>
+static bool VisitDocAccessibleParentDescendantsAtTopLevelInContentProcess(
+    dom::BrowserParent* aBrowser, Callback aCallback) {
+  // We can't use BrowserBridgeParent::VisitAllDescendants because it doesn't
+  // provide a way to stop the search.
+  const auto& bridges = aBrowser->ManagedPBrowserBridgeParent();
+  for (auto iter = bridges.ConstIter(); !iter.Done(); iter.Next()) {
+    auto bridge = static_cast<dom::BrowserBridgeParent*>(iter.Get()->GetKey());
+    dom::BrowserParent* childBrowser = bridge->GetBrowserParent();
+    DocAccessibleParent* childDocAcc = childBrowser->GetTopLevelDocAccessible();
+    if (!childDocAcc || childDocAcc->IsShutdown()) {
+      continue;
+    }
+    if (!aCallback(childDocAcc)) {
+      return false;  // Stop traversal.
+    }
+    if (!VisitDocAccessibleParentDescendantsAtTopLevelInContentProcess(
+            childBrowser, aCallback)) {
+      return false;  // Stop traversal.
+    }
+  }
+  return true;  // Continue traversal.
+}
+
 already_AddRefed<IAccessible> AccessibleWrap::GetRemoteIAccessibleFor(
     const VARIANT& aVarChild) {
   a11y::RootAccessible* root = RootAccessible();
   const nsTArray<DocAccessibleParent*>* remoteDocs =
       DocManager::TopLevelRemoteDocs();
   if (!remoteDocs) {
     return nullptr;
   }
 
   RefPtr<IAccessible> result;
 
   // We intentionally leave the call to remoteDocs->Length() inside the loop
   // condition because it is possible for reentry to occur in the call to
   // GetProxiedAccessibleInSubtree() such that remoteDocs->Length() is mutated.
   for (size_t i = 0; i < remoteDocs->Length(); i++) {
-    DocAccessibleParent* remoteDoc = remoteDocs->ElementAt(i);
+    DocAccessibleParent* topRemoteDoc = remoteDocs->ElementAt(i);
 
-    uint32_t remoteDocMsaaId = WrapperFor(remoteDoc)->GetExistingID();
-    if (!sIDGen.IsSameContentProcessFor(aVarChild.lVal, remoteDocMsaaId)) {
-      continue;
-    }
-
-    Accessible* outerDoc = remoteDoc->OuterDocOfRemoteBrowser();
+    Accessible* outerDoc = topRemoteDoc->OuterDocOfRemoteBrowser();
     if (!outerDoc) {
       continue;
     }
 
     if (outerDoc->RootAccessible() != root) {
       continue;
     }
 
-    RefPtr<IDispatch> disp =
-        GetProxiedAccessibleInSubtree(remoteDoc, aVarChild);
+    RefPtr<IDispatch> disp;
+    auto checkDoc = [&aVarChild,
+                     &disp](DocAccessibleParent* aRemoteDoc) -> bool {
+      uint32_t remoteDocMsaaId = WrapperFor(aRemoteDoc)->GetExistingID();
+      if (!sIDGen.IsSameContentProcessFor(aVarChild.lVal, remoteDocMsaaId)) {
+        return true;  // Continue the search.
+      }
+      if ((disp = GetProxiedAccessibleInSubtree(aRemoteDoc, aVarChild))) {
+        return false;  // Found it! Stop traversal!
+      }
+      return true;  // Continue the search.
+    };
+
+    // Check the top level document for this id.
+    checkDoc(topRemoteDoc);
+    if (!disp) {
+      // The top level document doesn't contain this id. Recursively check any
+      // out-of-process iframe documents it embeds.
+      VisitDocAccessibleParentDescendantsAtTopLevelInContentProcess(
+          static_cast<dom::BrowserParent*>(topRemoteDoc->Manager()), checkDoc);
+    }
+
     if (!disp) {
       continue;
     }
 
     DebugOnly<HRESULT> hr =
         disp->QueryInterface(IID_IAccessible, getter_AddRefs(result));
     MOZ_ASSERT(SUCCEEDED(hr));
     return result.forget();
--- a/dom/ipc/BrowserParent.cpp
+++ b/dom/ipc/BrowserParent.cpp
@@ -1136,16 +1136,17 @@ mozilla::ipc::IPCResult BrowserParent::R
     if (!added) {
 #  ifdef DEBUG
       return added;
 #  else
       return IPC_OK();
 #  endif
     }
 #  ifdef XP_WIN
+    a11y::WrapperFor(doc)->SetID(aMsaaID);
     if (a11y::nsWinUtils::IsWindowEmulationStarted()) {
       doc->SetEmulatedWindowHandle(embedderDoc->GetEmulatedWindowHandle());
     }
 #  endif
     return IPC_OK();
   } else {
     // null aParentDoc means this document is at the top level in the child
     // process.  That means it makes no sense to get an id for an accessible