Bug 1793941: Don't fire EVENT_TABLE_STYLING_CHANGED when the cache is enabled r=Jamie

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

accessible/base/nsAccessibilityService.cpp

@@ -573,8 +573,13 @@ void nsAccessibilityService::TableLayoutGuessMaybeChanged(
   if (DocAccessible* document = GetDocAccessible(aPresShell)) {
     if (LocalAccessible* acc = document->GetAccessible(aContent)) {
       if (LocalAccessible* table = nsAccUtils::TableFor(acc)) {
-        document->FireDelayedEvent(
-            nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED, table);
+        if (!StaticPrefs::accessibility_cache_enabled_AtStartup()) {
+          // Only fire this event when the cache is off -- we don't
+          // need to maintain the mac table cache otherwise, since
+          // we'll use the core cache instead.
+          document->FireDelayedEvent(
+              nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED, table);
+        }
         document->QueueCacheUpdate(table, CacheDomain::Table);
       }
     }

accessible/generic/DocAccessible.cpp

@@ -1376,12 +1376,16 @@ bool DocAccessible::PruneOrInsertSubtree(nsIContent* aRoot) {
 
     // If the accessible is a table, or table part, its layout table
     // status may have changed. We need to invalidate the associated
-    // table cache, which listens for the following event.
+    // mac table cache, which listens for the following event. We don't
+    // use this cache when the core cache is enabled, so to minimise event
+    // traffic only fire this event when that cache is off.
     if (acc->IsTable() || acc->IsTableRow() || acc->IsTableCell()) {
       LocalAccessible* table = nsAccUtils::TableFor(acc);
       if (table && table->IsTable()) {
-        FireDelayedEvent(nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED,
-                         table);
+        if (!StaticPrefs::accessibility_cache_enabled_AtStartup()) {
+          FireDelayedEvent(nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED,
+                           table);
+        }
         QueueCacheUpdate(table, CacheDomain::Table);
       }
     }

accessible/mac/mozTableAccessible.mm

@@ -133,11 +133,13 @@ enum CachedBool { eCachedBoolMiss, eCachedTrue, eCachedFalse };
 }
 
 - (void)handleAccessibleEvent:(uint32_t)eventType {
-  if (![self isKindOfClass:[mozTableAccessible class]]) {
+  if (![self isKindOfClass:[mozTableAccessible class]] &&
+      !StaticPrefs::accessibility_cache_enabled_AtStartup()) {
     // If we are not a table, we are a cell or a row.
     // Check to see if the event we're handling should
     // invalidate the mIsLayoutTable cache on our parent
-    // table.
+    // table. Only do this when the core cache is off, because
+    // we don't use the platform cache when its on.
     if (eventType == nsIAccessibleEvent::EVENT_REORDER ||
         eventType == nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED) {
       // Invalidate the cache on our parent table
@@ -180,11 +182,16 @@ enum CachedBool { eCachedBoolMiss, eCachedTrue, eCachedFalse };
 @implementation mozTableAccessible
 
 - (void)invalidateLayoutTableCache {
+  MOZ_ASSERT(!StaticPrefs::accessibility_cache_enabled_AtStartup(),
+             "If the core cache is enabled we shouldn't be maintaining the "
+             "platform table cache!");
   mIsLayoutTable = eCachedBoolMiss;
 }
 
 - (BOOL)isLayoutTablePart {
-  if (mIsLayoutTable != eCachedBoolMiss) {
+  if (mIsLayoutTable != eCachedBoolMiss &&
+      !StaticPrefs::accessibility_cache_enabled_AtStartup()) {
+    // Only use the platform cache if the core cache is not on
     return mIsLayoutTable == eCachedTrue;
   }
 
@@ -197,7 +204,7 @@ enum CachedBool { eCachedBoolMiss, eCachedTrue, eCachedFalse };
   }
 
   bool tableGuess;
-  // For LocalAccessible and cached RemoteAccessible, We could use
+  // For LocalAccessible and cached RemoteAccessible, we could use
   // AsTableBase()->IsProbablyLayoutTable(). However, if the cache is enabled,
   // that would build the table cache, which is pointless for layout tables on
   // Mac because layout tables are AXGroups and do not expose table properties
@@ -217,7 +224,9 @@ enum CachedBool { eCachedBoolMiss, eCachedTrue, eCachedFalse };
   if (eventType == nsIAccessibleEvent::EVENT_REORDER ||
       eventType == nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED ||
       eventType == nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED) {
-    [self invalidateLayoutTableCache];
+    if (!StaticPrefs::accessibility_cache_enabled_AtStartup()) {
+      [self invalidateLayoutTableCache];
+    }
     [self invalidateColumns];
   }
 

accessible/tests/browser/e10s/browser_caching_table.js

@@ -283,8 +283,20 @@ addAccessibleTask(
     await invokeContentTask(browser, [], () => {
       content.document.getElementById("cell").style.border = "1px solid black";
     });
-    await styleChanged;
-    testAbsentAttrs(layout, { "layout-guess": "true" });
+    if (!isCacheEnabled) {
+      // this event doesn't get fired when the cache is on, so we can't await it
+      await styleChanged;
+    }
+    await untilCacheOk(() => {
+      // manually verify the attribute doesn't exist, since `testAbsentAttrs`
+      // has internal calls to ok() which fail if the cache hasn't yet updated
+      for (let prop of layout.attributes.enumerate()) {
+        if (prop.key == "layout-guess") {
+          return false;
+        }
+      }
+      return true;
+    }, "Table is a data table");
   },
   {
     chrome: true,

accessible/tests/browser/mac/browser_table.js

@@ -314,7 +314,12 @@ async function testIsLayout(table, elem, event, change, isLayout) {
     event == EVENT_TABLE_STYLING_CHANGED ? "table" : elem
   );
   await change();
-  await toWait;
+  if (event != EVENT_TABLE_STYLING_CHANGED || !isCacheEnabled) {
+    // We can't wait for this event when the cache is on because
+    // we don't fire it. Instead we rely on the `untilCacheIs` check
+    // below.
+    await toWait;
+  }
   let intendedRole = isLayout ? "AXGroup" : "AXTable";
   await untilCacheIs(
     () => table.getAttributeValue("AXRole"),

accessible/tests/mochitest/events/test_stylechange.html

@@ -25,18 +25,25 @@
     }
 
     async function doTests() {
-      info("Set new row background");
-      await testGotStyleChange("rowOne", "background-color", "green");
+      if (Services.prefs.getBoolPref(
+        "accessibility.cache.enabled",
+        false
+      )) {
+        ok(true, "This event isn't fired when the cache is on " +
+           "but we can't leave this test empty, or the harness complains");
+      } else {
+        info("Set new row background");
+        await testGotStyleChange("rowOne", "background-color", "green");
 
-      info("Remove row background");
-      await testGotStyleChange("rowOne", "background-color", null);
+        info("Remove row background");
+        await testGotStyleChange("rowOne", "background-color", null);
 
-      info("Change cell border");
-      await testGotStyleChange("cellOne", "border", "5px solid green");
-
-      info("Remove cell border");
-      await testGotStyleChange("cellOne", "border", null);
+        info("Change cell border");
+        await testGotStyleChange("cellOne", "border", "5px solid green");
 
+        info("Remove cell border");
+        await testGotStyleChange("cellOne", "border", null);
+      }
       SimpleTest.finish();
     }