Componentize component_updater: Use Configurator to build query parameters.

chrome/browser/component_updater/component_updater_utils.cc

 // Copyright 2013 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 "chrome/browser/component_updater/component_updater_utils.h"
 
 #include <cmath>
 
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/guid.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
 #include "base/win/windows_version.h"
+#include "chrome/browser/component_updater/component_updater_configurator.h"
 #include "chrome/browser/component_updater/crx_update_item.h"
-#include "chrome/browser/omaha_query_params/chrome_omaha_query_params_delegate.h"
-#include "chrome/common/chrome_version_info.h"
 #include "components/omaha_query_params/omaha_query_params.h"
 #include "extensions/common/extension.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 
 using omaha_query_params::OmahaQueryParams;
 
 namespace component_updater {
 
 namespace {
 
 // Returns the amount of physical memory in GB, rounded to the nearest GB.
 int GetPhysicalMemoryGB() {
   const double kOneGB = 1024 * 1024 * 1024;
   const int64 phys_mem = base::SysInfo::AmountOfPhysicalMemory();
   return static_cast<int>(std::floor(0.5 + phys_mem / kOneGB));
 }
 
 }  // namespace
 
-std::string BuildProtocolRequest(const std::string& request_body,
+std::string BuildProtocolRequest(const Configurator& config,

[Sorin Jianu, 2014/07/10 12:27:28]

I was suggesting to not take the dependency on Configurator here. Instead, the
caller of this function breaks down the config and provides the arguments to
this function at the call site. This is a small deal but I am just hoping to
avoid the Configurator spilling into every single module of the component
updater. I don't have a strong opinion about this, maybe waffles@ has one.

[tommycli, 2014/07/10 16:08:14]

Done.

+                                 const std::string& request_body,
                                  const std::string& additional_attributes) {
   const std::string prod_id(
       OmahaQueryParams::GetProdIdString(OmahaQueryParams::CHROME));
-  const chrome::VersionInfo chrome_version_info;
-  const std::string chrome_version(chrome_version_info.Version());
-  const std::string channel(ChromeOmahaQueryParamsDelegate::GetChannelString());
-  const std::string lang(ChromeOmahaQueryParamsDelegate::GetLang());
+  const std::string app_version(config.ApplicationVersion().GetString());
 
   std::string request(
       "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
       "<request protocol=\"3.0\" ");
 
   if (!additional_attributes.empty())
     base::StringAppendF(&request, "%s ", additional_attributes.c_str());
 
   // Chrome version and platform information.
   base::StringAppendF(
       &request,
       "version=\"%s-%s\" prodversion=\"%s\" "
       "requestid=\"{%s}\" lang=\"%s\" updaterchannel=\"%s\" prodchannel=\"%s\" "
       "os=\"%s\" arch=\"%s\" nacl_arch=\"%s\"",
       prod_id.c_str(),
-      chrome_version.c_str(),            // "version"
-      chrome_version.c_str(),            // "prodversion"
+      app_version.c_str(),               // "version"

[Sorin Jianu, 2014/07/10 12:27:28]

I suggesting keeping the old naming. "app version" usually means the "version"
found in the "app" element.

[tommycli, 2014/07/10 16:08:14]

Done. Changed to browser_version since it's technically not supposed to be
'chrome' specific.

+      app_version.c_str(),               // "prodversion"
       base::GenerateGUID().c_str(),      // "requestid"
-      lang.c_str(),                      // "lang",
-      channel.c_str(),                   // "updaterchannel"
-      channel.c_str(),                   // "prodchannel"
+      config.GetLang().c_str(),          // "lang",
+      config.GetChannelString().c_str(), // "updaterchannel"

[Sorin Jianu, 2014/07/10 12:27:28]

we could also keep the local variable "channel", and maybe the "lang" too.

[tommycli, 2014/07/10 16:08:13]

Done.

+      config.GetChannelString().c_str(), // "prodchannel"
       OmahaQueryParams::GetOS(),         // "os"
       OmahaQueryParams::GetArch(),       // "arch"
       OmahaQueryParams::GetNaclArch());  // "nacl_arch"
 #if defined(OS_WIN)
   const bool is_wow64(base::win::OSInfo::GetInstance()->wow64_status() ==
                       base::win::OSInfo::WOW64_ENABLED);
   if (is_wow64)
     base::StringAppendF(&request, " wow64=\"1\"");
 #endif
   base::StringAppendF(&request, ">");
 
   // HW platform information.
   base::StringAppendF(&request,
                       "<hw physmemory=\"%d\"/>",
                       GetPhysicalMemoryGB());  // "physmem" in GB.
 
   // OS version and platform information.
   base::StringAppendF(
       &request,
       "<os platform=\"%s\" version=\"%s\" arch=\"%s\"/>",
-      chrome::VersionInfo().OSType().c_str(),                  // "platform"
+      config.GetOSLongName().c_str(),                          // "platform"
       base::SysInfo().OperatingSystemVersion().c_str(),        // "version"
       base::SysInfo().OperatingSystemArchitecture().c_str());  // "arch"
 
   // The actual payload of the request.
   base::StringAppendF(&request, "%s</request>", request_body.c_str());
 
   return request;
 }
 
 net::URLFetcher* SendProtocolRequest(
@@ skipping 82 matching lines @@
   DCHECK(extensions::Extension::IdIsValid(id));
   return id;
 }
 
 std::string GetCrxComponentID(const CrxComponent& component) {
   return HexStringToID(StringToLowerASCII(
       base::HexEncode(&component.pk_hash[0], component.pk_hash.size() / 2)));
 }
 
 }  // namespace component_updater