Bug 115657

Summary:

Use adoptCF and adoptNS in more places, test code and code not compiled on Mac

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

WebKit Misc.

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, commit-queue, eric.carlson, esprehn+autocc, glenn, gustavo, jer.noble, menard, mrobinson, pnormand, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	sam: review+, commit-queue: commit-queue-

Darin Adler

Reported 2013-05-06 09:57:33 PDT

Use adoptCF and adoptNS in more places, test code and code not compiled on Mac

Attachments
Patch (214.21 KB, patch) 2013-05-06 10:01 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (213.91 KB, patch) 2013-05-07 09:10 PDT, Darin Adler	sam: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2013-05-06 10:01:08 PDT

Created attachment 200716 [details] Patch

WebKit Commit Bot

Comment 2 2013-05-06 10:03:41 PDT

Attachment 200716 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/cf/win/CertificateCFWin.cpp', u'Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp', u'Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateLegacyAVFObjC.mm', u'Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp', u'Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp', u'Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp', u'Source/WebCore/platform/graphics/ca/win/WKCACFViewLayerTreeHost.cpp', u'Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindowMac.mm', u'Source/WebCore/platform/graphics/win/FontCustomPlatformData.cpp', u'Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp', u'Source/WebCore/platform/graphics/win/GraphicsContextCGWin.cpp', u'Source/WebCore/platform/graphics/win/ImageCGWin.cpp', u'Source/WebCore/platform/graphics/win/MediaPlayerPrivateFullscreenWindow.cpp', u'Source/WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp', u'Source/WebCore/platform/graphics/win/WKCAImageQueue.cpp', u'Source/WebCore/platform/image-decoders/ImageDecoder.h', u'Source/WebCore/platform/network/cf/AuthenticationCF.cpp', u'Source/WebCore/platform/network/cf/CookieJarCFNet.cpp', u'Source/WebCore/platform/network/curl/ResourceHandleManager.cpp', u'Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp', u'Source/WebCore/platform/win/LocalizedStringsWin.cpp', u'Source/WebCore/platform/win/SearchPopupMenuWin.cpp', u'Source/WebCore/plugins/mac/PluginPackageMac.cpp', u'Source/WebCore/plugins/mac/PluginViewMac.mm', u'Source/WebCore/rendering/RenderThemeSafari.cpp', u'Source/WebKit/win/CFDictionaryPropertyBag.cpp', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebCache.cpp', u'Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp', u'Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp', u'Source/WebKit/win/WebDatabaseManager.cpp', u'Source/WebKit/win/WebDownloadCFNet.cpp', u'Source/WebKit/win/WebError.cpp', u'Source/WebKit/win/WebHistory.cpp', u'Source/WebKit/win/WebIconDatabase.cpp', u'Source/WebKit/win/WebLocalizableStrings.cpp', u'Source/WebKit/win/WebMutableURLRequest.cpp', u'Source/WebKit/win/WebPreferences.cpp', u'Source/WebKit/win/WebView.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Shared/Downloads/cfnet/DownloadCFNet.cpp', u'Tools/ChangeLog', u'Tools/DumpRenderTree/cf/WebArchiveDumpSupport.cpp', u'Tools/DumpRenderTree/cg/ImageDiffCG.cpp', u'Tools/DumpRenderTree/cg/PixelDumpSupportCG.cpp', u'Tools/DumpRenderTree/cg/PixelDumpSupportCG.h', u'Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm', u'Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm', u'Tools/DumpRenderTree/mac/TestRunnerMac.mm', u'Tools/DumpRenderTree/mac/WebArchiveDumpSupportMac.mm', u'Tools/DumpRenderTree/mac/WorkQueueItemMac.mm', u'Tools/DumpRenderTree/win/DumpRenderTree.cpp', u'Tools/DumpRenderTree/win/ImageDiffCairo.cpp', u'Tools/DumpRenderTree/win/PixelDumpSupportWin.cpp', u'Tools/DumpRenderTree/win/TestRunnerWin.cpp', u'Tools/TestWebKitAPI/Tests/CustomProtocolsSyncXHRTest.mm', u'Tools/TestWebKitAPI/Tests/TestWebKitAPI/mac/InstanceMethodSwizzler.mm', u'Tools/TestWebKitAPI/Tests/WTF/cf/RetainPtrHashing.cpp', u'Tools/TestWebKitAPI/Tests/WebKit2/FindMatches.mm', u'Tools/TestWebKitAPI/Tests/WebKit2/WebArchive.cpp', u'Tools/TestWebKitAPI/Tests/WebKit2/mac/GetBackingScaleFactor.mm', u'Tools/TestWebKitAPI/Tests/WebKit2/win/DoNotCopyANullCFURLResponse.cpp', u'Tools/TestWebKitAPI/Tests/mac/AcceptsFirstMouse.mm', u'Tools/TestWebKitAPI/Tests/mac/AttributedString.mm', u'Tools/TestWebKitAPI/Tests/mac/BackForwardList.mm', u'Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/ContextMenuCanCopyURL.mm', u'Tools/TestWebKitAPI/Tests/mac/DOMHTMLTableCellCellAbove.mm', u'Tools/TestWebKitAPI/Tests/mac/DOMRangeOfString.mm', u'Tools/TestWebKitAPI/Tests/mac/DeviceScaleFactorInDashboardRegions.mm', u'Tools/TestWebKitAPI/Tests/mac/DeviceScaleFactorOnBack.mm', u'Tools/TestWebKitAPI/Tests/mac/DynamicDeviceScaleFactor.mm', u'Tools/TestWebKitAPI/Tests/mac/HTMLCollectionNamedItem.mm', u'Tools/TestWebKitAPI/Tests/mac/HTMLFormCollectionNamedItem.mm', u'Tools/TestWebKitAPI/Tests/mac/InspectorBar.mm', u'Tools/TestWebKitAPI/Tests/mac/MemoryCacheDisableWithinResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/MemoryCachePruneWithinResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/PageVisibilityStateWithWindowChanges.mm', u'Tools/TestWebKitAPI/Tests/mac/RenderedImageFromDOMRange.mm', u'Tools/TestWebKitAPI/Tests/mac/SetAndUpdateCacheModel.mm', u'Tools/TestWebKitAPI/Tests/mac/SetDocumentURI.mm', u'Tools/TestWebKitAPI/Tests/mac/SimplifyMarkup.mm', u'Tools/TestWebKitAPI/Tests/mac/StringByEvaluatingJavaScriptFromString.mm', u'Tools/TestWebKitAPI/Tests/mac/WillSendSubmitEvent.mm', u'Tools/TestWebKitAPI/Tests/mac/WindowlessWebViewWithMedia.mm', u'Tools/TestWebKitAPI/mac/PlatformUtilitiesMac.mm', u'Tools/TestWebKitAPI/mac/WebKitAgnosticTest.mm', u'Tools/WebKitTestRunner/InjectedBundle/mac/InjectedBundlePageMac.mm', u'Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm', u'Tools/WebKitTestRunner/cg/TestInvocationCG.cpp', u'Tools/WebKitTestRunner/mac/EventSenderProxy.mm', u'Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm', u'Tools/WebKitTestRunner/win/TestControllerWin.cpp']" exit_code: 1 Source/WebCore/platform/win/LocalizedStringsWin.cpp:81: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:763: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:768: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:772: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:977: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp:91: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp:98: Use 0 instead of NULL. [readability/null] [5] Total errors found: 7 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 3 2013-05-06 10:33:54 PDT

