From 6a8ab1dceba32f021f112ce62c8ac80d70e658c3 Mon Sep 17 00:00:00 2001 From: "tommycli@chromium.org" Date: Thu, 10 Jul 2014 22:47:39 +0000 Subject: [PATCH] Componentize component_updater: Use Configurator to build query parameters. 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 --- .../component_updater_configurator.cc | 23 ++++++++++++++++ .../component_updater_configurator.h | 16 +++++++++++ .../component_updater_ping_manager.cc | 27 ++++++++++++------- .../component_updater_service.cc | 7 ++--- .../component_updater_utils.cc | 19 +++++++------ .../component_updater_utils.h | 7 ++++- .../sw_reporter_installer_win.cc | 8 +++--- .../test/test_configurator.cc | 18 +++++++++++++ .../test/test_configurator.h | 4 +++ .../component_updater/update_checker.cc | 12 ++++++--- chrome/chrome_browser.gypi | 2 +- chrome/common/pref_names.cc | 7 ----- chrome/common/pref_names.h | 4 --- .../component_updater_paths.cc | 3 +++ .../component_updater_paths.h | 1 + components/component_updater/pref_names.cc | 7 +++++ components/component_updater/pref_names.h | 6 +++++ 17 files changed, 127 insertions(+), 44 deletions(-) diff --git a/chrome/browser/component_updater/component_updater_configurator.cc b/chrome/browser/component_updater/component_updater_configurator.cc index f9027e782e6688..5e8d04712fcbd1 100644 --- 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 index 9cb73bd836083b..49fbf93f267f21 100644 --- 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 index 27951f0fc40af8..eb98f65d50cd03 100644 --- 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[] = "" "%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 index 50efcb670c329e..3fa2aca335d12f 100644 --- 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 blocking_task_runner_; - const Version chrome_version_; - bool running_; ObserverList 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 index d7ff01b4996023..199117187f8c35 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,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( "" @@ -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, "", - 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 index 7c238d28ed8dc8..b4b29432ec810d 100644 --- 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 index 513362f08f8590..578a70c0f7caea 100644 --- 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: // \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 index 8cf49d26f183fe..1bc3add41fd196 100644 --- a/chrome/browser/component_updater/test/test_configurator.cc +++ b/chrome/browser/component_updater/test/test_configurator.cc @@ -5,6 +5,7 @@ #include #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 index 5e8121b02f3e04..bf7b8681771885 100644 --- 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 index 2a030128b0d3cd..7fa828be2fadaf 100644 --- a/chrome/browser/component_updater/update_checker.cc +++ b/chrome/browser/component_updater/update_checker.cc @@ -33,7 +33,8 @@ namespace component_updater { // // // -std::string BuildUpdateCheckRequest(const std::vector& items, +std::string BuildUpdateCheckRequest(const Configurator& config, + const std::vector& 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& 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 index 784fb33c040f58..ec3faa6e185ea5 100644 --- 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 index 565beb95a23539..08636ed7af558a 100644 --- 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 index d336f658f9f815..d78ab487337a25 100644 --- 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[]; diff --git a/components/component_updater/component_updater_paths.cc b/components/component_updater/component_updater_paths.cc index 62f484b6afc2c0..bf9f4d39afb3fd 100644 --- a/components/component_updater/component_updater_paths.cc +++ b/components/component_updater/component_updater_paths.cc @@ -29,6 +29,9 @@ bool PathProvider(int key, base::FilePath* result) { case DIR_SWIFT_SHADER: cur = cur.Append(FILE_PATH_LITERAL("SwiftShader")); break; + case DIR_SW_REPORTER: + cur = cur.Append(FILE_PATH_LITERAL("SwReporter")); + break; default: return false; } diff --git a/components/component_updater/component_updater_paths.h b/components/component_updater/component_updater_paths.h index b25189567e240e..c69aec08d70903 100644 --- a/components/component_updater/component_updater_paths.h +++ b/components/component_updater/component_updater_paths.h @@ -16,6 +16,7 @@ enum { DIR_RECOVERY_BASE, // Full path to the dir for Recovery // component. DIR_SWIFT_SHADER, // Path to the SwiftShader component. + DIR_SW_REPORTER, // Path to the SwReporter component. PATH_END }; diff --git a/components/component_updater/pref_names.cc b/components/component_updater/pref_names.cc index 389100eedc6cd6..8306bae56ef229 100644 --- a/components/component_updater/pref_names.cc +++ b/components/component_updater/pref_names.cc @@ -10,4 +10,11 @@ namespace prefs { // takes the usual 'a.b.c.d' notation. const char kRecoveryComponentVersion[] = "recovery_component.version"; +#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 + } // namespace prefs diff --git a/components/component_updater/pref_names.h b/components/component_updater/pref_names.h index aedc2b1adc2e6b..2e11c222b43c43 100644 --- a/components/component_updater/pref_names.h +++ b/components/component_updater/pref_names.h @@ -5,10 +5,16 @@ #ifndef COMPONENTS_COMPONENT_UPDATER_PREF_NAMES_H_ #define COMPONENTS_COMPONENT_UPDATER_PREF_NAMES_H_ +#include "build/build_config.h" + namespace prefs { extern const char kRecoveryComponentVersion[]; +#if defined(OS_WIN) +extern const char kSwReporterExecuteTryCount[]; +#endif + } // namespace prefs #endif // COMPONENTS_COMPONENT_UPDATER_PREF_NAMES_H_