Componentize component_updater: Use Configurator to build query param…

…eters. This also breaks a few more misc. bonds between chrome/common and component_updater. BUG=371463 Review URL: https://codereview.chromium.org/375973003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282444 0039d316-1c4b-4281-b951-d872f2087c98
hylschwy · Jul 10, 2014 · 6a8ab1d · 6a8ab1d
1 parent 5296461
commit 6a8ab1d
Show file tree

Hide file tree

Showing 17 changed files with 127 additions and 44 deletions.
diff --git a/chrome/browser/component_updater/component_updater_configurator.cc b/chrome/browser/component_updater/component_updater_configurator.cc
@@ -11,7 +11,10 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/strings/string_util.h"
+#include "base/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 "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
@@ -101,6 +104,10 @@ class ChromeConfigurator : public Configurator {
   virtual int OnDemandDelay() const OVERRIDE;
   virtual GURL UpdateUrl() const OVERRIDE;
   virtual GURL PingUrl() const OVERRIDE;
+  virtual base::Version GetBrowserVersion() const OVERRIDE;
+  virtual std::string GetChannel() const OVERRIDE;
+  virtual std::string GetLang() const OVERRIDE;
+  virtual std::string GetOSLongName() const OVERRIDE;
   virtual std::string ExtraRequestParams() const OVERRIDE;
   virtual size_t UrlSizeLimit() const OVERRIDE;
   virtual net::URLRequestContextGetter* RequestContext() const OVERRIDE;
@@ -183,6 +190,22 @@ GURL ChromeConfigurator::PingUrl() const {
   return pings_enabled_ ? GURL(kPingUrl) : GURL();
 }
 
+base::Version ChromeConfigurator::GetBrowserVersion() const {
+  return base::Version(chrome::VersionInfo().Version());
+}
+
+std::string ChromeConfigurator::GetChannel() const {
+  return ChromeOmahaQueryParamsDelegate::GetChannelString();
+}
+
+std::string ChromeConfigurator::GetLang() const {
+  return ChromeOmahaQueryParamsDelegate::GetLang();
+}
+
+std::string ChromeConfigurator::GetOSLongName() const {
+  return chrome::VersionInfo().OSType();
+}
+
 std::string ChromeConfigurator::ExtraRequestParams() const {
   return extra_info_;
 }

diff --git a/chrome/browser/component_updater/component_updater_configurator.h b/chrome/browser/component_updater/component_updater_configurator.h
@@ -11,6 +11,7 @@ class GURL;
 
 namespace base {
 class CommandLine;
+class Version;
 }
 
 namespace net {
@@ -53,6 +54,21 @@ class Configurator {
   // pings are disabled.
   virtual GURL PingUrl() const = 0;
 
+  // Version of the application. Used to compare the component manifests.
+  virtual base::Version GetBrowserVersion() const = 0;
+
+  // Returns the value we use for the "updaterchannel=" and "prodchannel="
+  // parameters. Possible return values include: "canary", "dev", "beta", and
+  // "stable".
+  virtual std::string GetChannel() const = 0;
+
+  // Returns the language for the present locale. Possible return values are
+  // standard tags for languages, such as "en", "en-US", "de", "fr", "af", etc.
+  virtual std::string GetLang() const = 0;
+
+  // Returns the OS's long name like "Windows", "Mac OS X", etc.
+  virtual std::string GetOSLongName() const = 0;
+
   // Parameters added to each url request. It can be empty if none are needed.
   // The return string must be safe for insertion as an attribute in an
   // XML element.

diff --git a/chrome/browser/component_updater/component_updater_ping_manager.cc b/chrome/browser/component_updater/component_updater_ping_manager.cc
@@ -40,7 +40,7 @@ class PingSender : public net::URLFetcherDelegate {
  public:
   PingSender();
 
-  void SendPing(const GURL& ping_url,
+  void SendPing(const Configurator& config,
                 net::URLRequestContextGetter* url_request_context_getter,
                 const CrxUpdateItem* item);
 
@@ -50,7 +50,8 @@ class PingSender : public net::URLFetcherDelegate {
   // Overrides for URLFetcherDelegate.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
-  static std::string BuildPing(const CrxUpdateItem* item);
+  static std::string BuildPing(const Configurator& config,
+                               const CrxUpdateItem* item);
   static std::string BuildDownloadCompleteEventElements(
       const CrxUpdateItem* item);
   static std::string BuildUpdateCompleteEventElement(const CrxUpdateItem* item);
@@ -71,20 +72,23 @@ void PingSender::OnURLFetchComplete(const net::URLFetcher* source) {
 }
 
 void PingSender::SendPing(
-    const GURL& ping_url,
+    const Configurator& config,
     net::URLRequestContextGetter* url_request_context_getter,
     const CrxUpdateItem* item) {
   DCHECK(item);
 
-  if (!ping_url.is_valid())
+  if (!config.PingUrl().is_valid())
     return;
 
-  url_fetcher_.reset(SendProtocolRequest(
-      ping_url, BuildPing(item), this, url_request_context_getter));
+  url_fetcher_.reset(SendProtocolRequest(config.PingUrl(),
+                                         BuildPing(config, item),
+                                         this,
+                                         url_request_context_getter));
 }
 
 // Builds a ping message for the specified update item.
-std::string PingSender::BuildPing(const CrxUpdateItem* item) {
+std::string PingSender::BuildPing(const Configurator& config,
+                                  const CrxUpdateItem* item) {
   const char app_element_format[] =
       "<app appid=\"%s\" version=\"%s\" nextversion=\"%s\">"
       "%s"
@@ -98,7 +102,12 @@ std::string PingSender::BuildPing(const CrxUpdateItem* item) {
       BuildUpdateCompleteEventElement(item).c_str(),       // update event
       BuildDownloadCompleteEventElements(item).c_str()));  // download events
 
-  return BuildProtocolRequest(app_element, "");
+  return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
+                              config.GetChannel(),
+                              config.GetLang(),
+                              config.GetOSLongName(),
+                              app_element,
+                              "");
 }
 
 // Returns a string representing a sequence of download complete events
@@ -190,7 +199,7 @@ PingManager::~PingManager() {
 // sender object self-deletes after sending the ping.
 void PingManager::OnUpdateComplete(const CrxUpdateItem* item) {
   PingSender* ping_sender(new PingSender);
-  ping_sender->SendPing(config_.PingUrl(), config_.RequestContext(), item);
+  ping_sender->SendPing(config_, config_.RequestContext(), item);
 }
 
 }  // namespace component_updater
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
@@ -29,7 +29,6 @@
 #include "chrome/browser/component_updater/crx_update_item.h"
 #include "chrome/browser/component_updater/update_checker.h"
 #include "chrome/browser/component_updater/update_response.h"
-#include "chrome/common/chrome_version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_throttle.h"
@@ -263,8 +262,6 @@ class CrxUpdateService : public ComponentUpdateService, public OnDemandUpdater {
 
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
 
-  const Version chrome_version_;
-
   bool running_;
 
   ObserverList<Observer> observer_list_;
@@ -282,7 +279,6 @@ CrxUpdateService::CrxUpdateService(Configurator* config)
               GetSequencedTaskRunnerWithShutdownBehavior(
                   BrowserThread::GetBlockingPool()->GetSequenceToken(),
                   base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)),
-      chrome_version_(chrome::VersionInfo().Version()),
       running_(false) {
 }
 
@@ -720,7 +716,8 @@ void CrxUpdateService::OnUpdateCheckSucceeded(
     }
 
     if (!it->manifest.browser_min_version.empty()) {
-      if (IsVersionNewer(chrome_version_, it->manifest.browser_min_version)) {
+      if (IsVersionNewer(config_->GetBrowserVersion(),
+                         it->manifest.browser_min_version)) {
         // The component is not compatible with this Chrome version.
         VLOG(1) << "Ignoring incompatible component: " << crx->id;
         ChangeItemState(crx, CrxUpdateItem::kNoUpdate);

diff --git 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,14 @@ int GetPhysicalMemoryGB() {
 
 }  // namespace
 
-std::string BuildProtocolRequest(const std::string& request_body,
+std::string BuildProtocolRequest(const std::string& browser_version,
+                                 const std::string& channel,
+                                 const std::string& lang,
+                                 const std::string& os_long_name,
+                                 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());
 
   std::string request(
       "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
@@ -63,8 +62,8 @@ 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"
+      browser_version.c_str(),           // "version"
+      browser_version.c_str(),           // "prodversion"
       base::GenerateGUID().c_str(),      // "requestid"
       lang.c_str(),                      // "lang",
       channel.c_str(),                   // "updaterchannel"
@@ -89,7 +88,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"
+      os_long_name.c_str(),                                    // "platform"
       base::SysInfo().OperatingSystemVersion().c_str(),        // "version"
       base::SysInfo().OperatingSystemArchitecture().c_str());  // "arch"
 

diff --git a/chrome/browser/component_updater/component_updater_utils.h b/chrome/browser/component_updater/component_updater_utils.h
@@ -22,6 +22,7 @@ class URLRequestContextGetter;
 
 namespace component_updater {
 
+class Configurator;
 struct CrxUpdateItem;
 
 // An update protocol request starts with a common preamble which includes
@@ -44,7 +45,11 @@ struct CrxUpdateItem;
 // If specified, |additional_attributes| are appended as attributes of the
 // request element. The additional attributes have to be well-formed for
 // insertion in the request element.
-std::string BuildProtocolRequest(const std::string& request_body,
+std::string BuildProtocolRequest(const std::string& browser_version,
+                                 const std::string& channel,
+                                 const std::string& lang,
+                                 const std::string& os_long_name,
+                                 const std::string& request_body,
                                  const std::string& additional_attributes);
 
 // Sends a protocol request to the the service endpoint specified by |url|.

diff --git a/chrome/browser/component_updater/sw_reporter_installer_win.cc b/chrome/browser/component_updater/sw_reporter_installer_win.cc
@@ -28,8 +28,8 @@
 #include "chrome/browser/component_updater/component_updater_service.h"
 #include "chrome/browser/component_updater/component_updater_utils.h"
 #include "chrome/browser/component_updater/default_component_installer.h"
-#include "chrome/common/chrome_paths.h"
-#include "chrome/common/pref_names.h"
+#include "components/component_updater/component_updater_paths.h"
+#include "components/component_updater/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
@@ -162,8 +162,8 @@ class SwReporterInstallerTraits : public ComponentInstallerTraits {
     // The base directory on windows looks like:
     // <profile>\AppData\Local\Google\Chrome\User Data\SwReporter\.
     base::FilePath result;
-    PathService::Get(chrome::DIR_USER_DATA, &result);
-    return result.Append(FILE_PATH_LITERAL("SwReporter"));
+    PathService::Get(DIR_SW_REPORTER, &result);
+    return result;
   }
 
   static std::string ID() {

diff --git a/chrome/browser/component_updater/test/test_configurator.cc b/chrome/browser/component_updater/test/test_configurator.cc
@@ -5,6 +5,7 @@
 #include <string>
 
 #include "base/run_loop.h"
+#include "base/version.h"
 #include "chrome/browser/component_updater/test/test_configurator.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
@@ -66,6 +67,23 @@ GURL TestConfigurator::PingUrl() const {
   return UpdateUrl();
 }
 
+base::Version TestConfigurator::GetBrowserVersion() const {
+  // Needs to be larger than the required version in tested component manifests.
+  return base::Version("30.0");
+}
+
+std::string TestConfigurator::GetChannel() const {
+  return "fake_channel_string";
+}
+
+std::string TestConfigurator::GetLang() const {
+  return "fake_lang";
+}
+
+std::string TestConfigurator::GetOSLongName() const {
+  return "Fake Operating System";
+}
+
 std::string TestConfigurator::ExtraRequestParams() const {
   return "extra=\"foo\"";
 }

diff --git a/chrome/browser/component_updater/test/test_configurator.h b/chrome/browser/component_updater/test/test_configurator.h
@@ -37,6 +37,10 @@ class TestConfigurator : public Configurator {
   virtual int OnDemandDelay() const OVERRIDE;
   virtual GURL UpdateUrl() const OVERRIDE;
   virtual GURL PingUrl() const OVERRIDE;
+  virtual base::Version GetBrowserVersion() const OVERRIDE;
+  virtual std::string GetChannel() const OVERRIDE;
+  virtual std::string GetLang() const OVERRIDE;
+  virtual std::string GetOSLongName() const OVERRIDE;
   virtual std::string ExtraRequestParams() const OVERRIDE;
   virtual size_t UrlSizeLimit() const OVERRIDE;
   virtual net::URLRequestContextGetter* RequestContext() const OVERRIDE;

diff --git a/chrome/browser/component_updater/update_checker.cc b/chrome/browser/component_updater/update_checker.cc
@@ -33,7 +33,8 @@ namespace component_updater {
 //        <package fp="abcd" />
 //      </packages>
 //    </app>
-std::string BuildUpdateCheckRequest(const std::vector<CrxUpdateItem*>& items,
+std::string BuildUpdateCheckRequest(const Configurator& config,
+                                    const std::vector<CrxUpdateItem*>& items,
                                     const std::string& additional_attributes) {
   std::string app_elements;
   for (size_t i = 0; i != items.size(); ++i) {
@@ -59,7 +60,12 @@ std::string BuildUpdateCheckRequest(const std::vector<CrxUpdateItem*>& items,
     VLOG(1) << "Appending to update request: " << app;
   }
 
-  return BuildProtocolRequest(app_elements, additional_attributes);
+  return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
+                              config.GetChannel(),
+                              config.GetLang(),
+                              config.GetOSLongName(),
+                              app_elements,
+                              additional_attributes);
 }
 
 class UpdateCheckerImpl : public UpdateChecker, public net::URLFetcherDelegate {
@@ -114,7 +120,7 @@ bool UpdateCheckerImpl::CheckForUpdates(
 
   url_fetcher_.reset(SendProtocolRequest(
       config_.UpdateUrl(),
-      BuildUpdateCheckRequest(items_to_check, additional_attributes),
+      BuildUpdateCheckRequest(config_, items_to_check, additional_attributes),
       this,
       config_.RequestContext()));
 

diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
@@ -2836,7 +2836,6 @@
         '../components/components.gyp:captive_portal',
         '../components/components.gyp:cloud_devices_common',
         '../components/components.gyp:component_metrics_proto',
-        '../components/components.gyp:component_updater',
         '../components/components.gyp:data_reduction_proxy_browser',
         '../components/components.gyp:domain_reliability',
         '../components/components.gyp:favicon_base',
@@ -2928,6 +2927,7 @@
             '../third_party/re2/re2.gyp:re2',
             '../cc/cc.gyp:cc',
             '../components/components.gyp:autofill_content_browser',
+            '../components/components.gyp:component_updater',
             '../components/components.gyp:dom_distiller_content',
             '../components/components.gyp:keyed_service_content',
             '../components/components.gyp:navigation_interception',

diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
@@ -2099,13 +2099,6 @@ const char kDevicePolicyRefreshRate[] = "policy.device_refresh_rate";
 const char kAttemptedToEnableAutoupdate[] =
     "browser.attempted_to_enable_autoupdate";
 
-#if defined(OS_WIN)
-// The number of attempts left to execute the SwReporter. This starts at the max
-// number of retries allowed, and goes down as attempts are made and is cleared
-// back to 0 when it successfully completes.
-const char kSwReporterExecuteTryCount[] = "software_reporter.execute_try_count";
-#endif
-
 // The next media gallery ID to assign.
 const char kMediaGalleriesUniqueId[] = "media_galleries.gallery_id";
 

diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
@@ -729,10 +729,6 @@ extern const char kMessageCenterForcedOnTaskbar[];
 
 extern const char kAttemptedToEnableAutoupdate[];
 
-#if defined(OS_WIN)
-extern const char kSwReporterExecuteTryCount[];
-#endif
-
 extern const char kMediaGalleriesUniqueId[];
 extern const char kMediaGalleriesRememberedGalleries[];
 extern const char kMediaGalleriesLastScanTime[];