Bug 1959892 part 2: Block requesting of new cache domains while firing UIA events. r=morgan

Previously, UIA requested certain cache domains just by firing events, even if no client cared about those events.
This happened because unlike MSAA/IA2 win events, many UIA events include information about the thing that is changing; e.g. a name change event includes the new name.
In addition, live region events are based on other events (name change, text inserted, text removed) and thus need to query whether the element is a live region, which requires additional domains.
this caused our caching granularity tests to break, since those deliberately request only specific domains.
Firing core events caused us to fire UIA events (even if a UIA client didn't actually query us), which in turn caused us to request domains the test doesn't expect.
Less critical (but still important), this meant that these domains would always have been enabled as soon as an event is fired.
That's not great, since it somewhat defeats the point of caching granularity.
As mentioned in the previous patch, checking UiaClientsAreListening() is not sufficient to mitigate this in most cases.

The ideal solution is perhaps to avoid firing those events in the first place.
However, that would require checking for specific domains in the UIA code, which seems fragile if we change what domains are required for existing core methods, add new core methods, etc.
Also, we should probably really be avoiding these events at the core level, rather than requiring OS specific code to filter these out.

Instead, as a more straightforward solution, this patch adds a CacheDomainActivationBlocker RAII class to prevent requesting of new domains.
This is used specifically when firing UIA events.
The events we fire won't contain all the data they should, but that's okay because no client likely cares yet.
We still use explicit client queries (plus a list of known clients) as a signal to request new domains, after which the events will contain information from all the domains that are now active.
This approach could also easily be used on other platforms where needed.

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

accessible/base/CacheConstants.cpp

@@ -43,6 +43,11 @@ bool RequestDomainsIfInactive(uint64_t aRequiredCacheDomains) {
   const bool isMissingRequiredCacheDomain =
       (aRequiredCacheDomains & ~activeCacheDomains) != 0;
   if (isMissingRequiredCacheDomain) {
+    if (!accService->ShouldAllowNewCacheDomains()) {
+      // Return true to indicate that the domain is not active, but don't
+      // actually request it.
+      return true;
+    }
     aRequiredCacheDomains = GetCacheDomainSuperset(aRequiredCacheDomains);
 
     const uint64_t cacheDomains = aRequiredCacheDomains | activeCacheDomains;

accessible/base/CacheConstants.h

@@ -271,9 +271,11 @@ bool DomainsAreActive(uint64_t aRequiredCacheDomains);
 // false if all required domains are already active.
 bool RequestDomainsIfInactive(uint64_t aRequiredCacheDomains);
 
-#define ASSERT_DOMAINS_ACTIVE(aCacheDomains)  \
-  MOZ_ASSERT(DomainsAreActive(aCacheDomains), \
-             "Required domain(s) are not currently active.")
+#define ASSERT_DOMAINS_ACTIVE(aCacheDomains)                                 \
+  MOZ_ASSERT(                                                                \
+      (GetAccService() && !GetAccService()->ShouldAllowNewCacheDomains()) || \
+          DomainsAreActive(aCacheDomains),                                   \
+      "Required domain(s) are not currently active.")
 
 }  // namespace a11y
 }  // namespace mozilla

accessible/base/nsAccessibilityService.cpp

@@ -2096,5 +2096,34 @@ void PrefChanged(const char* aPref, void* aClosure) {
   }
 }
 
+uint32_t CacheDomainActivationBlocker::sEntryCount = 0;
+
+CacheDomainActivationBlocker::CacheDomainActivationBlocker() {
+  AssertIsOnMainThread();
+  if (sEntryCount++ != 0) {
+    // We're re-entering. This can happen if an earlier event (even in a
+    // different document) ends up calling an XUL method, since that can run
+    // script which can cause other events to fire. Only the outermost usage
+    // should change the flag.
+    return;
+  }
+  if (nsAccessibilityService* service = GetAccService()) {
+    MOZ_ASSERT(service->mShouldAllowNewCacheDomains);
+    service->mShouldAllowNewCacheDomains = false;
+  }
+}
+
+CacheDomainActivationBlocker::~CacheDomainActivationBlocker() {
+  AssertIsOnMainThread();
+  if (--sEntryCount != 0) {
+    // Only the outermost usage should change the flag.
+    return;
+  }
+  if (nsAccessibilityService* service = GetAccService()) {
+    MOZ_ASSERT(!service->mShouldAllowNewCacheDomains);
+    service->mShouldAllowNewCacheDomains = true;
+  }
+}
+
 }  // namespace a11y
 }  // namespace mozilla

accessible/base/nsAccessibilityService.h

@@ -92,6 +92,23 @@ void PrefChanged(const char* aPref, void* aClosure);
  */
 EPlatformDisabledState ReadPlatformDisabledState();
 
+/**
+ * RAII class to prevent new cache domains from being requested. This is
+ * necessary in some cases when code for an OS accessibility API requires
+ * information in order to fire an event. We don't necessarily know that a
+ * client is even interested in that event, so requesting data that the client
+ * may never query doesn't make sense.
+ */
+class MOZ_RAII CacheDomainActivationBlocker {
+ public:
+  CacheDomainActivationBlocker();
+  ~CacheDomainActivationBlocker();
+
+ private:
+  // Used to manage re-entry.
+  static uint32_t sEntryCount;
+};
+
 }  // namespace a11y
 }  // namespace mozilla
 
@@ -346,6 +363,7 @@ class nsAccessibilityService final : public mozilla::a11y::DocManager,
   };
 
   static uint64_t GetActiveCacheDomains() { return gCacheDomains; }
+  bool ShouldAllowNewCacheDomains() { return mShouldAllowNewCacheDomains; }
 
 #if defined(ANDROID)
   static mozilla::Monitor& GetAndroidMonitor();
@@ -415,6 +433,10 @@ class nsAccessibilityService final : public mozilla::a11y::DocManager,
    * Contains the currently active cache domains.
    */
   static uint64_t gCacheDomains;
+  // True if requesting new cache domains should be allowed, false if this
+  // should be disallowed. This should only be changed by
+  // CacheDomainActivationBlocker.
+  bool mShouldAllowNewCacheDomains = true;
 
   // Can be weak because all atoms are known static
   using MarkupMap = nsTHashMap<nsAtom*, const mozilla::a11y::MarkupMapInfo*>;
@@ -450,6 +472,7 @@ class nsAccessibilityService final : public mozilla::a11y::DocManager,
   friend mozilla::a11y::xpcAccessibleApplication*
   mozilla::a11y::XPCApplicationAcc();
   friend class xpcAccessibilityService;
+  friend class mozilla::a11y::CacheDomainActivationBlocker;
 };
 
 /**

accessible/windows/uia/uiaRawElmProvider.cpp

@@ -144,6 +144,11 @@ void uiaRawElmProvider::RaiseUiaEventForGeckoEvent(Accessible* aAcc,
   if (!uia) {
     return;
   }
+  // Some UIA events include or depend on data that might not be cached yet. We
+  // shouldn't request additional cache domains in this case because a client
+  // might not even care about these events. Instead, we use explicit client
+  // queries as a signal to request domains.
+  CacheDomainActivationBlocker cacheBlocker;
   PROPERTYID property = 0;
   _variant_t newVal;
   bool gotNewVal = false;