Simplify VersionInfo code, avoid hitting sandbox IPC constantly on Wi…

…ndows Using FileVersionInfo to fill the VersionInfo causes at least three sync IPCs when used from the renderer. This is used when filling the user agent string for every network request, so having it not do the sync ipcs is... better. So, use the previously posix-only path on Windows and Mac too, since it's simpler anyway. R=thestig@chromium.org, cpu@chromium.org BUG=417941,402714 Review URL: https://codereview.chromium.org/634583003 Cr-Commit-Position: refs/heads/master@{#298319}
cbchan · Oct 7, 2014 · 2e46253 · 2e46253
1 parent 277f651
commit 2e46253
Show file tree

Hide file tree

Showing 30 changed files with 105 additions and 247 deletions.
diff --git a/chrome/app/chrome_crash_reporter_client.cc b/chrome/app/chrome_crash_reporter_client.cc
@@ -35,7 +35,7 @@
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_IOS)
 #include "chrome/browser/crash_upload_list.h"
-#include "chrome/common/chrome_version_info_posix.h"
+#include "chrome/common/chrome_version_info_values.h"
 #endif
 
 #if defined(OS_POSIX)

diff --git a/chrome/browser/chromeos/drive/drive_integration_service.cc b/chrome/browser/chromeos/drive/drive_integration_service.cc
@@ -78,9 +78,7 @@ std::string GetDriveUserAgent() {
   const char kDriveClientName[] = "chromedrive";
 
   chrome::VersionInfo version_info;
-  const std::string version = (version_info.is_valid() ?
-                               version_info.Version() :
-                               std::string("unknown"));
+  const std::string version = version_info.Version();
 
   // This part is <client_name>/<version>.
   const char kLibraryInfo[] = "chrome-cc/none";

diff --git a/chrome/browser/diagnostics/recon_diagnostics.cc b/chrome/browser/diagnostics/recon_diagnostics.cc
@@ -359,10 +359,6 @@ class VersionTest : public DiagnosticsTest {
 
   virtual bool ExecuteImpl(DiagnosticsModel::Observer* observer) OVERRIDE {
     chrome::VersionInfo version_info;
-    if (!version_info.is_valid()) {
-      RecordFailure(DIAG_RECON_NO_VERSION, "No Version");
-      return true;
-    }
     std::string current_version = version_info.Version();
     if (current_version.empty()) {
       RecordFailure(DIAG_RECON_EMPTY_VERSION, "Empty Version");

diff --git a/chrome/browser/extensions/updater/extension_downloader.cc b/chrome/browser/extensions/updater/extension_downloader.cc
@@ -672,8 +672,7 @@ void ExtensionDownloader::DetermineUpdates(
       // First determine the browser version if we haven't already.
       if (!browser_version.IsValid()) {
         chrome::VersionInfo version_info;
-        if (version_info.is_valid())
-          browser_version = Version(version_info.Version());
+        browser_version = Version(version_info.Version());
       }
       Version browser_min_version(update->browser_min_version);
       if (browser_version.IsValid() && browser_min_version.IsValid() &&

diff --git a/chrome/browser/memory_details_mac.cc b/chrome/browser/memory_details_mac.cc
@@ -215,13 +215,8 @@ void MemoryDetails::CollectProcessDataChrome(
     info.process_type = content::PROCESS_TYPE_UNKNOWN;
 
   chrome::VersionInfo version_info;
-  if (version_info.is_valid()) {
-    info.product_name = base::ASCIIToUTF16(version_info.Name());
-    info.version = base::ASCIIToUTF16(version_info.Version());
-  } else {
-    info.product_name = process_data_[CHROME_BROWSER].name;
-    info.version = base::string16();
-  }
+  info.product_name = base::ASCIIToUTF16(version_info.Name());
+  info.version = base::ASCIIToUTF16(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.

diff --git a/chrome/browser/memory_details_win.cc b/chrome/browser/memory_details_win.cc
@@ -124,8 +124,7 @@ void MemoryDetails::CollectProcessData(
       TCHAR name[MAX_PATH];
       if (index2 == CHROME_BROWSER || index2 == CHROME_NACL_PROCESS) {
         chrome::VersionInfo version_info;
-        if (version_info.is_valid())
-          info.version = base::ASCIIToWide(version_info.Version());
+        info.version = base::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/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -197,11 +197,6 @@ metrics::SystemProfileProto::Channel ChromeMetricsServiceClient::GetChannel() {
 
 std::string ChromeMetricsServiceClient::GetVersionString() {
   chrome::VersionInfo version_info;
-  if (!version_info.is_valid()) {
-    NOTREACHED();
-    return std::string();
-  }
-
   std::string version = version_info.Version();
 #if defined(ARCH_CPU_64_BITS)
   version += "-64";

diff --git a/chrome/browser/metrics/variations/variations_service.cc b/chrome/browser/metrics/variations/variations_service.cc
@@ -259,9 +259,6 @@ bool VariationsService::CreateTrialsFromSeed() {
     return false;
 
   const chrome::VersionInfo current_version_info;
-  if (!current_version_info.is_valid())
-    return false;
-
   const base::Version current_version(current_version_info.Version());
   if (!current_version.IsValid())
     return false;

diff --git a/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc b/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc
@@ -41,17 +41,15 @@ ClientIncidentReport_EnvironmentData_Process_Channel MapChannelToProtobuf(
 // Populates |process| with data related to the chrome browser process.
 void CollectProcessData(ClientIncidentReport_EnvironmentData_Process* process) {
   chrome::VersionInfo version_info;
-  if (version_info.is_valid()) {
-    // TODO(grt): Move this logic into VersionInfo (it also appears in
-    // ChromeMetricsServiceClient).
-    std::string version(version_info.Version());
+  // TODO(grt): Move this logic into VersionInfo (it also appears in
+  // ChromeMetricsServiceClient).
+  std::string version(version_info.Version());
 #if defined(ARCH_CPU_64_BITS)
-    version += "-64";
+  version += "-64";
 #endif  // defined(ARCH_CPU_64_BITS)
-    if (!version_info.IsOfficialBuild())
-      version += "-devel";
-    process->set_version(version);
-  }
+  if (!version_info.IsOfficialBuild())
+    version += "-devel";
+  process->set_version(version);
 
   process->set_chrome_update_channel(
       MapChannelToProtobuf(chrome::VersionInfo::GetChannel()));

diff --git a/chrome/browser/safe_browsing/protocol_manager_helper.cc b/chrome/browser/safe_browsing/protocol_manager_helper.cc
@@ -31,7 +31,7 @@ SafeBrowsingProtocolConfig::~SafeBrowsingProtocolConfig() {
 // static
 std::string SafeBrowsingProtocolManagerHelper::Version() {
   chrome::VersionInfo version_info;
-  if (!version_info.is_valid() || version_info.Version().empty())
+  if (version_info.Version().empty())
     return "0.1";
   else
     return version_info.Version();

diff --git a/chrome/browser/search_engines/ui_thread_search_terms_data.cc b/chrome/browser/search_engines/ui_thread_search_terms_data.cc
@@ -178,17 +178,14 @@ std::string UIThreadSearchTermsData::NTPIsThemedParam() const {
 // VersionInfo.
 std::string UIThreadSearchTermsData::GoogleImageSearchSource() const {
   chrome::VersionInfo version_info;
-  if (version_info.is_valid()) {
-    std::string version(version_info.Name() + " " + version_info.Version());
-    if (version_info.IsOfficialBuild())
-      version += " (Official)";
-    version += " " + version_info.OSType();
-    std::string modifier(version_info.GetVersionStringModifier());
-    if (!modifier.empty())
-      version += " " + modifier;
-    return version;
-  }
-  return "unknown";
+  std::string version(version_info.Name() + " " + version_info.Version());
+  if (version_info.IsOfficialBuild())
+    version += " (Official)";
+  version += " " + version_info.OSType();
+  std::string modifier(version_info.GetVersionStringModifier());
+  if (!modifier.empty())
+    version += " " + modifier;
+  return version;
 }
 
 // static

diff --git a/chrome/browser/signin/chrome_signin_client.cc b/chrome/browser/signin/chrome_signin_client.cc
@@ -177,8 +177,6 @@ bool ChromeSigninClient::ShouldMergeSigninCredentialsIntoCookieJar() {
 
 std::string ChromeSigninClient::GetProductVersion() {
   chrome::VersionInfo chrome_version;
-  if (!chrome_version.is_valid())
-    return "invalid";
   return chrome_version.CreateVersionString();
 }
 

diff --git a/chrome/browser/sync/about_sync_util.cc b/chrome/browser/sync/about_sync_util.cc
@@ -143,8 +143,6 @@ std::string GetVersionString() {
   // Build a version string that matches MakeUserAgentForSyncApi with the
   // addition of channel info and proper OS names.
   chrome::VersionInfo chrome_version;
-  if (!chrome_version.is_valid())
-    return "invalid";
   // GetVersionStringModifier returns empty string for stable channel or
   // unofficial builds, the channel string otherwise. We want to have "-devel"
   // for unofficial builds only.

diff --git a/chrome/browser/sync/glue/local_device_info_provider_impl.cc b/chrome/browser/sync/glue/local_device_info_provider_impl.cc
@@ -87,11 +87,6 @@ std::string LocalDeviceInfoProviderImpl::MakeUserAgentForSyncApi(
 #elif defined(OS_MACOSX)
   user_agent += "MAC ";
 #endif
-  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/browser_commands.cc b/chrome/browser/ui/browser_commands.cc
@@ -1131,9 +1131,7 @@ void ToggleRequestTabletSite(Browser* browser) {
   } else {
     entry->SetIsOverridingUserAgent(true);
     chrome::VersionInfo version_info;
-    std::string product;
-    if (version_info.is_valid())
-      product = version_info.ProductNameAndVersionForUserAgent();
+    std::string product = version_info.ProductNameAndVersionForUserAgent();
     current_tab->SetUserAgentOverride(content::BuildUserAgentFromOSAndProduct(
         kOsOverrideForTabletSite, product));
   }

diff --git a/chrome/browser/ui/webui/help/help_handler.cc b/chrome/browser/ui/webui/help/help_handler.cc
@@ -312,7 +312,6 @@ void HelpHandler::Observe(int type, const content::NotificationSource& source,
 // static
 base::string16 HelpHandler::BuildBrowserVersionString() {
   chrome::VersionInfo version_info;
-  DCHECK(version_info.is_valid());
 
   std::string version = version_info.Version();
 

diff --git a/chrome/browser/ui/webui/net_internals/net_internals_ui.cc b/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
@@ -1705,22 +1705,17 @@ base::Value* NetInternalsUI::GetConstants() {
 
     chrome::VersionInfo version_info;
 
-    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("name", version_info.Name());
-      dict->SetString("version", version_info.Version());
-      dict->SetString("cl", version_info.LastChange());
-      dict->SetString("version_mod",
-                      chrome::VersionInfo::GetVersionStringModifier());
-      dict->SetString("official",
-                      version_info.IsOfficialBuild() ? "official" :
-                                                       "unofficial");
-      dict->SetString("os_type", version_info.OSType());
-      dict->SetString("command_line",
-                      CommandLine::ForCurrentProcess()->GetCommandLineString());
-    }
+    // We have everything we need to send the right values.
+    dict->SetString("name", version_info.Name());
+    dict->SetString("version", version_info.Version());
+    dict->SetString("cl", version_info.LastChange());
+    dict->SetString("version_mod",
+                    chrome::VersionInfo::GetVersionStringModifier());
+    dict->SetString("official",
+                    version_info.IsOfficialBuild() ? "official" : "unofficial");
+    dict->SetString("os_type", version_info.OSType());
+    dict->SetString("command_line",
+                    CommandLine::ForCurrentProcess()->GetCommandLineString());
 
     constants_dict->Set("clientInfo", dict);
   }

diff --git a/chrome/browser/upgrade_detector_impl.cc b/chrome/browser/upgrade_detector_impl.cc
@@ -315,10 +315,6 @@ void UpgradeDetectorImpl::DetectUpgradeTask(
 
   // 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;
-  }
   Version running_version(version_info.Version());
   if (!running_version.IsValid()) {
     NOTREACHED();

diff --git a/chrome/chrome_common.gypi b/chrome/chrome_common.gypi
@@ -533,60 +533,56 @@
       # GN version: //chrome/common:version
       'target_name': 'common_version',
       'type': 'none',
-      'conditions': [
-        ['os_posix == 1 and OS != "mac" and OS != "ios"', {
-          'direct_dependent_settings': {
-            'include_dirs': [
-              '<(SHARED_INTERMEDIATE_DIR)',
-            ],
+      'direct_dependent_settings': {
+        'include_dirs': [
+          '<(SHARED_INTERMEDIATE_DIR)',
+        ],
+      },
+      # Because generate_version generates a header, we must set the
+      # hard_dependency flag.
+      'hard_dependency': 1,
+      'actions': [
+        {
+          'action_name': 'generate_version',
+          'variables': {
+            'lastchange_path': '<(DEPTH)/build/util/LASTCHANGE',
+            'version_py_path': '<(DEPTH)/build/util/version.py',
+            'version_path': 'VERSION',
+            'template_input_path': 'common/chrome_version_info_values.h.version',
           },
-          # Because posix_version generates a header, we must set the
-          # hard_dependency flag.
-          'hard_dependency': 1,
-          'actions': [
-            {
-              'action_name': 'posix_version',
+          'conditions': [
+            [ 'branding == "Chrome"', {
               'variables': {
-                'lastchange_path': '<(DEPTH)/build/util/LASTCHANGE',
-                'version_py_path': '<(DEPTH)/build/util/version.py',
-                'version_path': 'VERSION',
-                'template_input_path': 'common/chrome_version_info_posix.h.version',
+                  'branding_path':
+                    'app/theme/google_chrome/BRANDING',
               },
-              'conditions': [
-                [ 'branding == "Chrome"', {
-                  'variables': {
-                     'branding_path':
-                       'app/theme/google_chrome/BRANDING',
-                  },
-                }, { # else branding!="Chrome"
-                  'variables': {
-                     'branding_path':
-                       'app/theme/chromium/BRANDING',
-                  },
-                }],
-              ],
-              'inputs': [
-                '<(template_input_path)',
-                '<(version_path)',
-                '<(branding_path)',
-                '<(lastchange_path)',
-              ],
-              'outputs': [
-                '<(SHARED_INTERMEDIATE_DIR)/chrome/common/chrome_version_info_posix.h',
-              ],
-              'action': [
-                'python',
-                '<(version_py_path)',
-                '-f', '<(version_path)',
-                '-f', '<(branding_path)',
-                '-f', '<(lastchange_path)',
-                '<(template_input_path)',
-                '<@(_outputs)',
-              ],
-              'message': 'Generating version information',
-            },
+            }, { # else branding!="Chrome"
+              'variables': {
+                  'branding_path':
+                    'app/theme/chromium/BRANDING',
+              },
+            }],
           ],
-        }],
+          'inputs': [
+            '<(template_input_path)',
+            '<(version_path)',
+            '<(branding_path)',
+            '<(lastchange_path)',
+          ],
+          'outputs': [
+            '<(SHARED_INTERMEDIATE_DIR)/chrome/common/chrome_version_info_values.h',
+          ],
+          'action': [
+            'python',
+            '<(version_py_path)',
+            '-f', '<(version_path)',
+            '-f', '<(branding_path)',
+            '-f', '<(lastchange_path)',
+            '<(template_input_path)',
+            '<@(_outputs)',
+          ],
+          'message': 'Generating version information',
+        },
       ],
     },
     {

diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn
@@ -241,12 +241,8 @@ if (is_linux || is_android) {
   import("//chrome/version.gni")
   process_version("version") {
     visibility = [ ":common" ]
-    source = "chrome_version_info_posix.h.version"
-    output = "$target_gen_dir/chrome_version_info_posix.h"
-  }
-} else {
-  # Other platforms have a different way to do versioning.
-  group("version") {
+    source = "chrome_version_info_values.h.version"
+    output = "$target_gen_dir/chrome_version_info_values.h"
   }
 }
 

diff --git a/chrome/common/chrome_content_client.cc b/chrome/common/chrome_content_client.cc
@@ -411,8 +411,7 @@ bool GetBundledPepperFlash(content::PepperPluginInfo* plugin) {
 
 std::string GetProduct() {
   chrome::VersionInfo version_info;
-  return version_info.is_valid() ?
-      version_info.ProductNameAndVersionForUserAgent() : std::string();
+  return version_info.ProductNameAndVersionForUserAgent();
 }
 
 }  // namespace

diff --git a/chrome/common/chrome_content_client_ios.mm b/chrome/common/chrome_content_client_ios.mm
@@ -38,7 +38,7 @@
 std::string ChromeContentClient::GetProduct() const {
   chrome::VersionInfo version_info;
   std::string product("CriOS/");
-  product += version_info.is_valid() ? version_info.Version() : "0.0.0.0";
+  product += version_info.Version();
   return product;
 }