Componentize component_updater: Use Configurator to build query parameters.

chrome/browser/component_updater/component_updater_configurator.cc

Index: chrome/browser/component_updater/component_updater_configurator.cc
diff --git a/chrome/browser/component_updater/component_updater_configurator.cc b/chrome/browser/component_updater/component_updater_configurator.cc
index f9027e782e6688df466dc6a509ee5071edbe054d..e78ef90b0597300ff29931264cec9347d7dce1bb 100644
--- a/chrome/browser/component_updater/component_updater_configurator.cc
+++ b/chrome/browser/component_updater/component_updater_configurator.cc
@@ -11,11 +11,19 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/sys_info.h"
+#include "base/win/windows_version.h"
 #include "build/build_config.h"
+#include "chrome/browser/omaha_query_params/chrome_omaha_query_params_delegate.h"
+#include "chrome/common/chrome_version_info.h"
 #include "components/component_updater/component_updater_switches.h"
+#include "components/omaha_query_params/omaha_query_params.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
 
+using omaha_query_params::OmahaQueryParams;
+
 namespace component_updater {
 
 namespace {
@@ -101,7 +109,10 @@ class ChromeConfigurator : public Configurator {
   virtual int OnDemandDelay() const OVERRIDE;
   virtual GURL UpdateUrl() const OVERRIDE;
   virtual GURL PingUrl() const OVERRIDE;
+  virtual base::Version ApplicationVersion() const OVERRIDE;
+  virtual std::string PlatformRequestParams() const OVERRIDE;
   virtual std::string ExtraRequestParams() const OVERRIDE;
+  virtual std::string RequestOSTag() const OVERRIDE;
   virtual size_t UrlSizeLimit() const OVERRIDE;
   virtual net::URLRequestContextGetter* RequestContext() const OVERRIDE;
   virtual bool InProcess() const OVERRIDE;
@@ -110,6 +121,7 @@ class ChromeConfigurator : public Configurator {
 
  private:
   net::URLRequestContextGetter* url_request_getter_;
+  std::string platform_request_params_;
   std::string extra_info_;
   std::string url_source_;
   bool fast_update_;
@@ -147,6 +159,32 @@ ChromeConfigurator::ChromeConfigurator(
     url_source_ = kDefaultUrlSource;
   }
 
+  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());
+  platform_request_params_ = base::StringPrintf(
+      "version=\"%s-%s\" prodversion=\"%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"
+      lang.c_str(),                      // "lang",
+      channel.c_str(),                   // "updaterchannel"
+      channel.c_str(),                   // "prodchannel"
+      OmahaQueryParams::GetOS(),         // "os"
+      OmahaQueryParams::GetArch(),       // "arch"
+      OmahaQueryParams::GetNaclArch());  // "nacl_arch"
+#if defined(OS_WIN)
+  if (base::win::OSInfo::GetInstance()->wow64_status() ==
+      base::win::OSInfo::WOW64_ENABLED) {
+    base::StringAppendF(&platform_request_params, " wow64=\"1\"");
+  }
+#endif
+
   if (HasSwitchValue(switch_values, kSwitchRequestParam))
     extra_info_ += "testrequest=\"1\"";
 }
@@ -183,10 +221,26 @@ GURL ChromeConfigurator::PingUrl() const {
   return pings_enabled_ ? GURL(kPingUrl) : GURL();
 }
 
+base::Version ChromeConfigurator::ApplicationVersion() const {
+  return base::Version(chrome::VersionInfo().Version());
+}
+
+std::string ChromeConfigurator::PlatformRequestParams() const {
+  return platform_request_params_;
+}
+
 std::string ChromeConfigurator::ExtraRequestParams() const {
   return extra_info_;
 }
 
+std::string ChromeConfigurator::RequestOSTag() const {

[Sorin Jianu, 2014/07/09 08:29:16]

I'll say up front I don't know of a good design here. That being said, the
current implementation couples the serialization of XML parameters too tightly
with the configurator, here and above. We'd like to consolidate the xml
serialization in one place, so that we can see and test what is being
serialized. Therefore, I would prefer a design where the configurator provides
low level ambient parameters instead of pre-formatted XML string. Would it be
possible to have something like this?

[tommycli, 2014/07/09 19:48:10]

Done.

+  return base::StringPrintf(
+      "<os platform=\"%s\" version=\"%s\" arch=\"%s\"/>",
+      chrome::VersionInfo().OSType().c_str(),                  // "platform"
+      base::SysInfo().OperatingSystemVersion().c_str(),        // "version"
+      base::SysInfo().OperatingSystemArchitecture().c_str());  // "arch"
+}
+
 size_t ChromeConfigurator::UrlSizeLimit() const {
   return 1024ul;
 }