Revert 69592 - file_version_info was not finding Mac values correctly.

Changed file_version_info to find Mac values, and changed version_info to fail if values can't be found.

BUG=NONE
TEST=BUILD

Review URL: http://codereview.chromium.org/5815001

TBR=dmaclach@chromium.org
Review URL: http://codereview.chromium.org/6017003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69601 0039d316-1c4b-4281-b951-d872f2087c98

21 files changed, 127 insertions, 44 deletions

diff --git a/chrome/browser/bug_report_util.cc b/chrome/browser/bug_report_util.cc
index b89d742..28cc685 100644
--- a/chrome/browser/bug_report_util.cc
+++ b/chrome/browser/bug_report_util.cc
@@ -309,11 +309,13 @@ void BugReportUtil::SendReport(Profile* profile,
 
   // Add the Chrome version
   chrome::VersionInfo version_info;
-  std::string chrome_version = version_info.Name() + " - " +
-      version_info.Version() +
-      " (" + version_info.LastChange() + ")";
-  AddFeedbackData(&feedback_data, std::string(kChromeVersionTag),
-                  chrome_version);
+  if (version_info.is_valid()) {
+    std::string chrome_version = version_info.Name() + " - " +
+        version_info.Version() +
+        " (" + version_info.LastChange() + ")";
+    AddFeedbackData(&feedback_data, std::string(kChromeVersionTag),
+                    chrome_version);
+  }
 
   // Add OS version (eg, for WinXP SP2: "5.1.2600 Service Pack 2").
   std::string os_version = "";
diff --git a/chrome/browser/diagnostics/recon_diagnostics.cc b/chrome/browser/diagnostics/recon_diagnostics.cc
index 0a572f3..f657fef 100644
--- a/chrome/browser/diagnostics/recon_diagnostics.cc
+++ b/chrome/browser/diagnostics/recon_diagnostics.cc
@@ -117,6 +117,10 @@ class VersionTest : public DiagnosticTest {
 
   virtual bool ExecuteImpl(DiagnosticsModel::Observer* observer) {
     chrome::VersionInfo version_info;
+    if (!version_info.is_valid()) {
+      RecordFailure(ASCIIToUTF16("No Version"));
+      return true;
+    }
     std::string current_version = version_info.Version();
     if (current_version.empty()) {
       RecordFailure(ASCIIToUTF16("Empty Version"));
diff --git a/chrome/browser/dom_ui/gpu_internals_ui.cc b/chrome/browser/dom_ui/gpu_internals_ui.cc
index 9b2b58f..9b93bfb 100644
--- a/chrome/browser/dom_ui/gpu_internals_ui.cc
+++ b/chrome/browser/dom_ui/gpu_internals_ui.cc
@@ -206,19 +206,23 @@ Value* GpuMessageHandler::OnRequestClientInfo(const ListValue* list) {
 
   chrome::VersionInfo version_info;
 
-  // We have everything we need to send the right values.
-  dict->SetString("version", version_info.Version());
-  dict->SetString("cl", version_info.LastChange());
-  dict->SetString("version_mod",
-    platform_util::GetVersionStringModifier());
-  dict->SetString("official",
-    l10n_util::GetStringUTF16(
-    version_info.IsOfficialBuild() ?
-    IDS_ABOUT_VERSION_OFFICIAL
-    : IDS_ABOUT_VERSION_UNOFFICIAL));
-
-  dict->SetString("command_line",
-    CommandLine::ForCurrentProcess()->command_line_string());
+  if (!version_info.is_valid()) {
+    DLOG(ERROR) << "Unable to create chrome::VersionInfo";
+  } else {
+    // We have everything we need to send the right values.
+    dict->SetString("version", version_info.Version());
+    dict->SetString("cl", version_info.LastChange());
+    dict->SetString("version_mod",
+      platform_util::GetVersionStringModifier());
+    dict->SetString("official",
+      l10n_util::GetStringUTF16(
+      version_info.IsOfficialBuild() ?
+      IDS_ABOUT_VERSION_OFFICIAL
+      : IDS_ABOUT_VERSION_UNOFFICIAL));
+
+    dict->SetString("command_line",
+      CommandLine::ForCurrentProcess()->command_line_string());
+  }
 
   return dict;
 }
diff --git a/chrome/browser/dom_ui/net_internals_ui.cc b/chrome/browser/dom_ui/net_internals_ui.cc
index 1f81022..f44ac9e 100644
--- a/chrome/browser/dom_ui/net_internals_ui.cc
+++ b/chrome/browser/dom_ui/net_internals_ui.cc
@@ -537,19 +537,23 @@ void NetInternalsMessageHandler::IOThreadImpl::OnRendererReady(
 
     chrome::VersionInfo version_info;
 
-    // We have everything we need to send the right values.
-    dict->SetString("version", version_info.Version());
-    dict->SetString("cl", version_info.LastChange());
-    dict->SetString("version_mod",
-                    platform_util::GetVersionStringModifier());
-    dict->SetString("official",
-        l10n_util::GetStringUTF16(
-            version_info.IsOfficialBuild() ?
-              IDS_ABOUT_VERSION_OFFICIAL
-            : IDS_ABOUT_VERSION_UNOFFICIAL));
-
-    dict->SetString("command_line",
-        CommandLine::ForCurrentProcess()->command_line_string());
+    if (!version_info.is_valid()) {
+      DLOG(ERROR) << "Unable to create chrome::VersionInfo";
+    } else {
+      // We have everything we need to send the right values.
+      dict->SetString("version", version_info.Version());
+      dict->SetString("cl", version_info.LastChange());
+      dict->SetString("version_mod",
+                      platform_util::GetVersionStringModifier());
+      dict->SetString("official",
+          l10n_util::GetStringUTF16(
+              version_info.IsOfficialBuild() ?
+                IDS_ABOUT_VERSION_OFFICIAL
+              : IDS_ABOUT_VERSION_UNOFFICIAL));
+
+      dict->SetString("command_line",
+          CommandLine::ForCurrentProcess()->command_line_string());
+    }
 
     CallJavascriptFunction(L"g_browser.receivedClientInfo",
                            dict);
diff --git a/chrome/browser/dom_ui/options/about_page_handler.cc b/chrome/browser/dom_ui/options/about_page_handler.cc
index 66420be..71f994f7 100644
--- a/chrome/browser/dom_ui/options/about_page_handler.cc
+++ b/chrome/browser/dom_ui/options/about_page_handler.cc
@@ -143,6 +143,7 @@ void AboutPageHandler::GetLocalizedValues(DictionaryValue* localized_strings) {
   // browser version
 
   chrome::VersionInfo version_info;
+  DCHECK(version_info.is_valid());
 
   std::string browser_version = version_info.Version();
   std::string version_modifier = platform_util::GetVersionStringModifier();
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 1ebf65b..d143052 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -827,8 +827,10 @@ std::vector<int> ExtensionUpdater::DetermineUpdates(
       // First determine the browser version if we haven't already.
       if (!browser_version.get()) {
         chrome::VersionInfo version_info;
-        browser_version.reset(Version::GetVersionFromString(
-                                  version_info.Version()));
+        if (version_info.is_valid()) {
+          browser_version.reset(Version::GetVersionFromString(
+                                    version_info.Version()));
+        }
       }
       scoped_ptr<Version> browser_min_version(
           Version::GetVersionFromString(update->browser_min_version));
diff --git a/chrome/browser/memory_details_mac.cc b/chrome/browser/memory_details_mac.cc
index 08fced1..b17d25f 100644
--- a/chrome/browser/memory_details_mac.cc
+++ b/chrome/browser/memory_details_mac.cc
@@ -201,8 +201,13 @@ void MemoryDetails::CollectProcessDataChrome(
     info.type = ChildProcessInfo::UNKNOWN_PROCESS;
 
   chrome::VersionInfo version_info;
-  info.product_name = ASCIIToWide(version_info.Name());
-  info.version = ASCIIToWide(version_info.Version());
+  if (version_info.is_valid()) {
+    info.product_name = ASCIIToWide(version_info.Name());
+    info.version = ASCIIToWide(version_info.Version());
+  } else {
+    info.product_name = process_data_[CHROME_BROWSER].name;
+    info.version = L"";
+  }
 
   // Check if this is one of the child processes whose data we collected
   // on the IO thread, and if so copy over that data.
diff --git a/chrome/browser/memory_details_win.cc b/chrome/browser/memory_details_win.cc
index a3ea337..959cebd 100644
--- a/chrome/browser/memory_details_win.cc
+++ b/chrome/browser/memory_details_win.cc
@@ -121,7 +121,8 @@ void MemoryDetails::CollectProcessData(
       TCHAR name[MAX_PATH];
       if (index2 == CHROME_BROWSER || index2 == CHROME_NACL_PROCESS) {
         chrome::VersionInfo version_info;
-        info.version = ASCIIToWide(version_info.Version());
+        if (version_info.is_valid())
+          info.version = ASCIIToWide(version_info.Version());
         // Check if this is one of the child processes whose data we collected
         // on the IO thread, and if so copy over that data.
         for (size_t child = 0; child < child_info.size(); child++) {
diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index 14f6ba9..d0a1f01 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -71,6 +71,11 @@ std::string MetricsLog::GetInstallDate() const {
 // static
 std::string MetricsLog::GetVersionString() {
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    NOTREACHED() << "Unable to retrieve version info.";
+    return std::string();
+  }
+
   std::string version = version_info.Version();
   if (!version_extension_.empty())
     version += version_extension_;
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index e620c11..8e29489 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -119,7 +119,7 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
   next_update_sec_ = base::RandInt(60, kSbTimerStartIntervalSec);
 
   chrome::VersionInfo version_info;
-  if (version_info.Version().empty())
+  if (!version_info.is_valid() || version_info.Version().empty())
     version_ = "0.1";
   else
     version_ = version_info.Version();
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index b34943b33..4dafe24 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -547,6 +547,11 @@ std::string MakeUserAgentForSyncapi() {
   user_agent += "MAC ";
 #endif
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    DLOG(ERROR) << "Unable to create chrome::VersionInfo object";
+    return user_agent;
+  }
+
   user_agent += version_info.Version();
   user_agent += " (" + version_info.LastChange() + ")";
   if (!version_info.IsOfficialBuild())
diff --git a/chrome/browser/ui/views/about_chrome_view.cc b/chrome/browser/ui/views/about_chrome_view.cc
index 7d53af6..9e58f04 100644
--- a/chrome/browser/ui/views/about_chrome_view.cc
+++ b/chrome/browser/ui/views/about_chrome_view.cc
@@ -151,6 +151,11 @@ void AboutChromeView::Init() {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    NOTREACHED() << L"Failed to initialize about window";
+    return;
+  }
+
   current_version_ = ASCIIToWide(version_info.Version());
 
   std::string version_modifier = platform_util::GetVersionStringModifier();
diff --git a/chrome/browser/upgrade_detector.cc b/chrome/browser/upgrade_detector.cc
index 2e0eeed..7f3161d 100644
--- a/chrome/browser/upgrade_detector.cc
+++ b/chrome/browser/upgrade_detector.cc
@@ -113,6 +113,10 @@ class DetectUpgradeTask : public Task {
 
     // Get the version of the currently *running* instance of Chrome.
     chrome::VersionInfo version_info;
+    if (!version_info.is_valid()) {
+      NOTREACHED() << "Failed to get current file version";
+      return;
+    }
     scoped_ptr<Version> running_version(
         Version::GetVersionFromString(version_info.Version()));
     if (running_version.get() == NULL) {
diff --git a/chrome/common/chrome_version_info.cc b/chrome/common/chrome_version_info.cc
index 70e8cb24..5157c4a 100644
--- a/chrome/common/chrome_version_info.cc
+++ b/chrome/common/chrome_version_info.cc
@@ -9,38 +9,47 @@
 #include "base/string_util.h"
 #include "base/thread_restrictions.h"
 #include "build/build_config.h"
-#include "chrome/common/chrome_constants.h"
 
 namespace chrome {
 
 #if defined(OS_WIN) || defined(OS_MACOSX)
-// On Windows and Mac we get the Chrome version info by querying FileVersionInfo
-// for the current module.
+// On Windows and Mac, we get the Chrome version info by querying
+// FileVersionInfo for the current module.
 
 VersionInfo::VersionInfo() {
   // The current module is already loaded in memory, so this will be cheap.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   version_info_.reset(FileVersionInfo::CreateFileVersionInfoForCurrentModule());
-  DCHECK(version_info_.get());
 }
 
 VersionInfo::~VersionInfo() {
 }
 
+bool VersionInfo::is_valid() const {
+  return version_info_.get() != NULL;
+}
+
 std::string VersionInfo::Name() const {
-  std::wstring name = version_info_->product_name();
-  return WideToASCII(name);
+  if (!is_valid())
+    return std::string();
+  return WideToASCII(version_info_->product_name());
 }
 
 std::string VersionInfo::Version() const {
+  if (!is_valid())
+    return std::string();
   return WideToASCII(version_info_->product_version());
 }
 
 std::string VersionInfo::LastChange() const {
+  if (!is_valid())
+    return std::string();
   return WideToASCII(version_info_->last_change());
 }
 
 bool VersionInfo::IsOfficialBuild() const {
+  if (!is_valid())
+    return false;
   return version_info_->is_official_build();
 }
 
@@ -56,6 +65,10 @@ VersionInfo::VersionInfo() {
 VersionInfo::~VersionInfo() {
 }
 
+bool VersionInfo::is_valid() const {
+  return true;
+}
+
 std::string VersionInfo::Name() const {
   return PRODUCT_NAME;
 }
diff --git a/chrome/common/chrome_version_info.h b/chrome/common/chrome_version_info.h
index b785249..0ec911e 100644
--- a/chrome/common/chrome_version_info.h
+++ b/chrome/common/chrome_version_info.h
@@ -22,6 +22,12 @@ class VersionInfo {
   VersionInfo();
   ~VersionInfo();
 
+  // In the rare case where we fail to get the version info,
+  // is_valid() will return false.  The other functions will return
+  // the empty string in this case, so it's not harmful if you don't
+  // check is_valid().
+  bool is_valid() const;
+
   // E.g. "Chromium" or "Google Chrome".
   std::string Name() const;
 
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 3cbe44d..56e0683 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1343,6 +1343,11 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
     }
 
     chrome::VersionInfo current_version_info;
+    if (!current_version_info.is_valid()) {
+      NOTREACHED();
+      return false;
+    }
+
     scoped_ptr<Version> current_version(
         Version::GetVersionFromString(current_version_info.Version()));
     if (!current_version.get()) {
diff --git a/chrome/common/service_process_util.cc b/chrome/common/service_process_util.cc
index 4c22fda..bcbac69 100644
--- a/chrome/common/service_process_util.cc
+++ b/chrome/common/service_process_util.cc
@@ -82,6 +82,12 @@ ServiceProcessRunningState GetServiceProcessRunningState(
 
   // Get the version of the currently *running* instance of Chrome.
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    NOTREACHED() << "Failed to get current file version";
+    // Our own version is invalid. This is an error case. Pretend that we
+    // are out of date.
+    return SERVICE_NEWER_VERSION_RUNNING;
+  }
   scoped_ptr<Version> running_version(Version::GetVersionFromString(
       version_info.Version()));
   if (!running_version.get()) {
@@ -124,6 +130,7 @@ std::string GetServiceProcessScopedVersionedName(
     const std::string& append_str) {
   std::string versioned_str;
   chrome::VersionInfo version_info;
+  DCHECK(version_info.is_valid());
   versioned_str.append(version_info.Version());
   versioned_str.append(append_str);
   return GetServiceProcessScopedName(versioned_str);
@@ -191,6 +198,10 @@ bool ServiceProcessState::HandleOtherVersion() {
 
 bool ServiceProcessState::CreateSharedData() {
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    NOTREACHED() << "Failed to get current file version";
+    return false;
+  }
   if (version_info.Version().length() >= kMaxVersionStringLength) {
     NOTREACHED() << "Version string length is << " <<
         version_info.Version().length() << "which is longer than" <<
diff --git a/chrome/common/service_process_util_unittest.cc b/chrome/common/service_process_util_unittest.cc
index 34b855a..58b68e1 100644
--- a/chrome/common/service_process_util_unittest.cc
+++ b/chrome/common/service_process_util_unittest.cc
@@ -14,6 +14,7 @@ TEST(ServiceProcessUtilTest, ScopedVersionedName) {
   std::string test_str = "test";
   std::string scoped_name = GetServiceProcessScopedVersionedName(test_str);
   chrome::VersionInfo version_info;
+  DCHECK(version_info.is_valid());
   EXPECT_TRUE(EndsWith(scoped_name, test_str, true));
   EXPECT_NE(std::string::npos, scoped_name.find(version_info.Version()));
 }
diff --git a/chrome/renderer/renderer_glue.cc b/chrome/renderer/renderer_glue.cc
index 6b29531..5cae2a9 100644
--- a/chrome/renderer/renderer_glue.cc
+++ b/chrome/renderer/renderer_glue.cc
@@ -261,7 +261,8 @@ void ClearCache() {
 std::string GetProductVersion() {
   chrome::VersionInfo version_info;
   std::string product("Chrome/");
-  product += version_info.Version();
+  product += version_info.is_valid() ? version_info.Version()
+                                     : "0.0.0.0";
   return product;
 }
 
diff --git a/chrome/service/net/service_url_request_context.cc b/chrome/service/net/service_url_request_context.cc
index cd94482..7f589e0 100644
--- a/chrome/service/net/service_url_request_context.cc
+++ b/chrome/service/net/service_url_request_context.cc
@@ -86,6 +86,9 @@ std::string BuildOSCpuInfo() {
 std::string MakeUserAgentForServiceProcess() {
   std::string user_agent;
   chrome::VersionInfo version_info;
+  if (!version_info.is_valid()) {
+    DLOG(ERROR) << "Unable to create chrome::VersionInfo object";
+  }
   std::string extra_version_info;
   if (!version_info.IsOfficialBuild())
     extra_version_info = "-devel";
diff --git a/chrome/test/automation/automation_proxy.cc b/chrome/test/automation/automation_proxy.cc
index 62b003e..4da271b 100644
--- a/chrome/test/automation/automation_proxy.cc
+++ b/chrome/test/automation/automation_proxy.cc
@@ -174,6 +174,7 @@ AutomationLaunchResult AutomationProxy::WaitForAppLaunch() {
       // Obtain our own version number and compare it to what the automation
       // provider sent.
       chrome::VersionInfo version_info;
+      DCHECK(version_info.is_valid());
 
       // Note that we use a simple string comparison since we expect the version
       // to be a punctuated numeric string. Consider using base/Version if we