Comment on attachment 200716 [details] Patch Attachment 200716 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/378188

Darin Adler

Comment 4 2013-05-07 09:10:23 PDT

Created attachment 200910 [details] Patch

WebKit Commit Bot

Comment 5 2013-05-07 09:13:04 PDT

Attachment 200910 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/cf/win/CertificateCFWin.cpp', u'Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp', u'Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateLegacyAVFObjC.mm', u'Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp', u'Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp', u'Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp', u'Source/WebCore/platform/graphics/ca/win/WKCACFViewLayerTreeHost.cpp', u'Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindowMac.mm', u'Source/WebCore/platform/graphics/win/FontCustomPlatformData.cpp', u'Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp', u'Source/WebCore/platform/graphics/win/GraphicsContextCGWin.cpp', u'Source/WebCore/platform/graphics/win/ImageCGWin.cpp', u'Source/WebCore/platform/graphics/win/MediaPlayerPrivateFullscreenWindow.cpp', u'Source/WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp', u'Source/WebCore/platform/graphics/win/WKCAImageQueue.cpp', u'Source/WebCore/platform/image-decoders/ImageDecoder.h', u'Source/WebCore/platform/network/cf/AuthenticationCF.cpp', u'Source/WebCore/platform/network/cf/CookieJarCFNet.cpp', u'Source/WebCore/platform/network/curl/ResourceHandleManager.cpp', u'Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp', u'Source/WebCore/platform/win/LocalizedStringsWin.cpp', u'Source/WebCore/platform/win/SearchPopupMenuWin.cpp', u'Source/WebCore/plugins/mac/PluginPackageMac.cpp', u'Source/WebCore/plugins/mac/PluginViewMac.mm', u'Source/WebCore/rendering/RenderThemeSafari.cpp', u'Source/WebKit/win/CFDictionaryPropertyBag.cpp', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebCache.cpp', u'Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp', u'Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp', u'Source/WebKit/win/WebDatabaseManager.cpp', u'Source/WebKit/win/WebDownloadCFNet.cpp', u'Source/WebKit/win/WebError.cpp', u'Source/WebKit/win/WebHistory.cpp', u'Source/WebKit/win/WebIconDatabase.cpp', u'Source/WebKit/win/WebLocalizableStrings.cpp', u'Source/WebKit/win/WebMutableURLRequest.cpp', u'Source/WebKit/win/WebPreferences.cpp', u'Source/WebKit/win/WebView.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Shared/Downloads/cfnet/DownloadCFNet.cpp', u'Tools/ChangeLog', u'Tools/DumpRenderTree/cf/WebArchiveDumpSupport.cpp', u'Tools/DumpRenderTree/cg/ImageDiffCG.cpp', u'Tools/DumpRenderTree/cg/PixelDumpSupportCG.cpp', u'Tools/DumpRenderTree/cg/PixelDumpSupportCG.h', u'Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm', u'Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm', u'Tools/DumpRenderTree/mac/TestRunnerMac.mm', u'Tools/DumpRenderTree/mac/WebArchiveDumpSupportMac.mm', u'Tools/DumpRenderTree/mac/WorkQueueItemMac.mm', u'Tools/DumpRenderTree/win/DumpRenderTree.cpp', u'Tools/DumpRenderTree/win/ImageDiffCairo.cpp', u'Tools/DumpRenderTree/win/PixelDumpSupportWin.cpp', u'Tools/DumpRenderTree/win/TestRunnerWin.cpp', u'Tools/TestWebKitAPI/Tests/CustomProtocolsSyncXHRTest.mm', u'Tools/TestWebKitAPI/Tests/TestWebKitAPI/mac/InstanceMethodSwizzler.mm', u'Tools/TestWebKitAPI/Tests/WTF/cf/RetainPtrHashing.cpp', u'Tools/TestWebKitAPI/Tests/WebKit2/FindMatches.mm', u'Tools/TestWebKitAPI/Tests/WebKit2/WebArchive.cpp', u'Tools/TestWebKitAPI/Tests/WebKit2/mac/GetBackingScaleFactor.mm', u'Tools/TestWebKitAPI/Tests/WebKit2/win/DoNotCopyANullCFURLResponse.cpp', u'Tools/TestWebKitAPI/Tests/mac/AcceptsFirstMouse.mm', u'Tools/TestWebKitAPI/Tests/mac/AttributedString.mm', u'Tools/TestWebKitAPI/Tests/mac/BackForwardList.mm', u'Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/ContextMenuCanCopyURL.mm', u'Tools/TestWebKitAPI/Tests/mac/DOMHTMLTableCellCellAbove.mm', u'Tools/TestWebKitAPI/Tests/mac/DOMRangeOfString.mm', u'Tools/TestWebKitAPI/Tests/mac/DeviceScaleFactorInDashboardRegions.mm', u'Tools/TestWebKitAPI/Tests/mac/DeviceScaleFactorOnBack.mm', u'Tools/TestWebKitAPI/Tests/mac/DynamicDeviceScaleFactor.mm', u'Tools/TestWebKitAPI/Tests/mac/HTMLCollectionNamedItem.mm', u'Tools/TestWebKitAPI/Tests/mac/HTMLFormCollectionNamedItem.mm', u'Tools/TestWebKitAPI/Tests/mac/InspectorBar.mm', u'Tools/TestWebKitAPI/Tests/mac/MemoryCacheDisableWithinResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/MemoryCachePruneWithinResourceLoadDelegate.mm', u'Tools/TestWebKitAPI/Tests/mac/PageVisibilityStateWithWindowChanges.mm', u'Tools/TestWebKitAPI/Tests/mac/RenderedImageFromDOMRange.mm', u'Tools/TestWebKitAPI/Tests/mac/SetAndUpdateCacheModel.mm', u'Tools/TestWebKitAPI/Tests/mac/SetDocumentURI.mm', u'Tools/TestWebKitAPI/Tests/mac/SimplifyMarkup.mm', u'Tools/TestWebKitAPI/Tests/mac/StringByEvaluatingJavaScriptFromString.mm', u'Tools/TestWebKitAPI/Tests/mac/WillSendSubmitEvent.mm', u'Tools/TestWebKitAPI/Tests/mac/WindowlessWebViewWithMedia.mm', u'Tools/TestWebKitAPI/mac/PlatformUtilitiesMac.mm', u'Tools/TestWebKitAPI/mac/WebKitAgnosticTest.mm', u'Tools/WebKitTestRunner/InjectedBundle/mac/InjectedBundlePageMac.mm', u'Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm', u'Tools/WebKitTestRunner/cg/TestInvocationCG.cpp', u'Tools/WebKitTestRunner/mac/EventSenderProxy.mm', u'Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm', u'Tools/WebKitTestRunner/win/TestControllerWin.cpp']" exit_code: 1 Source/WebCore/platform/win/LocalizedStringsWin.cpp:81: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:763: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:768: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:772: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/rendering/RenderThemeSafari.cpp:977: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp:91: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/win/FontPlatformDataCGWin.cpp:98: Use 0 instead of NULL. [readability/null] [5] Total errors found: 7 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.

