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
: 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)
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

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>