[Cocoa] Update more code to be more ARC-compatible to prepare for future ARC adoption
Created attachment 341871 [details] Patch
Attachment 341871 [details] did not pass style-queue: ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:791: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:791: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] ERROR: Source/WebCore/bridge/objc/objc_class.mm:136: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/bridge/objc/objc_class.mm:223: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 4 in 52 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 341876 [details] Patch
Attachment 341876 [details] did not pass style-queue: ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:791: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:791: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] ERROR: Source/WebCore/bridge/objc/objc_class.mm:136: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/bridge/objc/objc_class.mm:223: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 4 in 52 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 341876 [details] Patch Attachment 341876 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7966497 New failing tests: accessibility/mac/attributed-string/attributed-string-for-range-with-options.html accessibility/content-editable-as-textarea.html accessibility/mac/attributed-string/attributed-string-for-range.html
Created attachment 341879 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 341876 [details] Patch Attachment 341876 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7966532 New failing tests: accessibility/mac/attributed-string/attributed-string-for-range-with-options.html accessibility/content-editable-as-textarea.html accessibility/mac/attributed-string/attributed-string-for-range.html
Created attachment 341880 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 341876 [details] Patch Attachment 341876 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7966582 New failing tests: accessibility/mac/attributed-string/attributed-string-for-range-with-options.html accessibility/content-editable-as-textarea.html accessibility/mac/attributed-string/attributed-string-for-range.html
Created attachment 341882 [details] Archive of layout-test-results from ews114 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 341888 [details] Patch
Comment on attachment 341888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341888&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:562 > + NSDictionary *attributes = @{ (__bridge NSString *)kCVPixelBufferPixelFormatTypeKey: @(kCVPixelFormatType_32BGRA) }; This is ok as-is. Could use auto here. > Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:222 > + NSDictionary* videoDecoderSpecification = @{ (__bridge NSString *)kVTVideoDecoderSpecification_EnableHardwareAcceleratedVideoDecoder: @YES }; Ditto. > Source/WebCore/platform/mac/WebCoreNSURLExtras.mm:819 > + CFURLRef result = CFURLCreateWithBytes(NULL, urlBytes.data(), fragRg.location - 1, kCFStringEncodingUTF8, NULL); This is ok as-is. NULL => nullptr (There are two in this line). > Source/WebCore/platform/mac/WebCoreNSURLExtras.mm:821 > + result = CFURLCreateWithBytes(NULL, urlBytes.data(), fragRg.location - 1, kCFStringEncodingISOLatin1, NULL); Ditto. > Source/WebCore/platform/mac/WebCoreNSURLExtras.mm:1012 > + CFURLRef result = CFURLCreateWithBytes(NULL, urlBytes, numBytes - range.length, kCFStringEncodingUTF8, NULL); Ditto. > Source/WebCore/platform/mac/WebCoreNSURLExtras.mm:1014 > + result = CFURLCreateWithBytes(NULL, urlBytes, numBytes - range.length, kCFStringEncodingISOLatin1, NULL); Ditto. > Source/WebCore/platform/network/mac/ResourceErrorMac.mm:121 > + : ResourceError((__bridge NSError *)cfError) This is ok as-is. Could use uniform initializer syntax.
Comment on attachment 341888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341888&action=review It’s tempting to comment on how to clean up the surrounding code, but that is not the purpose of this patch :) > Source/WebCore/bridge/objc/objc_class.mm:53 > + ObjcClass* aClass = reinterpret_cast<ObjcClass*>(const_cast<void*>(CFDictionaryGetValue(classesByIsA, (__bridge CFTypeRef)isa))); This is ok as-is. Could use auto here. > Source/WebCore/platform/mac/PluginBlacklist.mm:172 > + : m_bundleIDToMinimumSecureVersion(adoptNS([bundleIDToMinimumSecureVersion copy])) This is ok as-is. Could use uniform initializer syntax in this line and the lines below that are part of the initializer list. > Source/WebCore/platform/mac/URLMac.mm:57 > + // Creating a toll-free bridged CFURL, because creation with NSURL methods would not preserve the original string. Is the comma before “because” necessary? Since you’re touching this function, that is unrelated to this purpose of this patch, we could use a C++ style cast in the return statement.
(In reply to Daniel Bates from comment #13) > Since you’re touching this > function, that is unrelated to this purpose of this patch, we could use a > C++ style cast in the return statement. I made the other changes you requested, but I will not be changing this return statement for two reasons: 1) I prefer not to use C++ style typecasts for conversions between CF types and NS types because of the need for __bridge and this is a conversion from a CF type to an NS type. 2) The reason I didn’t change this particular line to add a __bridge is that it’s tricky to get it right for ARC and I am waiting until later when I figure out how to do it 100% correctly. It doesn’t seem helpful for me to change it in any way at this time since I don’t know how to make it right. I limited my changes to ones I am confident are correct for ARC, to make the actual conversion a bit easier so we can focus on the more difficult cases.
Committed r232501: <https://trac.webkit.org/changeset/232501>
<rdar://problem/40793363>
(In reply to Darin Adler from comment #14) > (In reply to Daniel Bates from comment #13) > > Since you’re touching this > > function, that is unrelated to this purpose of this patch, we could use a > > C++ style cast in the return statement. > > I made the other changes you requested, but I will not be changing this > return statement for two reasons: > > 1) I prefer not to use C++ style typecasts for conversions between CF types > and NS types because of the need for __bridge and this is a conversion from > a CF type to an NS type. > > 2) The reason I didn’t change this particular line to add a __bridge is that > it’s tricky to get it right for ARC and I am waiting until later when I > figure out how to do it 100% correctly. > > It doesn’t seem helpful for me to change it in any way at this time since I > don’t know how to make it right. I limited my changes to ones I am confident > are correct for ARC, to make the actual conversion a bit easier so we can > focus on the more difficult cases. Makes sense. Out of curiosity, are you using ARC conversion feature in Xcode to help you with the conversion?
(In reply to Daniel Bates from comment #17) > Out of curiosity, are you using ARC conversion feature in Xcode > to help you with the conversion? I tried. So far it has not been helpful.