Bug 1801234: Queue a relations cache update on dependent accs when DOM ID mutations are observed r=Jamie

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

accessible/generic/DocAccessible.cpp

@@ -377,6 +377,29 @@ void DocAccessible::QueueCacheUpdate(LocalAccessible* aAcc,
   Controller()->ScheduleProcessing();
 }
 
+void DocAccessible::QueueCacheUpdateForDependentRelations(
+    LocalAccessible* aAcc) {
+  if (!mIPCDoc || !StaticPrefs::accessibility_cache_enabled_AtStartup() ||
+      !aAcc || !aAcc->Elm() || !aAcc->IsInDocument() || aAcc->IsDefunct()) {
+    return;
+  }
+  nsAutoString ID;
+  aAcc->DOMNodeID(ID);
+  if (AttrRelProviders* list = GetRelProviders(aAcc->Elm(), ID)) {
+    // We call this function when we've noticed an ID change, or when an acc
+    // is getting bound to its document. We need to ensure any existing accs
+    // that depend on this acc's ID have their rel cache entries updated.
+    for (const auto& provider : *list) {
+      LocalAccessible* relatedAcc = GetAccessible(provider->mContent);
+      if (!relatedAcc || relatedAcc->IsDefunct() ||
+          !relatedAcc->IsInDocument()) {
+        continue;
+      }
+      QueueCacheUpdate(relatedAcc, CacheDomain::Relations);
+    }
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // LocalAccessible
 
@@ -829,6 +852,7 @@ void DocAccessible::AttributeChanged(dom::Element* aElement,
     RelocateARIAOwnedIfNeeded(elm);
     ARIAActiveDescendantIDMaybeMoved(accessible);
     accessible->SendCache(CacheDomain::DOMNodeID, CacheUpdateType::Update);
+    QueueCacheUpdateForDependentRelations(accessible);
   }
 
   // The activedescendant universal property redirects accessible focus events
@@ -1166,6 +1190,8 @@ void DocAccessible::BindToDocument(LocalAccessible* aAccessible,
   if (mIPCDoc) {
     mInsertedAccessibles.EnsureInserted(aAccessible);
   }
+
+  QueueCacheUpdateForDependentRelations(aAccessible);
 }
 
 void DocAccessible::UnbindFromDocument(LocalAccessible* aAccessible) {

accessible/generic/DocAccessible.h

@@ -127,6 +127,14 @@ class DocAccessible : public HyperTextAccessibleWrap,
    */
   void QueueCacheUpdate(LocalAccessible* aAcc, uint64_t aNewDomain);
 
+  /**
+   * Walks the mDependentIDsHashes list for the given accessible and
+   * queues a CacheDomain::Relations cache update fore each related acc.
+   * We call this when we observe an ID mutation or when an acc is bound
+   * to its document.
+   */
+  void QueueCacheUpdateForDependentRelations(LocalAccessible* aAcc);
+
   /**
    * Return virtual cursor associated with the document.
    */

accessible/ipc/RemoteAccessibleBase.cpp

@@ -59,16 +59,10 @@ void RemoteAccessibleBase<Derived>::Shutdown() {
 
   if (StaticPrefs::accessibility_cache_enabled_AtStartup()) {
     // Remove this acc's relation map from the doc's map of
-    // reverse relations. We don't need to do additional processing
-    // of the corresponding forward relations, because this shutdown
-    // should trigger a cache update from the content process.
-    // Similarly, we don't need to remove the reverse rels created
-    // by this acc's forward rels because they'll be cleared during
-    // the next update's call to PreProcessRelations().
-    // In short, accs are responsible for managing their own
-    // reverse relation map, both in PreProcessRelations() and in
-    // Shutdown().
-    Unused << mDoc->mReverseRelations.Remove(ID());
+    // reverse relations. Prune forward relations associated with this
+    // acc's reverse relations. This also removes the acc's map of reverse
+    // rels from the mDoc's mReverseRelations.
+    PruneRelationsOnShutdown();
   }
 
   // XXX Ideally  this wouldn't be necessary, but it seems OuterDoc
@@ -1016,6 +1010,44 @@ void RemoteAccessibleBase<Derived>::PostProcessRelations(
   }
 }
 
+template <class Derived>
+void RemoteAccessibleBase<Derived>::PruneRelationsOnShutdown() {
+  auto reverseRels = mDoc->mReverseRelations.Lookup(ID());
+  if (!reverseRels) {
+    return;
+  }
+  for (auto const& data : kRelationTypeAtoms) {
+    // Fetch the list of targets for this reverse relation
+    auto reverseTargetList =
+        reverseRels->Lookup(static_cast<uint64_t>(data.mReverseType));
+    if (!reverseTargetList) {
+      continue;
+    }
+    for (uint64_t id : *reverseTargetList) {
+      // For each target, retrieve its corresponding forward relation target
+      // list
+      RemoteAccessible* affectedAcc = mDoc->GetAccessible(id);
+      if (!affectedAcc) {
+        // It's possible the affect acc also shut down, in which case
+        // we don't have anything to update.
+        continue;
+      }
+      if (auto forwardTargetList =
+              affectedAcc->mCachedFields
+                  ->GetMutableAttribute<nsTArray<uint64_t>>(data.mAtom)) {
+        forwardTargetList->RemoveElement(ID());
+        if (!forwardTargetList->Length()) {
+          // The ID we removed was the only thing in the list, so remove the
+          // entry from the cache entirely -- don't leave an empty array.
+          affectedAcc->mCachedFields->Remove(data.mAtom);
+        }
+      }
+    }
+  }
+  // Remove this ID from the document's map of reverse relations.
+  reverseRels.Remove();
+}
+
 template <class Derived>
 uint32_t RemoteAccessibleBase<Derived>::GetCachedTextLength() {
   MOZ_ASSERT(!HasChildren());

accessible/ipc/RemoteAccessibleBase.h

@@ -348,6 +348,15 @@ class RemoteAccessibleBase : public Accessible, public HyperTextAccessibleBase {
    */
   void PostProcessRelations(const nsTArray<bool>& aToUpdate);
 
+  /**
+   * This method is called during shutdown, before we clear our
+   * reverse rel map from the document's mReverseRelations cache.
+   * Here, we traverse our reverse relations, removing our ID from
+   * the corresponding forward relation's target list. This ensures
+   * the stored forward relations do not reference defunct accessibles.
+   */
+  void PruneRelationsOnShutdown();
+
   uint32_t GetCachedTextLength();
   Maybe<const nsTArray<int32_t>&> GetCachedTextLines();
   Maybe<nsTArray<nsRect>> GetCachedCharData();

accessible/tests/browser/e10s/browser_caching_relations_002.js

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
+requestLongerTimeout(2);
 
 /**
  * Test MEMBER_OF relation caching on HTML radio buttons
@@ -120,6 +121,43 @@ addAccessibleTask(
   }
 );
 
+/*
+ * Test mutation of LABEL relations via DOM ID reuse.
+ */
+addAccessibleTask(
+  `
+  <div id="label">before</div><input id="input" aria-labelledby="label">
+  `,
+  async function(browser, accDoc) {
+    let label = findAccessibleChildByID(accDoc, "label");
+    const input = findAccessibleChildByID(accDoc, "input");
+
+    await testCachedRelation(label, RELATION_LABEL_FOR, input);
+    await testCachedRelation(input, RELATION_LABELLED_BY, label);
+
+    const r = waitForEvent(EVENT_REORDER, accDoc);
+    await invokeContentTask(browser, [], () => {
+      content.document.getElementById("label").remove();
+      let l = content.document.createElement("div");
+      l.id = "label";
+      l.textContent = "after";
+      content.document.body.insertBefore(
+        l,
+        content.document.getElementById("input")
+      );
+    });
+    await r;
+    label = findAccessibleChildByID(accDoc, "label");
+    await testCachedRelation(label, RELATION_LABEL_FOR, input);
+    await testCachedRelation(input, RELATION_LABELLED_BY, label);
+  },
+  {
+    chrome: true,
+    iframe: true,
+    remoteIframe: true,
+  }
+);
+
 /*
  * Test LINKS_TO relation caching an anchor with multiple hashes
  */
@@ -142,3 +180,66 @@ addAccessibleTask(
     remoteIframe: !isWinNoCache,
   }
 );
+
+/*
+ * 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);
+    await untilCacheOk(() => {
+      try {
+        // We should get an acc ID back from this, but we don't have a way of
+        // verifying its correctness -- it should be the ID of the select.
+        return label.cache.getStringProperty("for");
+      } catch (e) {
+        ok(false, "Exception thrown while trying to read from the cache");
+        return false;
+      }
+    }, "Label for relation exists");
+
+    const r = waitForEvent(EVENT_REORDER, "l");
+    await invokeContentTask(browser, [], () => {
+      content.document.getElementById("s").remove();
+    });
+    await r;
+    await untilCacheOk(() => {
+      try {
+        label.cache.getStringProperty("for");
+      } catch (e) {
+        // This property should no longer exist in the cache, so we should
+        // get an exception if we try to fetch it.
+        return true;
+      }
+      return false;
+    }, "Label for relation exists");
+
+    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);
+  },
+  {
+    /**
+     * This functionality is broken in our LocalAcccessible implementation,
+     * so we avoid running this test in chrome or when the cache is off.
+     */
+    chrome: false,
+    iframe: isCacheEnabled,
+    remoteIframe: isCacheEnabled,
+    topLevel: isCacheEnabled,
+  }
+);