| Summary: | Reduce explicit usage of [objC autorelease] in WebKit even further | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
| Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | achristensen, darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, jiewen_tan, philipj, sam, sergio, 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=221932 | ||||||||||||
| Bug Depends on: | 222009 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Chris Dumez
2021-02-17 12:37:03 PST
Created attachment 420702 [details]
Patch
Created attachment 420705 [details]
Patch
Comment on attachment 420705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420705&action=review > Source/WebKit/UIProcess/ios/WKTextPlaceholder.mm:53 > + return @[ adoptNS([[WKTextSelectionRect alloc] initWithCGRect:_elementContext.boundingRect]).autorelease() ]; This would be a bit more efficient as: auto rect = adoptNS([[WKTextSelectionRect alloc] initWithCGRect:_elementContext.boundingRect]); return @[ rect.get() ]; I don't think the autorelease is needed in this case because the array should retain it. > Source/WebKit/UIProcess/ios/WebDataListSuggestionsDropdownIOS.mm:324 > - [_popover setPopoverController:[[[UIPopoverController alloc] initWithContentViewController:_suggestionsViewController.get()] autorelease]]; > + [_popover setPopoverController:adoptNS([[UIPopoverController alloc] initWithContentViewController:_suggestionsViewController.get()]).autorelease()]; Is this autorelease actually needed if we have a retainptr? I would guess setPopoverController: retains the parameter. > Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.mm:416 > - self.popoverController = [[[UIPopoverController alloc] initWithContentViewController:popoverViewController.get()] autorelease]; > + self.popoverController = adoptNS([[UIPopoverController alloc] initWithContentViewController:popoverViewController.get()]).autorelease(); Same question as above. Is this autorelease needed? > Source/WebKit/UIProcess/mac/WKTextInputWindowController.mm:142 > - *string = [[text copy] autorelease]; > + *string = adoptNS([text copy]).autorelease(); If this is not requirement to have a NSString ** out parameter here (e.g this method is part of an interface or protocol definition we are inheriting from / conforming to) I think we should probably just change things to return an Optional<RetainPtr<NSString>> or something like that. > Source/WebKitLegacy/mac/WebView/WebResource.mm:305 > - auto coreResource = ArchiveResource::create(SharedBuffer::create(copyData ? [[data copy] autorelease] : data), URL, MIMEType, textEncodingName, frameName, response); > + auto coreResource = ArchiveResource::create(SharedBuffer::create(copyData ? adoptNS([data copy]).autorelease() : data), URL, MIMEType, textEncodingName, frameName, response); I don't think this autorelease is needed. The RetainPtr's normal release should be fine. > Source/WebKitLegacy/mac/WebView/WebView.mm:3221 > - [schemesWithRepresentationsSet addObject:[[[URLScheme lowercaseString] copy] autorelease]]; > + [schemesWithRepresentationsSet addObject:adoptNS([[URLScheme lowercaseString] copy]).autorelease()]; I don't think this autorelease is necessary either. > Tools/DumpRenderTree/ios/DumpRenderTreeBrowserView.mm:117 > - self.scrollViewDelegate = [[[DumpRenderTreeWebScrollViewDelegate alloc] initWithScrollView:self] autorelease]; > + self.scrollViewDelegate = adoptNS([[DumpRenderTreeWebScrollViewDelegate alloc] initWithScrollView:self]).autorelease(); This autorelease is not necessary. Assigning to self.scrollViewDelegate will retain the thing, so just RetainPtr's release should be fine. You could also make scrollViewDelegate a RetainPtr. > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1430 > result = [NSMutableString stringWithFormat:@"\n--------\nFrame: '%@'\n--------\n", [frame name]]; > + else > + result = adoptNS([[NSMutableString alloc] init]); Could be a little more efficient and change the first branch to an adoptNS(NSMutableString alloc] initWithFormat...). > Tools/DumpRenderTree/mac/EventSendingController.mm:957 > + auto invocation = retainPtr([savedMouseEvents objectAtIndex:0]); You should be able to write this as retainPtr(savedMouseEvents[0]) I think. Comment on attachment 420705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420705&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:667 > + session.localContext = adoptNS([[WKDragSessionContext alloc] init]).autorelease(); This should just be get(), not autorelease(). There’s no need to autorelease something just to assign it to a strong property. > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8286 > + completionHandler(adoptNS([[WKTextPlaceholder alloc] initWithElementContext:placeholderToUse]).autorelease()); This should be get(), not autorelease(). There is no need to auto release something just to pass it to a function. >> Source/WebKit/UIProcess/ios/WKTextPlaceholder.mm:53 >> + return @[ adoptNS([[WKTextSelectionRect alloc] initWithCGRect:_elementContext.boundingRect]).autorelease() ]; > > This would be a bit more efficient as: > > auto rect = adoptNS([[WKTextSelectionRect alloc] initWithCGRect:_elementContext.boundingRect]); > return @[ rect.get() ]; > > I don't think the autorelease is needed in this case because the array should retain it. Yes, this should be get(), not autorelease(). There is no need to auto release something just to pass it to the @[] operator. It does not need to be refactored; can just change autorelease to get. > Source/WebKit/UIProcess/ios/WKWebEvent.mm:53 > + event = adoptNS([keyEvent _cloneEvent]).autorelease(); // UIKit uses a singleton for hardware keyboard events. This should be refactored to use RetainPtr instead of autorelease. >> Source/WebKit/UIProcess/ios/WebDataListSuggestionsDropdownIOS.mm:324 >> + [_popover setPopoverController:adoptNS([[UIPopoverController alloc] initWithContentViewController:_suggestionsViewController.get()]).autorelease()]; > > Is this autorelease actually needed if we have a retainptr? I would guess setPopoverController: retains the parameter. This should be get(), not autorelease(). There is no need to auto release something just to pass it to a setter method. >> Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.mm:416 >> + self.popoverController = adoptNS([[UIPopoverController alloc] initWithContentViewController:popoverViewController.get()]).autorelease(); > > Same question as above. Is this autorelease needed? This should just be get(), not autorelease(). There’s no need to autorelease something just to assign it to a strong property. > Source/WebKit/UIProcess/ios/forms/WKNumberPadViewController.mm:164 > + [self.delegate quickboard:self textEntered:adoptNS([[NSAttributedString alloc] initWithString:_inputText.get()]).autorelease()]; get() > Source/WebKit/UIProcess/ios/forms/WKTextInputListViewController.mm:100 > + [self.delegate quickboard:self textEntered:adoptNS([[NSAttributedString alloc] initWithString:text]).autorelease()]; get() > Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:552 > + _rootViewController.get().view = adoptNS([[UIView alloc] initWithFrame:_window.get().bounds]).autorelease(); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/UserContentController.mm:393 > + RetainPtr<_WKUserStyleSheet> styleSheet = adoptNS([[_WKUserStyleSheet alloc] initWithSource:styleSheetSource forWKWebView:nil forMainFrameOnly:NO includeMatchPatternStrings:@[] excludeMatchPatternStrings:@[] baseURL:adoptNS([[NSURL alloc] initWithString:@"http://CamelCase/"]).autorelease() level:_WKUserStyleUserLevel contentWorld:world.get()]); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/UserContentController.mm:989 > + replyHandler(adoptNS([[NSData alloc] init]).autorelease(), nil); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/UserContentWorld.mm:175 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:body.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:565 > + [richText appendAttributedString:adoptNS([[NSAttributedString alloc] initWithString:@"Lorem ipsum "]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:567 > + [richText appendAttributedString:adoptNS([[NSAttributedString alloc] initWithString:@" dolor sit amet."]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:383 > + [(id<WKURLSchemeTaskPrivate>)task _didPerformRedirection:adoptNS([[NSURLResponse alloc] init]).autorelease() newRequest:adoptNS([[NSURLRequest alloc] init]).autorelease()]; get(), get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:386 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] init]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:389 > + [task didReceiveData:adoptNS([[NSData alloc] init]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:577 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:strlen(bytes) textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:794 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:805 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:864 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:935 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:989 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:1067 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:mimeType expectedContentLength:response.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:1120 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:1179 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:response.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:1242 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:strlen(bytes) textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:1384 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:responseString.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:514 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:518 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:581 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:586 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:666 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:671 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:737 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:742 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEvaluateJavaScript.mm:747 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:0 textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebGLPolicy.mm:67 > + [urlSchemeTask didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:urlSchemeTask.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:132 > + configuration.get().websiteDataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]).autorelease(); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:468 > + [configuration setWebsiteDataStore:adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]).autorelease()]).autorelease()]; get(), get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:493 > + configuration.get().websiteDataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:dataStoreConfiguration.get()]).autorelease(); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:533 > + webViewConfiguration.get().websiteDataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]).autorelease(); get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:679 > + [webViewConfiguration setWebsiteDataStore:adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:712 > + [webViewConfiguration setWebsiteDataStore:adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:850 > + [webViewConfiguration setWebsiteDataStore:adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:704 > + [(id<WKURLSchemeTaskPrivate>)task _didPerformRedirection:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:nil expectedContentLength:0 textEncodingName:nil]).autorelease() newRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"test:///autoplay-check.html"]]]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:707 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:720 > + [task didReceiveResponse:response.autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:736 > + [configuration setURLSchemeHandler:adoptNS([[TestSchemeHandler alloc] initWithVideoData:WTFMove(videoData)]).autorelease() forURLScheme:@"test"]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1036 > + [task didReceiveResponse:adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:html.length textEncodingName:nil]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1622 > + [websitePolicies _setWebsiteDataStore:adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]).autorelease()]).autorelease()]; get(), get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1731 > + return adoptNS([[WKUserScript alloc] initWithSource:script injectionTime:WKUserScriptInjectionTimeAtDocumentStart forMainFrameOnly:YES]).autorelease(); Should return a RetainPtr instead of an autoreleased point. > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1747 > + [preferences _setUserContentController:adoptNS([WKUserContentController new]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1774 > + [preferences _setUserContentController:adoptNS([WKUserContentController new]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:1552 > + auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:adoptNS([[WKWebViewConfiguration alloc] init]).autorelease() processPoolConfiguration:processPoolConfiguration.get()]); get() > Tools/TestWebKitAPI/Tests/mac/MemoryPressureHandler.mm:35 > + adoptNS([[WebView alloc] initWithFrame:NSZeroRect frameName:nil groupName:nil]).autorelease(); Not sure why autorelease is wanted here. > Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:215 > + *errorOut = strongError.autorelease(); Not a big fan of these autoreleased out arguments. > Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:420 > + return [self initWithFrame:frame configuration:defaultConfiguration.autorelease()]; get() > Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:457 > + [configuration setProcessPool:adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration]).autorelease()]; get() > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:176 > + [items addObject:adoptNS([[UIDragItem alloc] initWithItemProvider:itemProvider]).autorelease()]; get() > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:347 > + return [self initWithWebView:adoptNS([[TestWKWebView alloc] initWithFrame:frame configuration:configuration]).autorelease()]; get() > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:349 > + return [self initWithWebView:adoptNS([[TestWKWebView alloc] initWithFrame:frame]).autorelease()]; get() > Tools/TestWebKitAPI/mac/DragAndDropSimulatorMac.mm:114 > + _webView = adoptNS([[DragAndDropTestWKWebView alloc] initWithFrame:frame configuration:configuration ?: adoptNS([[WKWebViewConfiguration alloc] init]).autorelease() simulator:self]); get() > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:144 > + m_websiteDataStore = (__bridge WKWebsiteDataStoreRef)adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfig.get()]).autorelease(); Can we use a RetainPtr for this? I don’t understand the lifetime guarantees here. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:198 > + PlatformWebView* view = new PlatformWebView(newConfiguration.autorelease(), options); get() > Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1146 > + @"eventInfo": adoptNS([eventInfo copy]).autorelease(), get() > Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1147 > + @"completionBlock": adoptNS([completionBlock copy]).autorelease() get() > Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1150 > + NSThread *eventDispatchThread = adoptNS([[NSThread alloc] initWithTarget:self selector:@selector(eventDispatchThreadEntry:) object:threadData]).autorelease(); Should use RetainPtr instead of autorelease here. Comment on attachment 420705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420705&action=review >> Source/WebKit/UIProcess/mac/WKTextInputWindowController.mm:142 >> + *string = adoptNS([text copy]).autorelease(); > > If this is not requirement to have a NSString ** out parameter here (e.g this method is part of an interface or protocol definition we are inheriting from / conforming to) I think we should probably just change things to return an Optional<RetainPtr<NSString>> or something like that. The function cannot return a Optional<RetainPtr<NSString>> because the BOOL being returned is not linked to the NSString being returned. While we have some flexibility with how _interpretKeyEvent is defined, changing it won't help much because it is called from interpretKeyEvent (without leading _) which has the same prototype and is exposed in the header. Created attachment 420764 [details]
Patch
Comment on attachment 420764 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420764&action=review > Source/WebKitLegacy/ios/WebView/WebPDFViewIOS.mm:298 > + _title = adoptNS([(NSString *)title.get() copy]); I don’t think we need a copy here. I would write: _title = (NSString *)title.get(); > Source/WebKitLegacy/ios/WebView/WebPDFViewIOS.mm:299 > + core([self _frame])->loader().client().dispatchDidReceiveTitle({ title.bridgingAutorelease(), TextDirection::LTR }); Should just use _title.get() here instead of title.bridgingAutorelease(). > Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm:1953 > + return adoptRef(new NetscapePluginWidget(pluginView.autorelease())); get() > Source/WebKitLegacy/mac/WebView/WebFrameView.mm:372 > + [scrollView setContentView:adoptNS([[WebClipView alloc] initWithFrame:[scrollView bounds]]).autorelease()]; get() > Source/WebKitLegacy/mac/WebView/WebFrameView.mm:1232 > + [scrollView setContentView:adoptNS([[WebClipView alloc] initWithFrame:[scrollView bounds]]).autorelease()]; get() > Tools/TestRunnerShared/cocoa/LayoutTestSpellChecker.mm:251 > + [resultsForWord addObject:adoptNS([[LayoutTestTextCheckingResult alloc] initWithType:nsTextCheckingType(typeValue.get()) range:NSMakeRange(fromValue, toValue - fromValue) replacement:(__bridge NSString *)replacementText.get() details:details.get()]).autorelease()]; get() > Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm:87 > + checkRecoveryAfterCrash(adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]).autorelease()]).autorelease()); get(), get() Created attachment 420779 [details]
Patch
Committed r273062: <https://commits.webkit.org/r273062> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420779 [details]. |