Bug 199306 – AX: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

Summary:
Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned.

<rdar://problem/52280799>

[chris fleizach]

Created attachment 373077 [details]
patch

[David Kilzer (:ddkilzer)]

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]

(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]

Created attachment 373130 [details]
patch

[EWS Watchlist]

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]

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 on attachment 373130 [details]
patch

Clearing flags on attachment: 373130

Committed r246941: <https://trac.webkit.org/changeset/246941>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.