RESOLVED FIXED 204918
Implementation of additional attribute caching in the IsolatedTree.
https://bugs.webkit.org/show_bug.cgi?id=204918
Summary Implementation of additional attribute caching in the IsolatedTree.
Andres Gonzalez
Reported 2019-12-05 14:21:19 PST
Implementation of additional attribute caching in the IsolatedTree.
Attachments
Patch (198.83 KB, patch)
2019-12-05 14:56 PST, Andres Gonzalez
no flags
Patch (200.60 KB, patch)
2019-12-05 18:29 PST, Andres Gonzalez
no flags
Patch (201.21 KB, patch)
2019-12-06 06:42 PST, Andres Gonzalez
no flags
Andres Gonzalez
Comment 1 2019-12-05 14:56:59 PST
chris fleizach
Comment 2 2019-12-05 16:39:42 PST
Looks like some real GTK failures ../../Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceAction.cpp:86:119: error: ‘class WTF::String’ has no member named ‘string’; did you mean ‘String’? return webkitAccessibleCacheAndReturnAtkProperty(accessible, AtkCachedActionKeyBinding, core(action)->accessKey().string().utf8()); ^~~~~~ String ../../Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp: In function ‘const gchar* webkitAccessibleHyperlinkActionGetKeybinding(AtkAction*, gint)’: ../../Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp:101:74: error: ‘class WTF::String’ has no member named ‘string’; did you mean ‘String’? accessibleHyperlink->priv->actionKeyBinding = coreObject.accessKey().string().utf8(); ^~~~~~ String
Radar WebKit Bug Importer
Comment 3 2019-12-05 16:40:34 PST
Andres Gonzalez
Comment 4 2019-12-05 18:29:20 PST
Andres Gonzalez
Comment 5 2019-12-05 18:34:45 PST
Fix for GTK build: AXCoreObject::accessKey() was returning an AtomString and now returns a String, so no need to call AtomString::string().
chris fleizach
Comment 6 2019-12-06 00:41:24 PST
Comment on attachment 384985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384985&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.h:641 > + bool isMediaControlLabel() const override { return boolAttributeValue(AXPropertyName::IsMediaControlLabel); } is isMediaControlLabel() this one in the wrong group? or is it implemented unnecessarily?
Andres Gonzalez
Comment 7 2019-12-06 06:42:05 PST
Andres Gonzalez
Comment 8 2019-12-06 06:48:01 PST
(In reply to chris fleizach from comment #6) > Comment on attachment 384985 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384985&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.h:641 > > + bool isMediaControlLabel() const override { return boolAttributeValue(AXPropertyName::IsMediaControlLabel); } > > is isMediaControlLabel() this one in the wrong group? or is it implemented > unnecessarily? I removed isMediaControlLabel from AXCoreObject since it is not used outside the AXObject class hierarchy. The same applies to all methods that need no implementation in AXIsolatedObject. But we can do this progressively as we go through all the methods. The goal would be that AXCoreObject contains only the methods that are used by client code, and it is a subset of AXObject and AXIsolatedObject.
WebKit Commit Bot
Comment 9 2019-12-07 12:34:43 PST
Comment on attachment 385010 [details] Patch Clearing flags on attachment: 385010 Committed r253261: <https://trac.webkit.org/changeset/253261>
WebKit Commit Bot
Comment 10 2019-12-07 12:34:45 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.