Bug 199306 - AX: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned
Summary: AX: Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-06-27 19:42 PDT by chris fleizach
Modified: 2019-06-28 15:21 PDT (History)
10 users (show)

See Also:


Attachments
patch (3.30 KB, patch)
2019-06-27 20:00 PDT, chris fleizach
ddkilzer: review+
ddkilzer: commit-queue-
Details | Formatted Diff | Diff
patch (15.63 KB, patch)
2019-06-28 10:47 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
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 Details

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2019-06-27 19:42:59 PDT
Summary:
Both convertToNSArray() functions in WebAccessibilityObjectWrapperBase.mm leak every NSMutableArray returned.

<rdar://problem/52280799>
Comment 1 chris fleizach 2019-06-27 20:00:03 PDT
Created attachment 373077 [details]
patch
Comment 2 David Kilzer (:ddkilzer) 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.
Comment 3 chris fleizach 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
Comment 4 chris fleizach 2019-06-28 10:47:49 PDT
Created attachment 373130 [details]
patch
Comment 5 EWS Watchlist 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
Comment 6 EWS Watchlist 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
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2019-06-28 15:21:44 PDT
All reviewed patches have been landed.  Closing bug.