Darin Adler

Comment 6 2013-05-07 09:17:05 PDT

I don’t plan to fix the style queue complaints; they are all just pre-existing NULLs in Windows-specific code.

Benjamin Poulain

Comment 7 2013-05-07 14:42:24 PDT

I don't mind the change but what is the rationale? I would think if you do: foo.adoptCF(bar); you only get a release for foo.m_ptr. While: foo = adoptCF(bar); would give you Retain(bar); Release(foo.m_ptr); Release(bar); Or do you rely on Clang making it an rvalue reference?

Darin Adler

Comment 8 2013-05-07 18:23:14 PDT

(In reply to comment #7) > I don't mind the change but what is the rationale? We’re getting rid of old slight micro-optimizations that aren’t needed with the newer compilers. > Or do you rely on Clang making it an rvalue reference? Yes. We don’t need all these multiple ways of doing things. RefPtr doesn’t have an adopt member function and RetainPtr doesn’t need it either.

WebKit Commit Bot

Comment 9 2013-05-07 18:27:00 PDT

Comment on attachment 200910 [details] Patch Rejecting attachment 200910 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 200910, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 149701 = 36721b40217e2588bfec97f3f88d27b75ea6765d r149702 = 97613a518ee6cc5d18fca5d9170bf51f75496bd8 r149703 = 3cd2c83578b6edba49a0da70da9940da79ad132c Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.appspot.com/results/447004

Darin Adler

Comment 10 2013-05-08 00:39:24 PDT

Committed r149716: <http://trac.webkit.org/changeset/149716>

Note You need to log in before you can comment on or make changes to this bug.