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

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

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

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebKit Misc. (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Darin Adler

URL:
Keywords:

Depends on:
Blocks:

Reported:	2013-05-06 09:57 PDT by Darin Adler
Modified:	2013-05-08 00:39 PDT (History)
CC List:	11 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

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

Description Darin Adler 2013-05-06 09:57:33 PDT

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

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

Created attachment 200716 [details]
Patch

Comment 2 WebKit Commit Bot 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.

Comment 3 Build Bot 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

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

Created attachment 200910 [details]
Patch

Comment 5 WebKit Commit Bot 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.

Comment 6 Darin Adler 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.

Comment 7 Benjamin Poulain 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?

Comment 8 Darin Adler 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.

Comment 9 WebKit Commit Bot 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

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

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