Bug 1705542: Don't assume an OuterDoc RemoteAccessible has a child document. r=eeejay
authorJames Teh <jteh@mozilla.com>
Tue, 11 May 2021 23:17:08 +0000
changeset 579434 f99981eb9e49a1da41854ce510165cc573ec80f6
parent 579433 109922bd00ad29feaefa745b46aa580af471300e
child 579435 d10063d1349d834c15ccdc11bdac3ba0363ab083
push id38455
push usercsabou@mozilla.com
push dateWed, 12 May 2021 04:08:16 +0000
treeherdermozilla-central@f99981eb9e49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseeejay
bugs1705542
milestone90.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 1705542: Don't assume an OuterDoc RemoteAccessible has a child document. r=eeejay An OuterDoc RemoteAccessible can have no child document for a short time if the embedded doc is changed. As part of this, get rid of the mOuterDoc variable, since it is now redundant and somewhat misleading. Instead, use IsOuterDoc(), since RemoteAccessible now has acc types. Differential Revision: https://phabricator.services.mozilla.com/D114431
accessible/ipc/RemoteAccessibleBase.cpp
accessible/ipc/RemoteAccessibleBase.h
accessible/ipc/other/RemoteAccessible.cpp
--- a/accessible/ipc/RemoteAccessibleBase.cpp
+++ b/accessible/ipc/RemoteAccessibleBase.cpp
@@ -27,17 +27,17 @@ void RemoteAccessibleBase<Derived>::Shut
       GetAccService()->GetCachedXPCDocument(Document());
   if (xpcDoc) {
     xpcDoc->NotifyOfShutdown(static_cast<Derived*>(this));
   }
 
   // XXX Ideally  this wouldn't be necessary, but it seems OuterDoc accessibles
   // can be destroyed before the doc they own.
   uint32_t childCount = mChildren.Length();
-  if (!mOuterDoc) {
+  if (!IsOuterDoc()) {
     for (uint32_t idx = 0; idx < childCount; idx++) mChildren[idx]->Shutdown();
   } else {
     if (childCount > 1) {
       MOZ_CRASH("outer doc has too many documents!");
     } else if (childCount == 1) {
       mChildren[0]->AsDoc()->Unbind();
     }
   }
@@ -48,17 +48,16 @@ void RemoteAccessibleBase<Derived>::Shut
 }
 
 template <class Derived>
 void RemoteAccessibleBase<Derived>::SetChildDoc(
     DocAccessibleParent* aChildDoc) {
   MOZ_ASSERT(aChildDoc);
   MOZ_ASSERT(mChildren.Length() == 0);
   mChildren.AppendElement(aChildDoc);
-  mOuterDoc = true;
 }
 
 template <class Derived>
 void RemoteAccessibleBase<Derived>::ClearChildDoc(
     DocAccessibleParent* aChildDoc) {
   MOZ_ASSERT(aChildDoc);
   // This is possible if we're replacing one document with another: Doc 1
   // has not had a chance to remove itself, but was already replaced by Doc 2
--- a/accessible/ipc/RemoteAccessibleBase.h
+++ b/accessible/ipc/RemoteAccessibleBase.h
@@ -175,27 +175,25 @@ class RemoteAccessibleBase : public Acce
   RemoteAccessibleBase(uint64_t aID, Derived* aParent,
                        DocAccessibleParent* aDoc, role aRole, AccType aType,
                        AccGenericType aGenericTypes, uint8_t aRoleMapEntryIndex)
       : Accessible(aType, aGenericTypes, aRoleMapEntryIndex),
         mParent(aParent->ID()),
         mDoc(aDoc),
         mWrapper(0),
         mID(aID),
-        mRole(aRole),
-        mOuterDoc(false) {}
+        mRole(aRole) {}
 
   explicit RemoteAccessibleBase(DocAccessibleParent* aThisAsDoc)
       : Accessible(),
         mParent(kNoParent),
         mDoc(aThisAsDoc),
         mWrapper(0),
         mID(0),
-        mRole(roles::DOCUMENT),
-        mOuterDoc(false) {
+        mRole(roles::DOCUMENT) {
     mGenericTypes = eDocument | eHyperText;
   }
 
  protected:
   void SetParent(Derived* aParent);
 
  private:
   uintptr_t mParent;
@@ -207,19 +205,16 @@ class RemoteAccessibleBase : public Acce
   DocAccessibleParent* mDoc;
   uintptr_t mWrapper;
   uint64_t mID;
 
  protected:
   // XXX DocAccessibleParent gets to change this to change the role of
   // documents.
   role mRole : 27;
-
- private:
-  bool mOuterDoc : 1;
 };
 
 extern template class RemoteAccessibleBase<RemoteAccessible>;
 
 }  // namespace a11y
 }  // namespace mozilla
 
 #endif
