Convert plain C-style casts to use CFCastStrict and GetValueFromDictionary template

net/proxy/proxy_resolver_mac.cc

Index: net/proxy/proxy_resolver_mac.cc
diff --git a/net/proxy/proxy_resolver_mac.cc b/net/proxy/proxy_resolver_mac.cc
index 6c5946c9987255f00e9d19295f844ffc4f0cf8a1..ed6ffba4edc7b3ec2cdae72f139e67170b9c4d77 100644
--- a/net/proxy/proxy_resolver_mac.cc
+++ b/net/proxy/proxy_resolver_mac.cc
@@ -123,15 +123,16 @@ int ProxyResolverMac::GetProxyForURL(const GURL& query_url,
   CFRunLoopRunInMode(private_runloop_mode, DBL_MAX, false);
   CFRunLoopRemoveSource(CFRunLoopGetCurrent(), runloop_source.get(),
                         private_runloop_mode);
-  DCHECK(result != NULL);
+  DCHECK(!result);

[Mark Mentovai, 2011/11/15 14:23:20]

This used to be DCHECK(result != NULL) and you’ve changed it to DCHECK(result),
which has the exact opposite meaning.

[KushalP, 2011/11/15 15:07:59]

Yup. What I did is the same as DCHECK(result == NULL). Fixing.

 
   if (CFGetTypeID(result) == CFErrorGetTypeID()) {
     // TODO(avi): do something better than this
     CFRelease(result);
     return ERR_FAILED;
   }
-  DCHECK(CFGetTypeID(result) == CFArrayGetTypeID());
-  base::mac::ScopedCFTypeRef<CFArrayRef> proxy_array_ref((CFArrayRef)result);
+  base::mac::ScopedCFTypeRef<CFArrayRef> proxy_array_ref(
+      base::mac::CFCast<CFArrayRef>(result));
+  DCHECK(!result);

[Mark Mentovai, 2011/11/15 14:23:20]

This, too, appears to be the wrong condition to DCHECK. Have you tested this?

[KushalP, 2011/11/15 15:07:59]

Will change this to CFCastStrict and add a check for NULL.

I tested locally and ran through the trybots which appears green.

Should I also add tests for this class?

[Mark Mentovai, 2011/11/15 17:35:28]

KushalP wrote:
Probably not necessary.

 
   // This string will be an ordered list of <proxy-uri> entries, separated by
   // semi-colons. It is the format that ProxyInfo::UseNamedProxy() expects.
@@ -141,9 +142,9 @@ int ProxyResolverMac::GetProxyForURL(const GURL& query_url,
 
   CFIndex proxy_array_count = CFArrayGetCount(proxy_array_ref.get());
   for (CFIndex i = 0; i < proxy_array_count; ++i) {
-    CFDictionaryRef proxy_dictionary =
-        (CFDictionaryRef)CFArrayGetValueAtIndex(proxy_array_ref.get(), i);
-    DCHECK(CFGetTypeID(proxy_dictionary) == CFDictionaryGetTypeID());
+    CFDictionaryRef proxy_dictionary = base::mac::CFCast<CFDictionaryRef>(
+        CFArrayGetValueAtIndex(proxy_array_ref.get(), i));
+    DCHECK(!proxy_dictionary);

[Mark Mentovai, 2011/11/15 14:23:20]

And once more.

 
     // The dictionary may have the following keys:
     // - kCFProxyTypeKey : The type of the proxy
@@ -159,10 +160,8 @@ int ProxyResolverMac::GetProxyForURL(const GURL& query_url,
     // - kCFProxyAutoConfigurationURLKey : If the PAC file specifies another
     //                                     PAC file, I'm going home.
 
-    CFStringRef proxy_type =
-        (CFStringRef)base::mac::GetValueFromDictionary(proxy_dictionary,
-                                                      kCFProxyTypeKey,
-                                                      CFStringGetTypeID());
+    CFStringRef proxy_type = base::mac::GetValueFromDictionary<CFStringRef>(
+        proxy_dictionary, kCFProxyTypeKey);
     ProxyServer proxy_server = ProxyServer::FromDictionary(
         GetProxyServerScheme(proxy_type),
         proxy_dictionary,