Componentize component_updater: Use Configurator to build query parameters.

chrome/browser/component_updater/component_updater_utils.cc

Index: chrome/browser/component_updater/component_updater_utils.cc
diff --git a/chrome/browser/component_updater/component_updater_utils.cc b/chrome/browser/component_updater/component_updater_utils.cc
index d7ff01b49960232503667b7b5248563b40140865..08df0e193492cd1993d1cbfa0be3c9cae76eaf62 100644
--- a/chrome/browser/component_updater/component_updater_utils.cc
+++ b/chrome/browser/component_updater/component_updater_utils.cc
@@ -15,9 +15,8 @@
 #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"
@@ -40,14 +39,12 @@ int GetPhysicalMemoryGB() {
 
 }  // 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\"?>"
@@ -63,12 +60,12 @@ std::string BuildProtocolRequest(const std::string& request_body,
       "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"
@@ -89,7 +86,7 @@ std::string BuildProtocolRequest(const std::string& request_body,
   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"