Merge r241547 - Web Inspector: don't include accessibility role in DO…

…M.Node object payloads https://bugs.webkit.org/show_bug.cgi?id=194623 <rdar://problem/36384037> Reviewed by Devin Rousso. Source/JavaScriptCore: Remove property of DOM.Node that is no longer being sent. * inspector/protocol/DOM.json: Source/WebCore: Accessibility properties are complicated to fetch at all the points where we want to build and push nodes immediately. Turning on AX often indirectly causes style recalc and layout. This is bad because we are often building nodes in the first place due to a DOM node tree update (i.e., NodeInserted). It turns out that DOM.getAccessibilityPropertiesForNode is called every time we display the computed role in the Elements Tab > Nodes Sidebar > Accessibility Section. So it is not necessary to collect this information in a problematic way when initially pushing the node, as it will be updated anyway. No new tests, no change in behavior. * inspector/agents/InspectorDOMAgent.cpp: (WebCore::InspectorDOMAgent::buildObjectForNode):
WebKit · Feb 18, 2019 · a1a9eb1 · a1a9eb1
1 parent 48eeadc
commit a1a9eb1
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 11 deletions.
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,15 @@
+2019-02-13  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: don't include accessibility role in DOM.Node object payloads
+        https://bugs.webkit.org/show_bug.cgi?id=194623
+        <rdar://problem/36384037>
+
+        Reviewed by Devin Rousso.
+
+        Remove property of DOM.Node that is no longer being sent.
+
+        * inspector/protocol/DOM.json:
+
 2019-02-13  Keith Miller  <keith_miller@apple.com> and Yusuke Suzuki  <ysuzuki@apple.com>
 
         We should only make rope strings when concatenating strings long enough.

diff --git a/Source/JavaScriptCore/inspector/protocol/DOM.json b/Source/JavaScriptCore/inspector/protocol/DOM.json
@@ -65,7 +65,6 @@
                 { "name": "shadowRoots", "type": "array", "optional": true, "items": { "$ref": "Node" }, "description": "Shadow root list for given element host." },
                 { "name": "templateContent", "$ref": "Node", "optional": true, "description": "Content document fragment for template elements" },
                 { "name": "pseudoElements", "type": "array", "items": { "$ref": "Node" }, "optional": true, "description": "Pseudo elements associated with this node." },
-                { "name": "role", "type": "string", "optional": true, "description": "Computed value for first recognized role token, default role per element, or overridden role." },
                 { "name": "contentSecurityPolicyHash", "type": "string", "optional": true, "description": "Computed SHA-256 Content Security Policy hash source for given element." }
             ]
         },

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2019-02-13  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: don't include accessibility role in DOM.Node object payloads
+        https://bugs.webkit.org/show_bug.cgi?id=194623
+        <rdar://problem/36384037>
+
+        Reviewed by Devin Rousso.
+
+        Accessibility properties are complicated to fetch at all the points where we want to build and push nodes immediately.
+        Turning on AX often indirectly causes style recalc and layout. This is bad because we are often building nodes in the
+        first place due to a DOM node tree update (i.e., NodeInserted).
+
+        It turns out that DOM.getAccessibilityPropertiesForNode is called every time we display
+        the computed role in the Elements Tab > Nodes Sidebar > Accessibility Section. So it is not
+        necessary to collect this information in a problematic way when initially pushing the node, as
+        it will be updated anyway.
+
+        No new tests, no change in behavior.
+
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::buildObjectForNode):
+
 2019-02-14  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][BFC][MarginCollapse] Replaced boxes don't collapse through their margins

diff --git a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
@@ -1594,15 +1594,6 @@ Ref<Inspector::Protocol::DOM::Node> InspectorDOMAgent::buildObjectForNode(Node*
         value->setShadowRootType(shadowRootType(shadowRoot.mode()));
     }
 
-    // Need to enable AX to get the computed role.
-    if (!WebCore::AXObjectCache::accessibilityEnabled())
-        WebCore::AXObjectCache::enableAccessibility();
-
-    if (AXObjectCache* axObjectCache = node->document().axObjectCache()) {
-        if (AccessibilityObject* axObject = axObjectCache->getOrCreate(node))
-            value->setRole(axObject->computedRoleString());
-    }
-
     return value;
 }
 

diff --git a/Source/WebInspectorUI/UserInterface/Models/DOMNode.js b/Source/WebInspectorUI/UserInterface/Models/DOMNode.js
@@ -48,7 +48,7 @@ WI.DOMNode = class DOMNode extends WI.Object
         this._nodeValue = payload.nodeValue;
         this._pseudoType = payload.pseudoType;
         this._shadowRootType = payload.shadowRootType;
-        this._computedRole = payload.role;
+        this._computedRole = null;
         this._contentSecurityPolicyHash = payload.contentSecurityPolicyHash;
 
         if (this._nodeType === Node.DOCUMENT_NODE)
@@ -586,6 +586,8 @@ WI.DOMNode = class DOMNode extends WI.Object
         function accessibilityPropertiesCallback(error, accessibilityProperties)
         {
             if (!error && callback && accessibilityProperties) {
+                this._computedRole = accessibilityProperties.role;
+
                 callback({
                     activeDescendantNodeId: accessibilityProperties.activeDescendantNodeId,
                     busy: accessibilityProperties.busy,