Bug 1690456: Don't allow nsAccessibilityService to be shut down while a parent document is still shutting down. r=yzen

DocAccessible::Shutdown calls DocManager::NotifyOfDocumentShutdown, which can shut down nsAccessibilityService if there are no more consumers.
Previously, this could happen even when shutting down child documents.
Since shutting down the service shuts down all documents, this resulted in shutting down the parent document within an outer call to shut down that same document.
Even if that reentry were prevented, the service would have been gone when returning to the outer Shutdown call, which still needs the service to complete its cleanup.

To fix this, DocManager::NotifyOfDocumentShutdown takes an argument specifying whether to allow service shutdown.
This is set to false when shutting down child documents.
The service is thus allowed to shut down when returning to the parent document Shutdown.

In addition, mPresShell is cleared before shutting down child documents to prevent reentry like this.
While this should no longer happen, this should safeguard against similar pain in future.

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

accessible/base/DocManager.cpp

@@ -91,20 +91,22 @@ Accessible* DocManager::FindAccessibleInCache(nsINode* aNode) const {
   return nullptr;
 }
 
-void DocManager::RemoveFromXPCDocumentCache(DocAccessible* aDocument) {
+void DocManager::RemoveFromXPCDocumentCache(DocAccessible* aDocument,
+                                            bool aAllowServiceShutdown) {
   xpcAccessibleDocument* xpcDoc = mXPCDocumentCache.GetWeak(aDocument);
   if (xpcDoc) {
     xpcDoc->Shutdown();
     mXPCDocumentCache.Remove(aDocument);
 
-    if (!HasXPCDocuments()) {
+    if (aAllowServiceShutdown && !HasXPCDocuments()) {
       MaybeShutdownAccService(nsAccessibilityService::eXPCOM);
     }
   }
 }
 
 void DocManager::NotifyOfDocumentShutdown(DocAccessible* aDocument,
-                                          Document* aDOMDocument) {
+                                          Document* aDOMDocument,
+                                          bool aAllowServiceShutdown) {
   // We need to remove listeners in both cases, when document is being shutdown
   // or when accessibility service is being shut down as well.
   RemoveListeners(aDOMDocument);
@@ -115,7 +117,7 @@ void DocManager::NotifyOfDocumentShutdown(DocAccessible* aDocument,
     return;
   }
 
-  RemoveFromXPCDocumentCache(aDocument);
+  RemoveFromXPCDocumentCache(aDocument, aAllowServiceShutdown);
   mDocAccessibleCache.Remove(aDOMDocument);
 }
 

accessible/base/DocManager.h

@@ -56,11 +56,15 @@ class DocManager : public nsIWebProgressListener,
 
   /**
    * Called by document accessible when it gets shutdown.
+   * @param aAllowServiceShutdown true to shut down nsAccessibilityService
+   *        if it is no longer required, false to prevent it.
    */
   void NotifyOfDocumentShutdown(DocAccessible* aDocument,
-                                dom::Document* aDOMDocument);
+                                dom::Document* aDOMDocument,
+                                bool aAllowServiceShutdown = true);
 
-  void RemoveFromXPCDocumentCache(DocAccessible* aDocument);
+  void RemoveFromXPCDocumentCache(DocAccessible* aDocument,
+                                  bool aAllowServiceShutdown = true);
 
   /**
    * Return XPCOM accessible document.

accessible/generic/DocAccessible.cpp

@@ -396,6 +396,9 @@ void DocAccessible::Shutdown() {
 
   RemoveEventListeners();
 
+  // mParent->RemoveChild clears mParent, but we need to know whether we were a
+  // child later, so use a flag.
+  const bool isChild = !!mParent;
   if (mParent) {
     DocAccessible* parentDocument = mParent->Document();
     if (parentDocument) parentDocument->RemoveChildDocument(this);
@@ -404,6 +407,9 @@ void DocAccessible::Shutdown() {
     MOZ_ASSERT(!mParent, "Parent has to be null!");
   }
 
+  mPresShell->SetDocAccessible(nullptr);
+  mPresShell = nullptr;  // Avoid reentrancy
+
   // Walk the array backwards because child documents remove themselves from the
   // array as they are shutdown.
   int32_t childDocCount = mChildDocuments.Length();
@@ -424,9 +430,6 @@ void DocAccessible::Shutdown() {
     mVirtualCursor = nullptr;
   }
 
-  mPresShell->SetDocAccessible(nullptr);
-  mPresShell = nullptr;  // Avoid reentrancy
-
   mDependentIDsHashes.Clear();
   mNodeToAccessibleMap.Clear();
 
@@ -446,7 +449,13 @@ void DocAccessible::Shutdown() {
 
   HyperTextAccessibleWrap::Shutdown();
 
-  GetAccService()->NotifyOfDocumentShutdown(this, mDocumentNode);
+  MOZ_ASSERT(GetAccService());
+  GetAccService()->NotifyOfDocumentShutdown(
+      this, mDocumentNode,
+      // Make sure we don't shut down AccService while a parent document is
+      // still shutting down. The parent will allow service shutdown when it
+      // reaches this point.
+      /* aAllowServiceShutdown */ !isChild);
   mDocumentNode = nullptr;
 }