Fixed file_version_info so that it finds Mac values correctly.

base/file_version_info_mac.mm

 // Copyright (c) 2010 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 "base/file_version_info_mac.h"
 
-#import <Cocoa/Cocoa.h>
+#import <Foundation/Foundation.h>

[Mark Mentovai, 2010/12/16 22:11:47]

:) Thanks!

 
-#include "base/basictypes.h"
 #include "base/file_path.h"
-#include "base/string_util.h"
-#include "base/utf_string_conversions.h"
+#include "base/logging.h"
+#include "base/sys_string_conversions.h"
+#include "base/mac_util.h"
 
 FileVersionInfoMac::FileVersionInfoMac(NSBundle *bundle) : bundle_(bundle) {
-  [bundle_ retain];
-}
-
-FileVersionInfoMac::~FileVersionInfoMac() {
-  [bundle_ release];
 }
 
 // static
 FileVersionInfo* FileVersionInfo::CreateFileVersionInfoForCurrentModule() {
-  // TODO(erikkay): this should really use bundleForClass, but we don't have
-  // a class to hang onto yet.
-  NSBundle* bundle = [NSBundle mainBundle];
-  return new FileVersionInfoMac(bundle);
-}
-
-// static
-FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
-    const std::wstring& file_path) {
-  NSString* path = [NSString stringWithCString:
-      reinterpret_cast<const char*>(file_path.c_str())
-        encoding:NSUTF32StringEncoding];
-  return new FileVersionInfoMac([NSBundle bundleWithPath:path]);
+  return CreateFileVersionInfo(mac_util::MainAppBundlePath());

[Mark Mentovai, 2010/12/16 22:11:47]

Ah, that’ll do.

 }
 
 // static
 FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
     const FilePath& file_path) {
-  NSString* path = [NSString stringWithUTF8String:file_path.value().c_str()];
-  return new FileVersionInfoMac([NSBundle bundleWithPath:path]);
+  NSString* path = base::SysUTF8ToNSString(file_path.value());
+  NSBundle *bundle = [NSBundle bundleWithPath:path];
+  NSString *identifier = [bundle bundleIdentifier];
+  if (!identifier) {
+    NOTREACHED() << "Unable to create file version for " << file_path.value();
+    return NULL;
+  } else {
+    return new FileVersionInfoMac(bundle);
+  }
 }
 
 std::wstring FileVersionInfoMac::company_name() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::company_short_name() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::internal_name() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::product_name() {
-  return GetStringValue(L"CFBundleName");
+  return GetStringValue(kCFBundleNameKey);
 }
 
 std::wstring FileVersionInfoMac::product_short_name() {
-  return GetStringValue(L"CFBundleName");
+  return GetStringValue(kCFBundleNameKey);
 }
 
 std::wstring FileVersionInfoMac::comments() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::legal_copyright() {
-  return GetStringValue(L"CFBundleGetInfoString");
+  return GetStringValue(CFSTR("CFBundleGetInfoString"));
 }
 
 std::wstring FileVersionInfoMac::product_version() {
-  return GetStringValue(L"CFBundleShortVersionString");
+  return GetStringValue(CFSTR("CFBundleShortVersionString"));
 }
 
 std::wstring FileVersionInfoMac::file_description() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::legal_trademarks() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::private_build() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::file_version() {
   return product_version();
 }
 
 std::wstring FileVersionInfoMac::original_filename() {
-  return GetStringValue(L"CFBundleName");
+  return GetStringValue(kCFBundleNameKey);
 }
 
 std::wstring FileVersionInfoMac::special_build() {
   return std::wstring();
 }
 
 std::wstring FileVersionInfoMac::last_change() {
-  return GetStringValue(L"SVNRevision");
+  return GetStringValue(CFSTR("SVNRevision_"));

[Mark Mentovai, 2010/12/16 22:11:47]

Underscore? Eh?

[dmac, 2010/12/16 22:20:42]

Good catch. Not sure how that got in there...

 }
 
 bool FileVersionInfoMac::is_official_build() {
 #if defined (GOOGLE_CHROME_BUILD)
   return true;
 #else
   return false;
 #endif
 }
 
-bool FileVersionInfoMac::GetValue(const wchar_t* name,
-                                  std::wstring* value_str) {
+std::wstring FileVersionInfoMac::GetStringValue(CFStringRef name) {
   if (bundle_) {
-    NSString* value = [bundle_ objectForInfoDictionaryKey:
-        [NSString stringWithUTF8String:WideToUTF8(name).c_str()]];
+    const NSString *ns_name = reinterpret_cast<const NSString *>(name);

[Mark Mentovai, 2010/12/16 22:11:47]

Is const really correct here?

[dmac, 2010/12/16 22:20:42]

it's either that or

NSString *ns_name = const_cast<NSString *>(reinterpret_cast<const NSString
*>(name));

[Mark Mentovai, 2010/12/16 22:39:20]

You can use a C-style cast in preference to nesting C++-style ones.

I’m concerned that clang might not like |const NSString*|, but if it works, then
I have no real objection.

+    NSString* value = [bundle_ objectForInfoDictionaryKey:ns_name];
     if (value) {
-      *value_str = reinterpret_cast<const wchar_t*>(
-          [value cStringUsingEncoding:NSUTF32StringEncoding]);
-      return true;
+      return base::SysNSStringToWide(value);
     }
   }
-  return false;
-}
-
-std::wstring FileVersionInfoMac::GetStringValue(const wchar_t* name) {
-  std::wstring str;
-  if (GetValue(name, &str))
-    return str;
   return std::wstring();
 }