RESOLVED FIXED 199306
AX: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned
https://bugs.webkit.org/show_bug.cgi?id=199306
Summary AX: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm...
chris fleizach
Reported 2019-06-27 19:42:59 PDT
Summary: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned. <rdar://problem/52280799>
Attachments
patch (3.30 KB, patch)
2019-06-27 20:00 PDT, chris fleizach
ddkilzer: review+
ddkilzer: commit-queue-
patch (15.63 KB, patch)
2019-06-28 10:47 PDT, chris fleizach
no flags
Archive of layout-test-results from ews211 for win-future (14.02 MB, application/zip)
2019-06-28 14:47 PDT, EWS Watchlist
no flags
chris fleizach
Comment 1 2019-06-27 20:00:03 PDT
David Kilzer (:ddkilzer)
Comment 2 2019-06-28 09:55:34 PDT
Comment on attachment 373077 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=373077&action=review r=me > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h:105 > #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) > -extern RetainPtr<NSArray> convertToNSArray(const Vector<RefPtr<WebCore::AXIsolatedTreeNode>>&); > +extern NSArray *convertToNSArray(const Vector<RefPtr<WebCore::AXIsolatedTreeNode>>&); > #endif > -extern RetainPtr<NSArray> convertToNSArray(const WebCore::AccessibilityObject::AccessibilityChildrenVector&); > +extern NSArray *convertToNSArray(const WebCore::AccessibilityObject::AccessibilityChildrenVector&); I would really like to see statements like this cleaned up in WebAccessibilityObjectWrapperIOS.mm and WebAccessibilityObjectWrapperMac.mm: return (NSArray *)convertToNSArray(results); To remove the now-unnecessary cast: return convertToNSArray(results); I count 29 of them by searching for ")convertToNSArray" in the WebKit Xcode project. A search-and-replace of "(NSArray *)convertToNSArray(" to "convertToNSArray(" should do it.
chris fleizach
Comment 3 2019-06-28 10:36:18 PDT
(In reply to David Kilzer (:ddkilzer) from comment #2) > Comment on attachment 373077 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373077&action=review > > r=me > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h:105 > > #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) > > -extern RetainPtr<NSArray> convertToNSArray(const Vector<RefPtr<WebCore::AXIsolatedTreeNode>>&); > > +extern NSArray *convertToNSArray(const Vector<RefPtr<WebCore::AXIsolatedTreeNode>>&); > > #endif > > -extern RetainPtr<NSArray> convertToNSArray(const WebCore::AccessibilityObject::AccessibilityChildrenVector&); > > +extern NSArray *convertToNSArray(const WebCore::AccessibilityObject::AccessibilityChildrenVector&); > > I would really like to see statements like this cleaned up in > WebAccessibilityObjectWrapperIOS.mm and WebAccessibilityObjectWrapperMac.mm: > > return (NSArray *)convertToNSArray(results); > > To remove the now-unnecessary cast: > > return convertToNSArray(results); > > I count 29 of them by searching for ")convertToNSArray" in the WebKit Xcode > project. > > A search-and-replace of "(NSArray *)convertToNSArray(" to > "convertToNSArray(" should do it. will do
chris fleizach
Comment 4 2019-06-28 10:47:49 PDT
EWS Watchlist
Comment 5 2019-06-28 14:47:08 PDT
Comment on attachment 373130 [details] patch Attachment 373130 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12605747 New failing tests: storage/indexeddb/modern/get-keyrange.html
EWS Watchlist
Comment 6 2019-06-28 14:47:11 PDT
Created attachment 373152 [details] Archive of layout-test-results from ews211 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews211 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
WebKit Commit Bot
Comment 7 2019-06-28 15:21:42 PDT
Comment on attachment 373130 [details] patch Clearing flags on attachment: 373130 Committed r246941: <https://trac.webkit.org/changeset/246941>
WebKit Commit Bot
Comment 8 2019-06-28 15:21:44 PDT
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.