Bug 221932

Summary: Reduce explicit usage of [objC autorelease] in WebKit
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, bburg, cfleizach, darin, dmazzoni, eric.carlson, ews-watchlist, ggaren, glenn, hi, japhet, jcraig, jdiggs, jer.noble, joepeck, keith_miller, mark.lam, mifenton, msaboff, philipj, saam, samuel_white, sergio, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221914
https://bugs.webkit.org/show_bug.cgi?id=222060
Bug Depends on: 221914    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Follow-up
ews-feeder: commit-queue-
Follow up none

Chris Dumez
Reported 2021-02-15 15:10:20 PST
Reduce explicit usage of [objC autorelease] in WebKit by using RetainPtr<>.
Attachments
Patch (141.12 KB, patch)
2021-02-15 16:28 PST, Chris Dumez
no flags
Patch (141.11 KB, patch)
2021-02-15 17:28 PST, Chris Dumez
no flags
Patch (140.26 KB, patch)
2021-02-16 11:48 PST, Chris Dumez
no flags
Patch (175.36 KB, patch)
2021-02-16 13:23 PST, Chris Dumez
no flags
Follow-up (27.52 KB, patch)
2021-02-18 14:25 PST, Chris Dumez
ews-feeder: commit-queue-
Follow up (27.62 KB, patch)
2021-02-18 14:44 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2021-02-15 16:28:37 PST
Chris Dumez
Comment 2 2021-02-15 17:28:33 PST
Chris Dumez
Comment 3 2021-02-16 11:48:23 PST
EWS Watchlist
Comment 4 2021-02-16 11:49:08 PST
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Geoffrey Garen
Comment 5 2021-02-16 13:20:39 PST
Comment on attachment 420513 [details] Patch r=me -- note the style bot comment about test results
Chris Dumez
Comment 6 2021-02-16 13:23:07 PST
EWS
Comment 7 2021-02-16 16:00:08 PST
Committed r272936: <https://commits.webkit.org/r272936> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420530 [details].
Radar WebKit Bug Importer
Comment 8 2021-02-16 16:02:44 PST
Darin Adler
Comment 9 2021-02-18 13:51:39 PST
Comment on attachment 420530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420530&action=review Looking back at this to see where we have opportunity to use a little less autorelease. > Source/JavaScriptCore/API/JSContext.mm:71 > + return [self initWithVirtualMachine:adoptNS([[JSVirtualMachine alloc] init]).autorelease()]; get() > Source/JavaScriptCore/API/JSWrapperMap.mm:638 > + return m_classMap.get()[(id)cls] = adoptNS([[JSObjCClassInfo alloc] initForClass:cls]).autorelease(); get() > Source/JavaScriptCore/inspector/scripts/codegen/objc_generator.py:408 > + return 'adoptNS([[%s alloc] initWithJSONObject:%s]).autorelease()' % (objc_class, var_name) This should be changed to use RetainPtr instead of autorelease. > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2550 > + object = adoptNS([[NSMutableAttributedString alloc] initWithString:[NSString stringWithCharacters:&attachmentChar length:1] attributes:@{ UIAccessibilityTokenAttachment : object }]).autorelease(); Seems like we could use RetainPtr instead of autorelease here. > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:131 > + @"WebResourceHandler": adoptNS([WebArchiveResourceWebResourceHandler new]).autorelease(), get() > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:923 > + resourceLoader.URLSession = (NSURLSession *)adoptNS([[WebCoreNSURLSession alloc] initWithResourceLoader:*mediaResourceLoader delegate:resourceLoader.URLSessionDataDelegate delegateQueue:resourceLoader.URLSessionDataDelegateQueue]).autorelease(); get() > Source/WebCore/platform/ios/WebAVPlayerController.mm:72 > + self.playerControllerProxy = adoptNS([allocAVPlayerControllerInstance() init]).autorelease(); get() > Source/WebCore/platform/ios/wak/WAKView.mm:241 > + [adoptNS([subviewReferences copy]).autorelease() makeObjectsPerformSelector:@selector(removeFromSuperview)]; get() > Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm:666 > + request = mutableRequest.autorelease(); I think we can use a local RetainPtr instead. > Source/WebCore/platform/network/mac/ResourceHandleMac.mm:176 > + nsRequest = copyRequestWithStorageSession(d->m_storageSession.get(), nsRequest).autorelease(); I think we can use a local RetainPtr instead. > Source/WebCore/platform/network/mac/ResourceHandleMac.mm:183 > + NSMutableDictionary *streamProperties = streamPropertiesFromClient ? adoptNS([streamPropertiesFromClient mutableCopy]).autorelease() : [NSMutableDictionary dictionary]; I think we can use RetainPtr instead. > Source/WebKit/UIProcess/API/Cocoa/NSAttributedString.mm:313 > + finish(retainPtr(attributedString).autorelease(), retainPtr(documentAttributes).autorelease(), nil); I don’t think we need retain/autorelease here at all. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1219 > + completionHandler(adoptNS([[WKFindResult alloc] _initWithMatchFound:NO]).autorelease()); get() > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1224 > + handler(adoptNS([[WKFindResult alloc] _initWithMatchFound:found]).autorelease()); get() > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1713 > + completionHandler(node.autorelease()); I don’t think we need retain/autorelease at all here. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2580 > + handler(retainPtr(attributedString.string.get()).autorelease(), retainPtr(attributedString.documentAttributes.get()).autorelease(), nil); I don’t think we need retain/autorelease here at all. > Source/WebKit/UIProcess/API/Cocoa/_WKAutomationSession.mm:44 > + return [self initWithConfiguration:adoptNS([[_WKAutomationSessionConfiguration alloc] init]).autorelease()]; get() > Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:191 > + capturedBlock(nil, retainPtr(wrapper(API::InspectorExtension::create(protectedExtensionID.get(), protectedSelf->_inspector->extensionController()))).autorelease()); I don’t think we need retain/autorelease at all here. > Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm:171 > + capturedBlock(nil, retainPtr(wrapper(API::InspectorExtension::create(protectedExtensionID.get(), protectedSelf->m_remoteInspectorProxy->extensionController()))).autorelease()); I don’t think we need retain/autorelease at all here. > Source/WebKit/UIProcess/API/Cocoa/_WKUserContentExtensionStore.mm:75 > + completionHandler(contentFilter.autorelease(), toUserContentRuleListStoreError(error)); get() > Source/WebKit/UIProcess/API/Cocoa/_WKUserContentExtensionStore.mm:83 > + completionHandler(contentFilter.autorelease(), toUserContentRuleListStoreError(error)); get() > Source/WebKit/UIProcess/Automation/ios/WebAutomationSessionIOS.mm:135 > + [eventsToBeSent addObject:adoptNS([[::WebEvent alloc] initWithKeyEventType:WebEventKeyDown timeStamp:CFAbsoluteTimeGetCurrent() characters:characters charactersIgnoringModifiers:unmodifiedCharacters modifiers:m_currentModifiers isRepeating:NO withFlags:inputFlags withInputManagerHint:nil keyCode:keyCode isTabKey:isTabKey]).autorelease()]; get() > Source/WebKit/UIProcess/Automation/ios/WebAutomationSessionIOS.mm:141 > + [eventsToBeSent addObject:adoptNS([[::WebEvent alloc] initWithKeyEventType:WebEventKeyUp timeStamp:CFAbsoluteTimeGetCurrent() characters:characters charactersIgnoringModifiers:unmodifiedCharacters modifiers:m_currentModifiers isRepeating:NO withFlags:inputFlags withInputManagerHint:nil keyCode:keyCode isTabKey:isTabKey]).autorelease()]; get() > Source/WebKit/UIProcess/Automation/ios/WebAutomationSessionIOS.mm:146 > + [eventsToBeSent addObject:adoptNS([[::WebEvent alloc] initWithKeyEventType:WebEventKeyDown timeStamp:CFAbsoluteTimeGetCurrent() characters:characters charactersIgnoringModifiers:unmodifiedCharacters modifiers:m_currentModifiers isRepeating:NO withFlags:inputFlags withInputManagerHint:nil keyCode:keyCode isTabKey:isTabKey]).autorelease()]; get() > Source/WebKit/UIProcess/Automation/ios/WebAutomationSessionIOS.mm:147 > + [eventsToBeSent addObject:adoptNS([[::WebEvent alloc] initWithKeyEventType:WebEventKeyUp timeStamp:CFAbsoluteTimeGetCurrent() characters:characters charactersIgnoringModifiers:unmodifiedCharacters modifiers:m_currentModifiers isRepeating:NO withFlags:inputFlags withInputManagerHint:nil keyCode:keyCode isTabKey:isTabKey]).autorelease()]; get() > Source/WebKit/UIProcess/Cocoa/WKContactPicker.mm:294 > + [contacts addObject:contact.autorelease()]; get()
Chris Dumez
Comment 10 2021-02-18 14:25:08 PST
Reopening to attach new patch.
Chris Dumez
Comment 11 2021-02-18 14:25:11 PST
Created attachment 420872 [details] Follow-up
Chris Dumez
Comment 12 2021-02-18 14:44:36 PST
Created attachment 420874 [details] Follow up
EWS
Comment 13 2021-02-18 22:12:39 PST
Committed r273128: <https://commits.webkit.org/r273128> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420874 [details].
Note You need to log in before you can comment on or make changes to this bug.