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

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

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

[Darin Adler]

Created attachment 200716 [details]
Patch

[WebKit Commit Bot]

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 on attachment 200716 [details]
Patch

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

[Darin Adler]

Created attachment 200910 [details]
Patch

[WebKit Commit Bot]

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]

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

[Benjamin Poulain]

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]

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

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