Bug 1800777: Don't recreate reverse rel maps for accs that have been shutdown r=Jamie

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

accessible/generic/LocalAccessible.cpp

@@ -3652,7 +3652,8 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache(
       if (data.mType == RelationType::LABEL_FOR) {
         // Labels are a special case -- we need to validate that the target of
         // their `for` attribute is in fact labelable. DOM checks this when we
-        // call GetControl().
+        // call GetControl(). If a label contains an element we will return it
+        // here.
         if (dom::HTMLLabelElement* labelEl =
                 dom::HTMLLabelElement::FromNode(mContent)) {
           rel.AppendTarget(mDoc, labelEl->GetControl());

accessible/ipc/RemoteAccessibleBase.cpp

@@ -935,15 +935,23 @@ nsTArray<bool> RemoteAccessibleBase<Derived>::PreProcessRelations(
               mCachedFields->GetAttribute<nsTArray<uint64_t>>(relAtom)) {
         for (uint64_t id : *maybeOldIDs) {
           // For each target, fetch its reverse relation map
-          nsTHashMap<nsUint64HashKey, nsTArray<uint64_t>>& reverseRels =
-              Document()->mReverseRelations.LookupOrInsert(id);
-          // Then fetch its reverse relation's ID list
-          nsTArray<uint64_t>& reverseRelIDs = reverseRels.LookupOrInsert(
-              static_cast<uint64_t>(*data.mReverseType));
-          //  There might be other reverse relations stored for this acc, so
-          //  remove our ID instead of deleting the array entirely.
-          DebugOnly<bool> removed = reverseRelIDs.RemoveElement(ID());
-          MOZ_ASSERT(removed, "Can't find old reverse relation");
+          // We need to call `Lookup` here instead of `LookupOrInsert` because
+          // it's possible the ID we're querying is from an acc that has since
+          // been Shutdown(), and so has intentionally removed its reverse rels
+          // from the doc's reverse rel cache.
+          if (auto reverseRels = Document()->mReverseRelations.Lookup(id)) {
+            // Then fetch its reverse relation's ID list. This should be safe
+            // to do via LookupOrInsert because by the time we've gotten here,
+            // we know the acc and `this` are still alive in the doc. If we hit
+            // the following assert, we don't have parity on implicit/explicit
+            // rels and something is wrong.
+            nsTArray<uint64_t>& reverseRelIDs = reverseRels->LookupOrInsert(
+                static_cast<uint64_t>(*data.mReverseType));
+            //  There might be other reverse relations stored for this acc, so
+            //  remove our ID instead of deleting the array entirely.
+            DebugOnly<bool> removed = reverseRelIDs.RemoveElement(ID());
+            MOZ_ASSERT(removed, "Can't find old reverse relation");
+          }
         }
       }
     }

accessible/tests/browser/e10s/browser_caching_relations_002.js

@@ -82,6 +82,44 @@ addAccessibleTask(
   }
 );
 
+/*
+ * Test mutation of LABEL relations via accessible shutdown.
+ */
+addAccessibleTask(
+  `
+  <div id="d"></div>
+  <label id="l">
+    <select id="s">
+  `,
+  async function(browser, accDoc) {
+    const label = findAccessibleChildByID(accDoc, "l");
+    const select = findAccessibleChildByID(accDoc, "s");
+    const div = findAccessibleChildByID(accDoc, "d");
+
+    await testCachedRelation(label, RELATION_LABEL_FOR, select);
+    await testCachedRelation(select, RELATION_LABELLED_BY, label);
+    await testCachedRelation(div, RELATION_LABELLED_BY, null);
+
+    const r = waitForEvent(EVENT_REORDER, "l");
+    await invokeContentTask(browser, [], () => {
+      content.document.getElementById("s").remove();
+    });
+    await r;
+    await invokeContentTask(browser, [], () => {
+      const l = content.document.getElementById("l");
+      l.htmlFor = "d";
+    });
+    await testCachedRelation(label, RELATION_LABEL_FOR, div);
+    await testCachedRelation(div, RELATION_LABELLED_BY, label);
+  },
+  {
+    chrome: false,
+    iframe: isCacheEnabled,
+    remoteIframe: isCacheEnabled,
+    topLevel: isCacheEnabled,
+  }
+);
+
 /*
  * Test LINKS_TO relation caching an anchor with multiple hashes
  */