From 786351922e68417140224b341d72e9a8099789e9 Mon Sep 17 00:00:00 2001
From: James Teh <jteh@mozilla.com>
Date: Fri, 9 Aug 2024 01:45:31 +0000
Subject: [PATCH] Bug 1798620 part 2: Remove DocAccessibleParent::mParentDoc.
 r=eeejay

We have RemoteAccessible::mParent and RemoteAccessible::mDoc, so DocAccessibleParent::mParentDoc was redundant.
Aside from the redundancy, this was one extra thing we needed to keep up to date and reason about.

This involved changing the call to RemoveChildDoc in Destroy to use Unbind instead because Unbind clears the parent RemoteAccessible, but RemoveChildDoc didn't.
That meant we had a dangling RemoteAccessible::mParent pointer, which was always a problem, but is more problematic now that we no longer have mParentDoc and the destructor checks the parent document.
Since Unbind is the only caller of RemoveChildDoc, RemoveChildDoc has been merged into Unbind.
This means there is now only a single place where child documents are unbound from their parent, which should make things easier to reason about.

Differential Revision: https://phabricator.services.mozilla.com/D218802
---
 accessible/ipc/DocAccessibleParent.cpp | 15 ++++++---------
 accessible/ipc/DocAccessibleParent.h   | 24 +++++-------------------
 2 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/accessible/ipc/DocAccessibleParent.cpp b/accessible/ipc/DocAccessibleParent.cpp
index f46b68f2d035..9432ddfabc4a 100644
--- a/accessible/ipc/DocAccessibleParent.cpp
+++ b/accessible/ipc/DocAccessibleParent.cpp
@@ -42,7 +42,6 @@ uint64_t DocAccessibleParent::sMaxDocID = 0;
 
 DocAccessibleParent::DocAccessibleParent()
     : RemoteAccessible(this),
-      mParentDoc(kNoParentDoc),
 #if defined(XP_WIN)
       mEmulatedWindowHandle(nullptr),
 #endif  // defined(XP_WIN)
@@ -824,7 +823,6 @@ ipc::IPCResult DocAccessibleParent::AddChildDoc(DocAccessibleParent* aChildDoc,
   aChildDoc->SetParent(outerDoc);
   outerDoc->SetChildDoc(aChildDoc);
   mChildDocs.AppendElement(aChildDoc->mActorID);
-  aChildDoc->mParentDoc = mActorID;
 
   if (aCreating) {
     ProxyCreated(aChildDoc);
@@ -959,10 +957,10 @@ void DocAccessibleParent::Destroy() {
     return;
   }
 
-  if (DocAccessibleParent* parentDoc = thisDoc->ParentDoc()) {
-    parentDoc->RemoveChildDoc(thisDoc);
-  } else if (IsTopLevel()) {
+  if (IsTopLevel()) {
     GetAccService()->RemoteDocShutdown(this);
+  } else {
+    Unbind();
   }
 }
 
@@ -975,11 +973,10 @@ void DocAccessibleParent::ActorDestroy(ActorDestroyReason aWhy) {
 }
 
 DocAccessibleParent* DocAccessibleParent::ParentDoc() const {
-  if (mParentDoc == kNoParentDoc) {
-    return nullptr;
+  if (RemoteAccessible* parent = RemoteParent()) {
+    return parent->Document();
   }
-
-  return LiveDocs().Get(mParentDoc);
+  return nullptr;
 }
 
 bool DocAccessibleParent::CheckDocTree() const {
diff --git a/accessible/ipc/DocAccessibleParent.h b/accessible/ipc/DocAccessibleParent.h
index bb05fbafada4..317a76df2e76 100644
--- a/accessible/ipc/DocAccessibleParent.h
+++ b/accessible/ipc/DocAccessibleParent.h
@@ -157,8 +157,11 @@ class DocAccessibleParent : public RemoteAccessible,
       NotNull<PDocAccessibleParent*> aChildDoc, const uint64_t& aID) override;
 
   void Unbind() {
-    if (DocAccessibleParent* parent = ParentDoc()) {
-      parent->RemoveChildDoc(this);
+    if (RemoteAccessible* parent = RemoteParent()) {
+      DocAccessibleParent* parentDoc = parent->Document();
+      parent->ClearChildDoc(this);
+      DebugOnly<bool> result = parentDoc->mChildDocs.RemoveElement(mActorID);
+      MOZ_ASSERT(result);
     }
 
     SetParent(nullptr);
@@ -173,7 +176,6 @@ class DocAccessibleParent : public RemoteAccessible,
    * of the document this object represents.
    */
   DocAccessibleParent* ParentDoc() const;
-  static const uint64_t kNoParentDoc = UINT64_MAX;
 
   /**
    * Called when a document in a content process notifies the main process of a
@@ -195,21 +197,6 @@ class DocAccessibleParent : public RemoteAccessible,
     mPendingOOPChildDocs.Remove(aBridge);
   }
 
-  /*
-   * Called when the document in the content process this object represents
-   * notifies the main process a child document has been removed.
-   */
-  void RemoveChildDoc(DocAccessibleParent* aChildDoc) {
-    RemoteAccessible* parent = aChildDoc->RemoteParent();
-    MOZ_ASSERT(parent);
-    if (parent) {
-      aChildDoc->RemoteParent()->ClearChildDoc(aChildDoc);
-    }
-    DebugOnly<bool> result = mChildDocs.RemoveElement(aChildDoc->mActorID);
-    aChildDoc->mParentDoc = kNoParentDoc;
-    MOZ_ASSERT(result);
-  }
-
   void RemoveAccessible(RemoteAccessible* aAccessible) {
     MOZ_DIAGNOSTIC_ASSERT(mAccessibles.GetEntry(aAccessible->ID()));
     mAccessibles.RemoveEntry(aAccessible->ID());
@@ -357,7 +344,6 @@ class DocAccessibleParent : public RemoteAccessible,
   void ShutdownOrPrepareForMove(RemoteAccessible* aAcc);
 
   nsTArray<uint64_t> mChildDocs;
-  uint64_t mParentDoc;
 
 #if defined(XP_WIN)
   // The handle associated with the emulated window that contains this document