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

net/proxy/proxy_config_service_mac.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/proxy/proxy_config_service_mac.h"
 
 #include <CoreFoundation/CoreFoundation.h>
 #include <SystemConfiguration/SystemConfiguration.h>
 
 #include "base/logging.h"
+#include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"

[wtc, 2011/11/16 06:23:33]

Can you see if we can remove this #include line now?

 #include "base/mac/scoped_cftyperef.h"
 #include "base/message_loop.h"
 #include "base/sys_string_conversions.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_info.h"
 #include "net/proxy/proxy_server.h"
 
 namespace net {
 
 namespace {
 
 const int kPollIntervalSec = 5;
 
 // Utility function to pull out a boolean value from a dictionary and return it,
 // returning a default value if the key is not present.
 bool GetBoolFromDictionary(CFDictionaryRef dict,
                            CFStringRef key,
                            bool default_value) {
-  CFNumberRef number = (CFNumberRef)base::mac::GetValueFromDictionary(
-      dict, key, CFNumberGetTypeID());
+  CFNumberRef number = base::mac::CFCastStrict<CFNumberRef>(
+      base::mac::GetValueFromDictionary(dict, key, CFNumberGetTypeID()));

[Mark Mentovai, 2011/11/11 17:31:50]

I would only consider this a temporary measure. GetValueFromDictionary now
requires you to specify the type in two different ways: once as the third
argument and once in the cast as you store the value in an appropriately-typed
variable. That’s not cool. I think that GetValueFromDictionary should be
templatized to return the proper type.

   if (!number)
     return default_value;
 
   int int_value;
   if (CFNumberGetValue(number, kCFNumberIntType, &int_value))
     return int_value;
   else
     return default_value;
 }
 
@@ skipping 10 matching lines @@
   config->set_auto_detect(
       GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesProxyAutoDiscoveryEnable,
                             false));
 
   // PAC file
 
   if (GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesProxyAutoConfigEnable,
                             false)) {
-    CFStringRef pac_url_ref = (CFStringRef)base::mac::GetValueFromDictionary(
-        config_dict.get(),
-        kSCPropNetProxiesProxyAutoConfigURLString,
-        CFStringGetTypeID());
+    CFStringRef pac_url_ref = base::mac::CFCastStrict<CFStringRef>(
+        base::mac::GetValueFromDictionary(
+            config_dict.get(),
+            kSCPropNetProxiesProxyAutoConfigURLString,
+            CFStringGetTypeID()));
     if (pac_url_ref)
       config->set_pac_url(GURL(base::SysCFStringRefToUTF8(pac_url_ref)));
   }
 
   // proxies (for now ftp, http, https, and SOCKS)
 
   if (GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesFTPEnable,
                             false)) {
     ProxyServer proxy_server =
@@ skipping 45 matching lines @@
                                     kSCPropNetProxiesSOCKSPort);
     if (proxy_server.is_valid()) {
       config->proxy_rules().type =
           ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME;
       config->proxy_rules().fallback_proxy = proxy_server;
     }
   }
 
   // proxy bypass list
 
-  CFArrayRef bypass_array_ref =
-      (CFArrayRef)base::mac::GetValueFromDictionary(
+  CFArrayRef bypass_array_ref = base::mac::CFCastStrict<CFArrayRef>(
+      base::mac::GetValueFromDictionary(
           config_dict.get(),
           kSCPropNetProxiesExceptionsList,
-          CFArrayGetTypeID());
+          CFArrayGetTypeID()));
   if (bypass_array_ref) {
     CFIndex bypass_array_count = CFArrayGetCount(bypass_array_ref);
     for (CFIndex i = 0; i < bypass_array_count; ++i) {
-      CFStringRef bypass_item_ref =
-          (CFStringRef)CFArrayGetValueAtIndex(bypass_array_ref, i);
+      CFStringRef bypass_item_ref = base::mac::CFCast<CFStringRef>(
+              CFArrayGetValueAtIndex(bypass_array_ref, i));
       if (CFGetTypeID(bypass_item_ref) != CFStringGetTypeID()) {

[Mark Mentovai, 2011/11/11 17:31:50]

This is wrong now. bypass_item_ref will either be a pointer to a CFString, or it
will be NULL if the value in the array was not a CFString. This check can become

      if (!bypass_item_ref) {

         LOG(WARNING) << "Expected value for item " << i
                      << " in the kSCPropNetProxiesExceptionsList"
                         " to be a CFStringRef but it was not";
 
       } else {
         config->proxy_rules().bypass_rules.AddRuleFromString(
             base::SysCFStringRefToUTF8(bypass_item_ref));
       }
     }
   }
@@ skipping 111 matching lines @@
   // Keep track of the last value we have seen.
   has_fetched_config_ = true;
   last_config_fetched_ = new_config;
 
   // Notify all the observers.
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnProxyConfigChanged(new_config, CONFIG_VALID));
 }
 
 }  // namespace net