--- a/accessible/ipc/other/RemoteAccessible.cpp
+++ b/accessible/ipc/other/RemoteAccessible.cpp
@@ -767,23 +767,25 @@ double RemoteAccessible::Step() {
   double step = UnspecifiedNaN<double>();
   Unused << mDoc->SendStep(mID, &step);
   return step;
 }
 
 void RemoteAccessible::TakeFocus() { Unused << mDoc->SendTakeFocus(mID); }
 
 RemoteAccessible* RemoteAccessible::FocusedChild() {
-  if (mOuterDoc) {
+  if (IsOuterDoc()) {
     // If FocusedChild was called on an outer doc, it should behave
     // like a non-doc accessible and return its focused child, or null.
     // If the inner doc is OOP (fission), calling FocusedChild on the outer
     // doc would return null.
-    MOZ_ASSERT(ChildCount() == 1);
     RemoteAccessible* child = RemoteFirstChild();
+    if (!child) {
+      return (State() & states::FOCUSED) ? this : nullptr;
+    }
     MOZ_ASSERT(child->IsDoc());
     return (child->State() & states::FOCUSED) ? child : nullptr;
   }
 
   auto* doc = mDoc;
   uint64_t id = mID;
   if (IsDoc()) {
     // If this is a doc we should return the focused descendant, not just the
@@ -807,18 +809,25 @@ RemoteAccessible* RemoteAccessible::Focu
   // If useDoc is null, this means there is no focused child.
   return useDoc ? useDoc->GetAccessible(resultID) : nullptr;
 }
 
 Accessible* RemoteAccessible::ChildAtPoint(
     int32_t aX, int32_t aY, LocalAccessible::EWhichChildAtPoint aWhichChild) {
   RemoteAccessible* target = this;
   do {
-    if (target->mOuterDoc) {
-      MOZ_ASSERT(target->ChildCount() == 1);
+    if (target->IsOuterDoc()) {
+      if (target->ChildCount() == 0) {
+        // Return the OuterDoc if the requested point is within its bounds.
+        nsIntRect rect = target->Bounds();
+        if (rect.Contains(aX, aY)) {
+          return target;
+        }
+        return nullptr;
+      }
       DocAccessibleParent* childDoc = target->RemoteChildAt(0)->AsDoc();
       MOZ_ASSERT(childDoc);
       if (childDoc->IsTopLevelInContentProcess()) {
         // This is an OOP iframe. Remote calls can only work within their
         // process, so they stop at OOP iframes.
         if (aWhichChild == Accessible::EWhichChildAtPoint::DirectChild) {
           // Return the child document if it's within the bounds of the iframe.
           nsIntRect docRect = target->Bounds();
@@ -834,17 +843,17 @@ Accessible* RemoteAccessible::ChildAtPoi
     PDocAccessibleParent* resultDoc = nullptr;
     uint64_t resultID = 0;
     Unused << target->mDoc->SendChildAtPoint(target->mID, aX, aY,
                                              static_cast<uint32_t>(aWhichChild),
                                              &resultDoc, &resultID);
     // If resultDoc is null, this means there is no child at this point.
     auto useDoc = static_cast<DocAccessibleParent*>(resultDoc);
     target = resultDoc ? useDoc->GetAccessible(resultID) : nullptr;
-  } while (target && target->mOuterDoc &&
+  } while (target && target->IsOuterDoc() &&
            aWhichChild == Accessible::EWhichChildAtPoint::DeepestChild);
   return target;
 }
 
 nsIntRect RemoteAccessible::Bounds() {
   nsIntRect rect;
   Unused << mDoc->SendExtents(mID, false, &(rect.x), &(rect.y), &(rect.width),
                               &(rect.height));