Title: [174345] trunk/Source/WebCore
Revision
174345
Author
[email protected]
Date
2014-10-06 09:39:54 -0700 (Mon, 06 Oct 2014)

Log Message

AX: Performance: Certain Web site causes Safari to hang completely while entering form data
https://bugs.webkit.org/show_bug.cgi?id=137420

Reviewed by Mario Sanchez Prada.

If a website has multiple nested tables that are not "accessibility" tables, the performance of accessibility slows to a crawl because:
  1) We are re-computing accessibilityIsIgnored many times.
       As a solution, we can enable the isIgnoredCache when updating children.
  2) When asking if an object isTableCell, we'd go up the parent chain, asking each of those parents the same question, which exploded into calling this many times.
      As a solution, I've changed our determination of isTableCell to instead check if the parent is an accessibiltyTable which should be much faster.

No new functionality. Existing tests cover changes.

* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::updateChildrenIfNecessary):
(WebCore::AccessibilityObject::accessibilityIsIgnored):
* accessibility/AccessibilityTableCell.cpp:
(WebCore::AccessibilityTableCell::parentTable):
(WebCore::AccessibilityTableCell::isTableCell):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (174344 => 174345)


--- trunk/Source/WebCore/ChangeLog	2014-10-06 16:25:51 UTC (rev 174344)
+++ trunk/Source/WebCore/ChangeLog	2014-10-06 16:39:54 UTC (rev 174345)
@@ -1,3 +1,25 @@
+2014-10-06  Chris Fleizach  <[email protected]>
+
+        AX: Performance: Certain Web site causes Safari to hang completely while entering form data
+        https://bugs.webkit.org/show_bug.cgi?id=137420
+
+        Reviewed by Mario Sanchez Prada.
+
+        If a website has multiple nested tables that are not "accessibility" tables, the performance of accessibility slows to a crawl because:
+          1) We are re-computing accessibilityIsIgnored many times.
+               As a solution, we can enable the isIgnoredCache when updating children.
+          2) When asking if an object isTableCell, we'd go up the parent chain, asking each of those parents the same question, which exploded into calling this many times.
+              As a solution, I've changed our determination of isTableCell to instead check if the parent is an accessibiltyTable which should be much faster.
+
+        No new functionality. Existing tests cover changes.
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::updateChildrenIfNecessary):
+        (WebCore::AccessibilityObject::accessibilityIsIgnored):
+        * accessibility/AccessibilityTableCell.cpp:
+        (WebCore::AccessibilityTableCell::parentTable):
+        (WebCore::AccessibilityTableCell::isTableCell):
+
 2014-10-06  Csaba Osztrogonác  <[email protected]>
 
         Unreviewed, touch testing/Internals.idl to try to fix Windows EWS after r174315.

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (174344 => 174345)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2014-10-06 16:25:51 UTC (rev 174344)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2014-10-06 16:39:54 UTC (rev 174345)
@@ -1583,8 +1583,11 @@
 
 void AccessibilityObject::updateChildrenIfNecessary()
 {
-    if (!hasChildren())
-        addChildren();    
+    if (!hasChildren()) {
+        // Enable the cache in case we end up adding a lot of children, we don't want to recompute axIsIgnored each time.
+        AXAttributeCacheEnabler enableCache(axObjectCache());
+        addChildren();
+    }
 }
     
 void AccessibilityObject::clearChildren()
@@ -2494,7 +2497,8 @@
 bool AccessibilityObject::accessibilityIsIgnored() const
 {
     AXComputedObjectAttributeCache* attributeCache = nullptr;
-    if (AXObjectCache* cache = axObjectCache())
+    AXObjectCache* cache = axObjectCache();
+    if (cache)
         attributeCache = cache->computedObjectAttributeCache();
     
     if (attributeCache) {
@@ -2511,7 +2515,8 @@
 
     bool result = computeAccessibilityIsIgnored();
 
-    if (attributeCache)
+    // In case computing axIsIgnored disables attribute caching, we should refetch the object to see if it exists.
+    if (cache && (attributeCache = cache->computedObjectAttributeCache()))
         attributeCache->setIgnored(axObjectID(), result ? IgnoreObject : IncludeObject);
 
     return result;

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableCell.cpp (174344 => 174345)


--- trunk/Source/WebCore/accessibility/AccessibilityTableCell.cpp	2014-10-06 16:25:51 UTC (rev 174344)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableCell.cpp	2014-10-06 16:39:54 UTC (rev 174345)
@@ -86,17 +86,20 @@
     // by _javascript_, and creating a table element may try to access the render tree while in a bad state.
     // By using only get() implies that the AXTable must be created before AXTableCells. This should
     // always be the case when AT clients access a table.
-    // https://bugs.webkit.org/show_bug.cgi?id=42652    
-    return toAccessibilityTable(axObjectCache()->get(toRenderTableCell(m_renderer)->table()));
+    // https://bugs.webkit.org/show_bug.cgi?id=42652
+    AccessibilityObject* parentTable = axObjectCache()->get(toRenderTableCell(m_renderer)->table());
+    if (!parentTable || !parentTable->isTable())
+        return nullptr;
+    return toAccessibilityTable(parentTable);
 }
     
 bool AccessibilityTableCell::isTableCell() const
 {
-    AccessibilityObject* parent = parentObjectUnignored();
-    if (!parent || !parent->isTableRow())
-        return false;
-    
-    return true;
+    // If the parent table is an accessibility table, then we are a table cell.
+    // This used to check if the unignoredParent was a row, but that exploded performance if
+    // this was in nested tables. This check should be just as good.
+    AccessibilityObject* parentTable = this->parentTable();
+    return parentTable && parentTable->isAccessibilityTable();
 }
     
 AccessibilityRole AccessibilityTableCell::determineAccessibilityRole()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to