diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-08-24 12:15:48 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-08-28 13:30:04 +0000 |
| commit | b014812705fc80bff0a5c120dfcef88f349816dc (patch) | |
| tree | 25a2e2d9fa285f1add86aa333389a839f81a39ae /chromium/components/update_client | |
| parent | 9f4560b1027ae06fdb497023cdcaf91b8511fa74 (diff) | |
| download | qtwebengine-chromium-b014812705fc80bff0a5c120dfcef88f349816dc.tar.gz | |
BASELINE: Update Chromium to 68.0.3440.125
Change-Id: I23f19369e01f688e496f5bf179abb521ad73874f
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/components/update_client')
28 files changed, 925 insertions, 576 deletions
diff --git a/chromium/components/update_client/BUILD.gn b/chromium/components/update_client/BUILD.gn index 7d394a0fbcd..a105b0414ae 100644 --- a/chromium/components/update_client/BUILD.gn +++ b/chromium/components/update_client/BUILD.gn @@ -12,6 +12,8 @@ static_library("update_client") { "activity_data_service.h", "background_downloader_win.cc", "background_downloader_win.h", + "command_line_config_policy.cc", + "command_line_config_policy.h", "component.cc", "component.h", "component_patcher.cc", diff --git a/chromium/components/update_client/action_runner.cc b/chromium/components/update_client/action_runner.cc index c91388ac6e7..8cfb13c448a 100644 --- a/chromium/components/update_client/action_runner.cc +++ b/chromium/components/update_client/action_runner.cc @@ -44,7 +44,7 @@ void ActionRunner::Run(Callback run_complete) { void ActionRunner::Unpack( std::unique_ptr<service_manager::Connector> connector) { - const auto& installer = component_.crx_component().installer; + const auto& installer = component_.crx_component()->installer; base::FilePath file_path; installer->GetInstalledFile(component_.action_run(), &file_path); diff --git a/chromium/components/update_client/background_downloader_win.cc b/chromium/components/update_client/background_downloader_win.cc index a83f9b94884..fb8c9ed1591 100644 --- a/chromium/components/update_client/background_downloader_win.cc +++ b/chromium/components/update_client/background_downloader_win.cc @@ -719,9 +719,11 @@ HRESULT BackgroundDownloader::QueueBitsJob(const GURL& url, HRESULT BackgroundDownloader::CreateOrOpenJob(const GURL& url, ComPtr<IBackgroundCopyJob>* job) { std::vector<ComPtr<IBackgroundCopyJob>> jobs; - HRESULT hr = - FindBitsJobIf(std::bind2nd(std::ptr_fun(JobFileUrlEqualPredicate), url), - bits_manager_, &jobs); + HRESULT hr = FindBitsJobIf( + [&url](ComPtr<IBackgroundCopyJob> job) { + return JobFileUrlEqualPredicate(job, url); + }, + bits_manager_, &jobs); if (SUCCEEDED(hr) && !jobs.empty()) { *job = jobs.front(); return S_FALSE; @@ -899,9 +901,11 @@ void BackgroundDownloader::CleanupStaleJobs() { last_sweep = current_time; std::vector<ComPtr<IBackgroundCopyJob>> jobs; - FindBitsJobIf(std::bind2nd(std::ptr_fun(JobCreationOlderThanDaysPredicate), - kPurgeStaleJobsAfterDays), - bits_manager_, &jobs); + FindBitsJobIf( + [](ComPtr<IBackgroundCopyJob> job) { + return JobCreationOlderThanDaysPredicate(job, kPurgeStaleJobsAfterDays); + }, + bits_manager_, &jobs); for (const auto& job : jobs) CleanupJob(job); diff --git a/chromium/components/update_client/command_line_config_policy.cc b/chromium/components/update_client/command_line_config_policy.cc new file mode 100644 index 00000000000..a6456019b22 --- /dev/null +++ b/chromium/components/update_client/command_line_config_policy.cc @@ -0,0 +1,40 @@ +// Copyright 2018 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 "components/update_client/command_line_config_policy.h" + +#include "build/build_config.h" +#include "url/gurl.h" + +namespace update_client { + +bool CommandLineConfigPolicy::BackgroundDownloadsEnabled() const { +#if defined(OS_WIN) + return true; +#else + return false; +#endif +} + +bool CommandLineConfigPolicy::DeltaUpdatesEnabled() const { + return true; +} + +bool CommandLineConfigPolicy::FastUpdate() const { + return false; +} + +bool CommandLineConfigPolicy::PingsEnabled() const { + return true; +} + +bool CommandLineConfigPolicy::TestRequest() const { + return false; +} + +GURL CommandLineConfigPolicy::UrlSourceOverride() const { + return GURL(); +} + +} // namespace update_client diff --git a/chromium/components/update_client/command_line_config_policy.h b/chromium/components/update_client/command_line_config_policy.h new file mode 100644 index 00000000000..d515c994180 --- /dev/null +++ b/chromium/components/update_client/command_line_config_policy.h @@ -0,0 +1,40 @@ +// Copyright 2018 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. + +#ifndef COMPONENTS_UPDATE_CLIENT_COMMAND_LINE_CONFIG_POLICY_H_ +#define COMPONENTS_UPDATE_CLIENT_COMMAND_LINE_CONFIG_POLICY_H_ + +class GURL; + +namespace update_client { + +// This class provides additional settings from command line switches to the +// main configurator. +class CommandLineConfigPolicy { + public: + // If true, background downloads are enabled. + virtual bool BackgroundDownloadsEnabled() const; + + // If true, differential updates are enabled. + virtual bool DeltaUpdatesEnabled() const; + + // If true, speed up the initial update checking. + virtual bool FastUpdate() const; + + // If true, pings are enabled. Pings are the requests sent to the update + // server that report the success or failure of installs or update attempts. + virtual bool PingsEnabled() const; + + // If true, add "testrequest" attribute to update check requests. + virtual bool TestRequest() const; + + // The override URL for updates. Can be empty. + virtual GURL UrlSourceOverride() const; + + virtual ~CommandLineConfigPolicy() {} +}; + +} // namespace update_client + +#endif // COMPONENTS_UPDATE_CLIENT_COMMAND_LINE_CONFIG_POLICY_H_ diff --git a/chromium/components/update_client/component.cc b/chromium/components/update_client/component.cc index cbed9b77538..6c8ce2c8bcb 100644 --- a/chromium/components/update_client/component.cc +++ b/chromium/components/update_client/component.cc @@ -27,12 +27,12 @@ // The state machine representing how a CRX component changes during an update. // -// +------------------------> kNew <---------------------+--------+ -// | | | | -// | V | | -// | kChecking | | -// | | | | -// | error V no no | | +// +------------------------- kNew +// | | +// | V +// | kChecking +// | | +// V error V no no // kUpdateError <------------- [update?] -> [action?] -> kUpToDate kUpdated // ^ | | ^ ^ // | yes | | yes | | @@ -62,7 +62,7 @@ namespace update_client { namespace { using InstallOnBlockingTaskRunnerCompleteCallback = base::OnceCallback< - void(int error_category, int error_code, int extra_code1)>; + void(ErrorCategory error_category, int error_code, int extra_code1)>; void InstallComplete( scoped_refptr<base::SingleThreadTaskRunner> main_task_runner, @@ -78,11 +78,9 @@ void InstallComplete( const CrxInstaller::Result& result) { base::DeleteFile(unpack_path, true); const ErrorCategory error_category = - result.error ? ErrorCategory::kInstallError - : ErrorCategory::kErrorNone; + result.error ? ErrorCategory::kInstall : ErrorCategory::kNone; main_task_runner->PostTask( - FROM_HERE, base::BindOnce(std::move(callback), - static_cast<int>(error_category), + FROM_HERE, base::BindOnce(std::move(callback), error_category, static_cast<int>(result.error), result.extended_error)); }, @@ -109,8 +107,7 @@ void InstallOnBlockingTaskRunner( const CrxInstaller::Result result(InstallError::FINGERPRINT_WRITE_FAILED); main_task_runner->PostTask( FROM_HERE, - base::BindOnce(std::move(callback), - static_cast<int>(ErrorCategory::kInstallError), + base::BindOnce(std::move(callback), ErrorCategory::kInstall, static_cast<int>(result.error), result.extended_error)); return; } @@ -133,8 +130,7 @@ void UnpackCompleteOnBlockingTaskRunner( if (result.error != UnpackerError::kNone) { main_task_runner->PostTask( FROM_HERE, - base::BindOnce(std::move(callback), - static_cast<int>(ErrorCategory::kUnpackError), + base::BindOnce(std::move(callback), ErrorCategory::kUnpack, static_cast<int>(result.error), result.extended_error)); return; } @@ -212,10 +208,14 @@ CrxUpdateItem Component::GetCrxUpdateItem() const { CrxUpdateItem crx_update_item; crx_update_item.state = state_->state(); crx_update_item.id = id_; - crx_update_item.component = crx_component_; + if (crx_component_) + crx_update_item.component = *crx_component_; crx_update_item.last_check = last_check_; crx_update_item.next_version = next_version_; crx_update_item.next_fp = next_fp_; + crx_update_item.error_category = error_category_; + crx_update_item.error_code = error_code_; + crx_update_item.extra_code1 = extra_code1_; return crx_update_item; } @@ -256,6 +256,9 @@ void Component::Uninstall(const base::Version& version, int reason) { DCHECK_EQ(ComponentState::kNew, state()); + crx_component_ = std::make_unique<CrxComponent>(); + crx_component_->version = version; + previous_version_ = version; next_version_ = base::Version("0"); extra_code1_ = reason; @@ -274,7 +277,8 @@ void Component::UpdateCheckComplete() { bool Component::CanDoBackgroundDownload() const { // Foreground component updates are always downloaded in foreground. - return !is_foreground() && crx_component_.allows_background_download && + return !is_foreground() && + (crx_component() && crx_component()->allows_background_download) && update_context_.config->EnabledBackgroundDownloader(); } @@ -340,8 +344,13 @@ void Component::StateNew::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); - - TransitionState(std::make_unique<StateChecking>(&component)); + if (component.crx_component()) { + TransitionState(std::make_unique<StateChecking>(&component)); + } else { + component.error_code_ = static_cast<int>(Error::CRX_NOT_FOUND); + component.error_category_ = ErrorCategory::kService; + TransitionState(std::make_unique<StateUpdateError>(&component)); + } } Component::StateChecking::StateChecking(Component* component) @@ -361,6 +370,7 @@ void Component::StateChecking::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); + DCHECK(component.crx_component()); component.last_check_ = base::TimeTicks::Now(); component.update_check_complete_ = base::BindOnce( @@ -372,7 +382,7 @@ void Component::StateChecking::DoHandle() { void Component::StateChecking::UpdateCheckComplete() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); - if (!component.update_check_error_) { + if (!component.error_code_) { if (component.status_ == "ok") { TransitionState(std::make_unique<StateCanUpdate>(&component)); return; @@ -421,13 +431,15 @@ void Component::StateCanUpdate::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); + DCHECK(component.crx_component()); component.is_update_available_ = true; component.NotifyObservers(Events::COMPONENT_UPDATE_FOUND); - if (component.crx_component_.supports_group_policy_enable_component_updates && + if (component.crx_component() + ->supports_group_policy_enable_component_updates && !component.update_context_.enabled_component_updates) { - component.error_category_ = static_cast<int>(ErrorCategory::kServiceError); + component.error_category_ = ErrorCategory::kService; component.error_code_ = static_cast<int>(ServiceError::UPDATE_DISABLED); component.extra_code1_ = 0; TransitionState(std::make_unique<StateUpdateError>(&component)); @@ -462,6 +474,7 @@ void Component::StateUpToDate::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); + DCHECK(component.crx_component()); component.NotifyObservers(Events::COMPONENT_NOT_UPDATED); EndState(); @@ -480,6 +493,8 @@ void Component::StateDownloadingDiff::DoHandle() { const auto& component = Component::State::component(); const auto& update_context = component.update_context_; + DCHECK(component.crx_component()); + crx_downloader_ = update_context.crx_downloader_factory( component.CanDoBackgroundDownload(), update_context.config->RequestContext()); @@ -522,8 +537,7 @@ void Component::StateDownloadingDiff::DownloadComplete( if (download_result.error) { DCHECK(download_result.response.empty()); - component.diff_error_category_ = - static_cast<int>(ErrorCategory::kNetworkError); + component.diff_error_category_ = ErrorCategory::kDownload; component.diff_error_code_ = download_result.error; TransitionState(std::make_unique<StateDownloading>(&component)); @@ -548,6 +562,8 @@ void Component::StateDownloading::DoHandle() { const auto& component = Component::State::component(); const auto& update_context = component.update_context_; + DCHECK(component.crx_component()); + crx_downloader_ = update_context.crx_downloader_factory( component.CanDoBackgroundDownload(), update_context.config->RequestContext()); @@ -590,7 +606,7 @@ void Component::StateDownloading::DownloadComplete( if (download_result.error) { DCHECK(download_result.response.empty()); - component.error_category_ = static_cast<int>(ErrorCategory::kNetworkError); + component.error_category_ = ErrorCategory::kDownload; component.error_code_ = download_result.error; TransitionState(std::make_unique<StateUpdateError>(&component)); @@ -615,6 +631,8 @@ void Component::StateUpdatingDiff::DoHandle() { const auto& component = Component::State::component(); const auto& update_context = component.update_context_; + DCHECK(component.crx_component()); + component.NotifyObservers(Events::COMPONENT_UPDATE_READY); // Create a fresh connector that can be used on the other task runner. @@ -627,14 +645,14 @@ void Component::StateUpdatingDiff::DoHandle() { base::BindOnce( &update_client::StartInstallOnBlockingTaskRunner, base::ThreadTaskRunnerHandle::Get(), - component.crx_component_.pk_hash, component.crx_path_, - component.next_fp_, component.crx_component_.installer, + component.crx_component()->pk_hash, component.crx_path_, + component.next_fp_, component.crx_component()->installer, std::move(connector), base::BindOnce(&Component::StateUpdatingDiff::InstallComplete, base::Unretained(this)))); } -void Component::StateUpdatingDiff::InstallComplete(int error_category, +void Component::StateUpdatingDiff::InstallComplete(ErrorCategory error_category, int error_code, int extra_code1) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -650,13 +668,11 @@ void Component::StateUpdatingDiff::InstallComplete(int error_category, return; } - DCHECK_EQ(static_cast<int>(ErrorCategory::kErrorNone), - component.diff_error_category_); + DCHECK_EQ(ErrorCategory::kNone, component.diff_error_category_); DCHECK_EQ(0, component.diff_error_code_); DCHECK_EQ(0, component.diff_extra_code1_); - DCHECK_EQ(static_cast<int>(ErrorCategory::kErrorNone), - component.error_category_); + DCHECK_EQ(ErrorCategory::kNone, component.error_category_); DCHECK_EQ(0, component.error_code_); DCHECK_EQ(0, component.extra_code1_); @@ -679,6 +695,8 @@ void Component::StateUpdating::DoHandle() { const auto& component = Component::State::component(); const auto& update_context = component.update_context_; + DCHECK(component.crx_component()); + component.NotifyObservers(Events::COMPONENT_UPDATE_READY); // Create a fresh connector that can be used on the other task runner. @@ -690,14 +708,14 @@ void Component::StateUpdating::DoHandle() { base::BindOnce( &update_client::StartInstallOnBlockingTaskRunner, base::ThreadTaskRunnerHandle::Get(), - component.crx_component_.pk_hash, component.crx_path_, - component.next_fp_, component.crx_component_.installer, + component.crx_component()->pk_hash, component.crx_path_, + component.next_fp_, component.crx_component()->installer, std::move(connector), base::BindOnce(&Component::StateUpdating::InstallComplete, base::Unretained(this)))); } -void Component::StateUpdating::InstallComplete(int error_category, +void Component::StateUpdating::InstallComplete(ErrorCategory error_category, int error_code, int extra_code1) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -713,8 +731,7 @@ void Component::StateUpdating::InstallComplete(int error_category, return; } - DCHECK_EQ(static_cast<int>(ErrorCategory::kErrorNone), - component.error_category_); + DCHECK_EQ(ErrorCategory::kNone, component.error_category_); DCHECK_EQ(0, component.error_code_); DCHECK_EQ(0, component.extra_code1_); @@ -737,8 +754,10 @@ void Component::StateUpdated::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); - component.crx_component_.version = component.next_version_; - component.crx_component_.fingerprint = component.next_fp_; + DCHECK(component.crx_component()); + + component.crx_component_->version = component.next_version_; + component.crx_component_->fingerprint = component.next_fp_; component.AppendEvent(BuildUpdateCompleteEventElement(component)); @@ -759,6 +778,8 @@ void Component::StateUninstalled::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); auto& component = State::component(); + DCHECK(component.crx_component()); + component.AppendEvent(BuildUninstalledEventElement(component)); EndState(); @@ -775,6 +796,8 @@ void Component::StateRun::DoHandle() { DCHECK(thread_checker_.CalledOnValidThread()); const auto& component = State::component(); + DCHECK(component.crx_component()); + action_runner_ = std::make_unique<ActionRunner>(component); action_runner_->Run( base::BindOnce(&StateRun::ActionRunComplete, base::Unretained(this))); diff --git a/chromium/components/update_client/component.h b/chromium/components/update_client/component.h index ff52638f4fb..749e3009bba 100644 --- a/chromium/components/update_client/component.h +++ b/chromium/components/update_client/component.h @@ -72,9 +72,9 @@ class Component { std::string id() const { return id_; } - const CrxComponent& crx_component() const { return crx_component_; } - void set_crx_component(const CrxComponent& crx_component) { - crx_component_ = crx_component; + const CrxComponent* crx_component() const { return crx_component_.get(); } + void set_crx_component(std::unique_ptr<CrxComponent> crx_component) { + crx_component_ = std::move(crx_component); } const base::Version& previous_version() const { return previous_version_; } @@ -92,9 +92,10 @@ class Component { std::string next_fp() const { return next_fp_; } void set_next_fp(const std::string& next_fp) { next_fp_ = next_fp; } - int update_check_error() const { return update_check_error_; } void set_update_check_error(int update_check_error) { - update_check_error_ = update_check_error; + error_category_ = ErrorCategory::kUpdateCheck; + error_code_ = update_check_error; + extra_code1_ = 0; } bool is_foreground() const; @@ -105,10 +106,10 @@ class Component { bool diff_update_failed() const { return !!diff_error_code_; } - int error_category() const { return error_category_; } + ErrorCategory error_category() const { return error_category_; } int error_code() const { return error_code_; } int extra_code1() const { return extra_code1_; } - int diff_error_category() const { return diff_error_category_; } + ErrorCategory diff_error_category() const { return diff_error_category_; } int diff_error_code() const { return diff_error_code_; } int diff_extra_code1() const { return diff_extra_code1_; } @@ -292,7 +293,9 @@ class Component { // State overrides. void DoHandle() override; - void InstallComplete(int error_category, int error_code, int extra_code1); + void InstallComplete(ErrorCategory error_category, + int error_code, + int extra_code1); DISALLOW_COPY_AND_ASSIGN(StateUpdatingDiff); }; @@ -306,7 +309,9 @@ class Component { // State overrides. void DoHandle() override; - void InstallComplete(int error_category, int error_code, int extra_code1); + void InstallComplete(ErrorCategory error_category, + int error_code, + int extra_code1); DISALLOW_COPY_AND_ASSIGN(StateUpdating); }; @@ -369,7 +374,7 @@ class Component { base::ThreadChecker thread_checker_; const std::string id_; - CrxComponent crx_component_; + std::unique_ptr<CrxComponent> crx_component_; // The status of the updatecheck response. std::string status_; @@ -412,10 +417,10 @@ class Component { // the |extra_code1| usually contains a system error, but it can contain // any extended information that is relevant to either the category or the // error itself. - int error_category_ = 0; + ErrorCategory error_category_ = ErrorCategory::kNone; int error_code_ = 0; int extra_code1_ = 0; - int diff_error_category_ = 0; + ErrorCategory diff_error_category_ = ErrorCategory::kNone; int diff_error_code_ = 0; int diff_extra_code1_ = 0; diff --git a/chromium/components/update_client/component_patcher_unittest.cc b/chromium/components/update_client/component_patcher_unittest.cc index 2ec97786982..1a530d8bda0 100644 --- a/chromium/components/update_client/component_patcher_unittest.cc +++ b/chromium/components/update_client/component_patcher_unittest.cc @@ -51,7 +51,7 @@ void TestCallback::Set(update_client::UnpackerError error, int extra_code) { base::FilePath test_file(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components") .AppendASCII("test") .AppendASCII("data") diff --git a/chromium/components/update_client/component_unpacker_unittest.cc b/chromium/components/update_client/component_unpacker_unittest.cc index 221ba8df603..910bb4d99b8 100644 --- a/chromium/components/update_client/component_unpacker_unittest.cc +++ b/chromium/components/update_client/component_unpacker_unittest.cc @@ -15,6 +15,7 @@ #include "base/memory/ref_counted.h" #include "base/path_service.h" #include "base/run_loop.h" +#include "base/single_thread_task_runner.h" #include "base/task_scheduler/post_task.h" #include "base/test/scoped_task_environment.h" #include "base/threading/thread_task_runner_handle.h" @@ -53,7 +54,7 @@ void TestCallback::Set(update_client::UnpackerError error, int extra_code) { base::FilePath test_file(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components") .AppendASCII("test") .AppendASCII("data") diff --git a/chromium/components/update_client/crx_downloader_unittest.cc b/chromium/components/update_client/crx_downloader_unittest.cc index 7dec8b1e898..cb5c80842f9 100644 --- a/chromium/components/update_client/crx_downloader_unittest.cc +++ b/chromium/components/update_client/crx_downloader_unittest.cc @@ -11,7 +11,6 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop.h" #include "base/path_service.h" #include "base/run_loop.h" #include "base/test/scoped_task_environment.h" @@ -40,7 +39,7 @@ const char hash_jebg[] = base::FilePath MakeTestFilePath(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components/test/data/update_client") .AppendASCII(file); } diff --git a/chromium/components/update_client/crx_update_item.h b/chromium/components/update_client/crx_update_item.h index 2882a8421c9..9fd73988a7e 100644 --- a/chromium/components/update_client/crx_update_item.h +++ b/chromium/components/update_client/crx_update_item.h @@ -15,6 +15,7 @@ #include "base/version.h" #include "components/update_client/crx_downloader.h" #include "components/update_client/update_client.h" +#include "components/update_client/update_client_errors.h" namespace update_client { @@ -33,6 +34,10 @@ struct CrxUpdateItem { base::Version next_version; std::string next_fp; + + ErrorCategory error_category = ErrorCategory::kNone; + int error_code = 0; + int extra_code1 = 0; }; } // namespace update_client diff --git a/chromium/components/update_client/ping_manager.cc b/chromium/components/update_client/ping_manager.cc index 7ec516eb402..f9b885b4eae 100644 --- a/chromium/components/update_client/ping_manager.cc +++ b/chromium/components/update_client/ping_manager.cc @@ -70,8 +70,10 @@ void PingSender::SendPing(const Component& component, Callback callback) { return; } + DCHECK(component.crx_component()); + auto urls(config_->PingUrl()); - if (component.crx_component().requires_network_encryption) + if (component.crx_component()->requires_network_encryption) RemoveUnsecureUrls(&urls); if (urls.empty()) { diff --git a/chromium/components/update_client/ping_manager_unittest.cc b/chromium/components/update_client/ping_manager_unittest.cc index 7f945bc8fc6..32cd92ba9dd 100644 --- a/chromium/components/update_client/ping_manager_unittest.cc +++ b/chromium/components/update_client/ping_manager_unittest.cc @@ -11,7 +11,6 @@ #include "base/bind.h" #include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/test/scoped_task_environment.h" #include "base/threading/thread_task_runner_handle.h" @@ -114,7 +113,7 @@ TEST_F(PingManagerTest, SendPing) { { Component component(*update_context, "abc"); - + component.crx_component_ = std::make_unique<CrxComponent>(); component.state_ = std::make_unique<Component::StateUpdated>(&component); component.previous_version_ = base::Version("1.0"); component.next_version_ = base::Version("2.0"); @@ -147,6 +146,7 @@ TEST_F(PingManagerTest, SendPing) { { // Test eventresult="0" is sent for failed updates. Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); component.state_ = std::make_unique<Component::StateUpdateError>(&component); component.previous_version_ = base::Version("1.0"); @@ -170,16 +170,17 @@ TEST_F(PingManagerTest, SendPing) { { // Test the error values and the fingerprints. Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); component.state_ = std::make_unique<Component::StateUpdateError>(&component); component.previous_version_ = base::Version("1.0"); component.next_version_ = base::Version("2.0"); component.previous_fp_ = "prev fp"; component.next_fp_ = "next fp"; - component.error_category_ = 1; + component.error_category_ = ErrorCategory::kDownload; component.error_code_ = 2; component.extra_code1_ = -1; - component.diff_error_category_ = 10; + component.diff_error_category_ = ErrorCategory::kService; component.diff_error_code_ = 20; component.diff_extra_code1_ = -10; component.crx_diffurls_.push_back(GURL("http://host/path")); @@ -195,7 +196,7 @@ TEST_F(PingManagerTest, SendPing) { "<app appid=\"abc\">" "<event eventtype=\"3\" eventresult=\"0\" errorcat=\"1\" " "errorcode=\"2\" extracode1=\"-1\" diffresult=\"0\" " - "differrorcat=\"10\" " + "differrorcat=\"4\" " "differrorcode=\"20\" diffextracode1=\"-10\" " "previousfp=\"prev fp\" nextfp=\"next fp\" " "previousversion=\"1.0\" nextversion=\"2.0\"/></app>")) @@ -206,6 +207,7 @@ TEST_F(PingManagerTest, SendPing) { { // Test an invalid |next_version| is not serialized. Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); component.state_ = std::make_unique<Component::StateUpdateError>(&component); component.previous_version_ = base::Version("1.0"); @@ -229,6 +231,7 @@ TEST_F(PingManagerTest, SendPing) { // Test a valid |previouversion| and |next_version| = base::Version("0") // are serialized correctly under <event...> for uninstall. Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); component.Uninstall(base::Version("1.2.3.4"), 0); component.AppendEvent(BuildUninstalledEventElement(component)); @@ -249,6 +252,7 @@ TEST_F(PingManagerTest, SendPing) { { // Test the download metrics. Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); component.state_ = std::make_unique<Component::StateUpdated>(&component); component.previous_version_ = base::Version("1.0"); component.next_version_ = base::Version("2.0"); @@ -306,9 +310,10 @@ TEST_F(PingManagerTest, RequiresEncryption) { const auto update_context = MakeMockUpdateContext(); Component component(*update_context, "abc"); + component.crx_component_ = std::make_unique<CrxComponent>(); // The default value for |requires_network_encryption| is true. - EXPECT_TRUE(component.crx_component_.requires_network_encryption); + EXPECT_TRUE(component.crx_component_->requires_network_encryption); component.state_ = std::make_unique<Component::StateUpdated>(&component); component.previous_version_ = base::Version("1.0"); diff --git a/chromium/components/update_client/protocol_builder.cc b/chromium/components/update_client/protocol_builder.cc index 46794ffcd47..e708c46706a 100644 --- a/chromium/components/update_client/protocol_builder.cc +++ b/chromium/components/update_client/protocol_builder.cc @@ -141,8 +141,9 @@ std::string BuildUpdateCompleteEventElement(const Component& component) { std::string event("<event eventtype=\"3\""); const int event_result = component.state() == ComponentState::kUpdated; StringAppendF(&event, " eventresult=\"%d\"", event_result); - if (component.error_category()) - StringAppendF(&event, " errorcat=\"%d\"", component.error_category()); + if (component.error_category() != ErrorCategory::kNone) + StringAppendF(&event, " errorcat=\"%d\"", + static_cast<int>(component.error_category())); if (component.error_code()) StringAppendF(&event, " errorcode=\"%d\"", component.error_code()); if (component.extra_code1()) @@ -150,9 +151,9 @@ std::string BuildUpdateCompleteEventElement(const Component& component) { if (HasDiffUpdate(component)) StringAppendF(&event, " diffresult=\"%d\"", !component.diff_update_failed()); - if (component.diff_error_category()) { + if (component.diff_error_category() != ErrorCategory::kNone) { StringAppendF(&event, " differrorcat=\"%d\"", - component.diff_error_category()); + static_cast<int>(component.diff_error_category())); } if (component.diff_error_code()) StringAppendF(&event, " differrorcode=\"%d\"", component.diff_error_code()); @@ -232,11 +233,11 @@ std::string BuildProtocolRequest( // Chrome version and platform information. base::StringAppendF(&request, - "version=\"%s-%s\" prodversion=\"%s\" " + "updater=\"%s\" updaterversion=\"%s\" prodversion=\"%s\" " "lang=\"%s\" updaterchannel=\"%s\" prodchannel=\"%s\" " "os=\"%s\" arch=\"%s\" nacl_arch=\"%s\"", - prod_id.c_str(), // "version" is prefixed by prod_id. - browser_version.c_str(), + prod_id.c_str(), // "updater" + browser_version.c_str(), // "updaterversion" browser_version.c_str(), // "prodversion" lang.c_str(), // "lang" channel.c_str(), // "updaterchannel" @@ -334,22 +335,27 @@ std::string BuildUpdateCheckRequest( for (const auto& id : ids_checked) { DCHECK_EQ(1u, components.count(id)); const auto& component = *components.at(id); - const auto& crx_component = component.crx_component(); const auto& component_id = component.id(); + const auto* crx_component = component.crx_component(); + + DCHECK(crx_component); const update_client::InstallerAttributes installer_attributes( - SanitizeInstallerAttributes(crx_component.installer_attributes)); + SanitizeInstallerAttributes(crx_component->installer_attributes)); std::string app("<app "); base::StringAppendF(&app, "appid=\"%s\" version=\"%s\"", component_id.c_str(), - crx_component.version.GetString().c_str()); + crx_component->version.GetString().c_str()); if (!brand.empty()) base::StringAppendF(&app, " brand=\"%s\"", brand.c_str()); - if (!crx_component.install_source.empty()) + if (!crx_component->install_source.empty()) base::StringAppendF(&app, " installsource=\"%s\"", - crx_component.install_source.c_str()); + crx_component->install_source.c_str()); else if (component.is_foreground()) base::StringAppendF(&app, " installsource=\"ondemand\""); + if (!crx_component->install_location.empty()) + base::StringAppendF(&app, " installedby=\"%s\"", + crx_component->install_location.c_str()); for (const auto& attr : installer_attributes) { base::StringAppendF(&app, " %s=\"%s\"", attr.first.c_str(), attr.second.c_str()); @@ -357,7 +363,7 @@ std::string BuildUpdateCheckRequest( const auto& cohort = metadata->GetCohort(component_id); const auto& cohort_name = metadata->GetCohortName(component_id); const auto& cohort_hint = metadata->GetCohortHint(component_id); - const auto& disabled_reasons = crx_component.disabled_reasons; + const auto& disabled_reasons = crx_component->disabled_reasons; if (!cohort.empty()) base::StringAppendF(&app, " cohort=\"%s\"", cohort.c_str()); if (!cohort_name.empty()) @@ -371,7 +377,7 @@ std::string BuildUpdateCheckRequest( base::StringAppendF(&app, "<disabled reason=\"%d\"/>", disabled_reason); base::StringAppendF(&app, "<updatecheck"); - if (crx_component.supports_group_policy_enable_component_updates && + if (crx_component->supports_group_policy_enable_component_updates && !enabled_component_updates) { base::StringAppendF(&app, " updatedisabled=\"true\""); } @@ -399,12 +405,12 @@ std::string BuildUpdateCheckRequest( base::StringAppendF(&app, " ping_freshness=\"%s\"/>", metadata->GetPingFreshness(component_id).c_str()); - if (!crx_component.fingerprint.empty()) { + if (!crx_component->fingerprint.empty()) { base::StringAppendF(&app, "<packages>" "<package fp=\"%s\"/>" "</packages>", - crx_component.fingerprint.c_str()); + crx_component->fingerprint.c_str()); } base::StringAppendF(&app, "</app>"); app_elements.append(app); diff --git a/chromium/components/update_client/protocol_builder_unittest.cc b/chromium/components/update_client/protocol_builder_unittest.cc index 74700482259..f748ced1951 100644 --- a/chromium/components/update_client/protocol_builder_unittest.cc +++ b/chromium/components/update_client/protocol_builder_unittest.cc @@ -28,7 +28,8 @@ TEST(BuildProtocolRequest, SessionIdProdIdVersion) { string::npos, request.find(" sessionid=\"{15160585-8ADE-4D3C-839B-1281A6035D1F}\" ")); EXPECT_NE(string::npos, - request.find(" version=\"some_prod_id-1.0\" prodversion=\"1.0\" ")); + request.find(" updater=\"some_prod_id\" updaterversion=\"1.0\" " + "prodversion=\"1.0\" ")); } TEST(BuildProtocolRequest, DownloadPreference) { diff --git a/chromium/components/update_client/request_sender_unittest.cc b/chromium/components/update_client/request_sender_unittest.cc index ecebd945d05..f480cac3a66 100644 --- a/chromium/components/update_client/request_sender_unittest.cc +++ b/chromium/components/update_client/request_sender_unittest.cc @@ -31,7 +31,7 @@ const char kUrlPath2[] = "path2"; // TODO(sorin): refactor as a utility function for unit tests. base::FilePath test_file(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components") .AppendASCII("test") .AppendASCII("data") diff --git a/chromium/components/update_client/update_checker.cc b/chromium/components/update_client/update_checker.cc index d63f0eb12da..1fb0479865e 100644 --- a/chromium/components/update_client/update_checker.cc +++ b/chromium/components/update_client/update_checker.cc @@ -41,7 +41,8 @@ namespace { bool IsEncryptionRequired(const IdToComponentPtrMap& components) { for (const auto& item : components) { const auto& component = item.second; - if (component->crx_component().requires_network_encryption) + if (component->crx_component() && + component->crx_component()->requires_network_encryption) return true; } return false; diff --git a/chromium/components/update_client/update_checker_unittest.cc b/chromium/components/update_client/update_checker_unittest.cc index 8dc0c61ebde..a3249b985c3 100644 --- a/chromium/components/update_client/update_checker_unittest.cc +++ b/chromium/components/update_client/update_checker_unittest.cc @@ -28,6 +28,7 @@ #include "components/update_client/update_engine.h" #include "components/update_client/url_request_post_interceptor.h" #include "net/url_request/url_request_test_util.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -39,7 +40,7 @@ namespace { base::FilePath test_file(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components") .AppendASCII("test") .AppendASCII("data") @@ -104,7 +105,7 @@ void ActivityDataServiceTest::SetDaysSinceLastRollCall(const std::string& id, } // namespace class UpdateCheckerTest : public testing::Test, - public ::testing::WithParamInterface<bool> { + public testing::WithParamInterface<bool> { public: UpdateCheckerTest(); ~UpdateCheckerTest() override; @@ -145,7 +146,7 @@ class UpdateCheckerTest : public testing::Test, DISALLOW_COPY_AND_ASSIGN(UpdateCheckerTest); }; -INSTANTIATE_TEST_CASE_P(IsForeground, UpdateCheckerTest, ::testing::Bool()); +INSTANTIATE_TEST_CASE_P(IsForeground, UpdateCheckerTest, testing::Bool()); UpdateCheckerTest::UpdateCheckerTest() : scoped_task_environment_( @@ -211,16 +212,17 @@ scoped_refptr<UpdateContext> UpdateCheckerTest::MakeMockUpdateContext() const { } std::unique_ptr<Component> UpdateCheckerTest::MakeComponent() const { - CrxComponent crx_component; - crx_component.name = "test_jebg"; - crx_component.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx_component.installer = nullptr; - crx_component.version = base::Version("0.9"); - crx_component.fingerprint = "fp1"; + std::unique_ptr<CrxComponent> crx_component = + std::make_unique<CrxComponent>(); + crx_component->name = "test_jebg"; + crx_component->pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); + crx_component->installer = nullptr; + crx_component->version = base::Version("0.9"); + crx_component->fingerprint = "fp1"; auto component = std::make_unique<Component>(*update_context_, kUpdateItemId); component->state_ = std::make_unique<Component::StateNew>(component.get()); - component->crx_component_ = crx_component; + component->crx_component_ = std::move(crx_component); return component; } @@ -238,11 +240,11 @@ TEST_P(UpdateCheckerTest, UpdateCheckSuccess) { components[kUpdateItemId] = MakeComponent(); auto& component = components[kUpdateItemId]; - component->crx_component_.installer_attributes["ap"] = "some_ap"; + component->crx_component_->installer_attributes["ap"] = "some_ap"; update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -253,30 +255,29 @@ TEST_P(UpdateCheckerTest, UpdateCheckSuccess) { << post_interceptor_->GetRequestsAsString(); // Sanity check the request. - const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, - request.find("request protocol=\"3.1\" extra=\"params\"")); + const auto& request = post_interceptor_->GetRequestBody(0); + EXPECT_THAT(request, + testing::HasSubstr(R"(request protocol="3.1" extra="params")")); // The request must not contain any "dlpref" in the default case. - EXPECT_EQ(string::npos, request.find(" dlpref=\"")); - EXPECT_NE(string::npos, - request.find(std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" " - "brand=\"TEST\"" + - (GetParam() ? " installsource=\"ondemand\"" : "") + - " ap=\"some_ap\" enabled=\"1\">" - "<updatecheck/><ping r=\"-2\" ")); - EXPECT_NE(string::npos, - request.find("<packages><package fp=\"fp1\"/></packages></app>")); - - EXPECT_NE(string::npos, request.find("<hw physmemory=")); - - // Tests that the progid is injected correctly from the configurator. - EXPECT_NE( - string::npos, - request.find(" version=\"fake_prodid-30.0\" prodversion=\"30.0\" ")); + EXPECT_THAT(request, testing::Not(testing::HasSubstr(R"( dlpref=")"))); + EXPECT_THAT(request, + testing::HasSubstr( + std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" brand="TEST")" + + (GetParam() ? R"( installsource="ondemand")" : "") + + R"( ap="some_ap" enabled="1"><updatecheck/><ping r="-2" )")); + EXPECT_THAT( + request, + testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); + EXPECT_THAT(request, testing::HasSubstr("<hw physmemory=")); + + // Tests that the product id is injected correctly from the configurator. + EXPECT_THAT(request, testing::HasSubstr( + R"( updater="fake_prodid" updaterversion="30.0")" + R"( prodversion="30.0" )")); // Tests that there is a sessionid attribute. - EXPECT_NE(string::npos, request.find(" sessionid=")); + EXPECT_THAT(request, testing::HasSubstr(" sessionid=")); // Sanity check the arguments of the callback after parsing. EXPECT_EQ(0, error_); @@ -290,11 +291,11 @@ TEST_P(UpdateCheckerTest, UpdateCheckSuccess) { EXPECT_STREQ("this", component->action_run_.c_str()); #if (OS_WIN) - EXPECT_NE(string::npos, request.find(" domainjoined=")); + EXPECT_THAT(request, testing::HasSubstr(" domainjoined=")); #if defined(GOOGLE_CHROME_BUILD) // Check the Omaha updater state data in the request. - EXPECT_NE(string::npos, request.find("<updater ")); - EXPECT_NE(string::npos, request.find(" name=\"Omaha\" ")); + EXPECT_THAT(request, testing::HasSubstr("<updater ")); + EXPECT_THAT(request, testing::HasSubstr(R"( name="Omaha" )")); #endif // GOOGLE_CHROME_BUILD #endif // OS_WINDOWS @@ -323,23 +324,23 @@ TEST_F(UpdateCheckerTest, UpdateCheckInvalidAp) { // Make "ap" too long. auto& component = components[kUpdateItemId]; - component->crx_component_.installer_attributes["ap"] = std::string(257, 'a'); + component->crx_component_->installer_attributes["ap"] = std::string(257, 'a'); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, - request.find(std::string("app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" brand=\"TEST\" enabled=\"1\">" - "<updatecheck/><ping r=\"-2\" ")); - EXPECT_NE(string::npos, - request.find("<packages><package fp=\"fp1\"/></packages></app>")); + EXPECT_THAT(request, testing::HasSubstr( + std::string(R"(app appid=")") + kUpdateItemId + + R"(" version="0.9" brand="TEST" enabled="1">)" + + R"(<updatecheck/><ping r="-2" )")); + EXPECT_THAT( + request, + testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); } TEST_F(UpdateCheckerTest, UpdateCheckSuccessNoBrand) { @@ -354,20 +355,21 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccessNoBrand) { components[kUpdateItemId] = MakeComponent(); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, - request.find(std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" enabled=\"1\">" - "<updatecheck/><ping r=\"-2\" ")); - EXPECT_NE(string::npos, - request.find("<packages><package fp=\"fp1\"/></packages></app>")); + EXPECT_THAT( + request, + testing::HasSubstr( + std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" enabled="1"><updatecheck/><ping r="-2" )")); + EXPECT_THAT( + request, + testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); } // Simulates a 403 server response error. @@ -383,8 +385,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckError) { auto& component = components[kUpdateItemId]; update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -411,8 +412,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) { components[kUpdateItemId] = MakeComponent(); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); @@ -420,7 +421,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) { // The request must contain dlpref="cacheable". const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, request.find(" dlpref=\"cacheable\"")); + EXPECT_THAT(request, testing::HasSubstr(R"( dlpref="cacheable")")); } // This test is checking that an update check signed with CUP fails, since there @@ -440,8 +441,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckCupError) { const auto& component = components[kUpdateItemId]; update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); @@ -453,14 +453,14 @@ TEST_F(UpdateCheckerTest, UpdateCheckCupError) { << post_interceptor_->GetRequestsAsString(); // Sanity check the request. - const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, - request.find(std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" " - "brand=\"TEST\" enabled=\"1\">" - "<updatecheck/><ping r=\"-2\" ")); - EXPECT_NE(string::npos, - request.find("<packages><package fp=\"fp1\"/></packages></app>")); + const auto& request = post_interceptor_->GetRequestBody(0); + EXPECT_THAT(request, testing::HasSubstr( + std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" brand="TEST" enabled="1">)" + + R"(<updatecheck/><ping r="-2" )")); + EXPECT_THAT( + request, + testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); // Expect an error since the response is not trusted. EXPECT_EQ(-10000, error_); @@ -478,11 +478,10 @@ TEST_F(UpdateCheckerTest, UpdateCheckRequiresEncryptionError) { components[kUpdateItemId] = MakeComponent(); auto& component = components[kUpdateItemId]; - component->crx_component_.requires_network_encryption = true; + component->crx_component_->requires_network_encryption = true; update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -509,16 +508,16 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastRollCall) { // Do two update-checks. activity_data_service_->SetDaysSinceLastRollCall(kUpdateItemId, 5); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -527,10 +526,10 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastRollCall) { << post_interceptor_->GetRequestsAsString(); ASSERT_EQ(2, post_interceptor_->GetCount()) << post_interceptor_->GetRequestsAsString(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(0).find( - "<ping r=\"5\" ping_freshness=")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(1).find( - "<ping rd=\"3383\" ping_freshness=")); + EXPECT_THAT(post_interceptor_->GetRequestBody(0), + testing::HasSubstr(R"(<ping r="5" ping_freshness=)")); + EXPECT_THAT(post_interceptor_->GetRequestBody(1), + testing::HasSubstr(R"(<ping rd="3383" ping_freshness=)")); } TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { @@ -552,8 +551,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { activity_data_service_->SetActiveBit(kUpdateItemId, true); activity_data_service_->SetDaysSinceLastActive(kUpdateItemId, 10); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -564,8 +563,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { activity_data_service_->SetActiveBit(kUpdateItemId, true); update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -575,8 +574,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "extra=\"params\"", true, + update_context_->session_id, {kUpdateItemId}, components, + "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -587,12 +586,13 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { << post_interceptor_->GetRequestsAsString(); ASSERT_EQ(3, post_interceptor_->GetCount()) << post_interceptor_->GetRequestsAsString(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(0).find( - "<ping a=\"10\" r=\"-2\" ping_freshness=")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(1).find( - "<ping ad=\"3383\" rd=\"3383\" ping_freshness=")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(2).find( - "<ping rd=\"3383\" ping_freshness=")); + EXPECT_THAT(post_interceptor_->GetRequestBody(0), + testing::HasSubstr(R"(<ping a="10" r="-2" ping_freshness=)")); + EXPECT_THAT( + post_interceptor_->GetRequestBody(1), + testing::HasSubstr(R"(<ping ad="3383" rd="3383" ping_freshness=)")); + EXPECT_THAT(post_interceptor_->GetRequestBody(2), + testing::HasSubstr(R"(<ping rd="3383" ping_freshness=)")); } TEST_F(UpdateCheckerTest, UpdateCheckInstallSource) { @@ -602,64 +602,65 @@ TEST_F(UpdateCheckerTest, UpdateCheckInstallSource) { components[kUpdateItemId] = MakeComponent(); auto& component = components[kUpdateItemId]; - auto& crx_component = const_cast<CrxComponent&>(component->crx_component()); + auto* crx_component = const_cast<CrxComponent*>(component->crx_component()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_EQ(string::npos, - post_interceptor_->GetRequestBody(0).find("installsource=")); + EXPECT_THAT(post_interceptor_->GetRequestBody(0), + testing::Not(testing::HasSubstr("installsource="))); update_context_->is_foreground = true; EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(1).find( - "installsource=\"ondemand\"")); + const auto& body1 = post_interceptor_->GetRequestBody(1); + EXPECT_THAT(body1, testing::HasSubstr(R"(installsource="ondemand")")); + EXPECT_THAT(body1, testing::Not(testing::HasSubstr(R"(installedby=)"))); update_context_->is_foreground = false; - crx_component.install_source = "webstore"; + crx_component->install_source = "webstore"; + crx_component->install_location = "external"; EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(2).find( - "installsource=\"webstore\"")); + const auto& body2 = post_interceptor_->GetRequestBody(2); + EXPECT_THAT(body2, testing::HasSubstr(R"(installsource="webstore")")); + EXPECT_THAT(body2, testing::HasSubstr(R"(installedby="external")")); update_context_->is_foreground = true; - crx_component.install_source = "sideload"; + crx_component->install_source = "sideload"; + crx_component->install_location = "policy"; EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(3).find( - "installsource=\"sideload\"")); + const auto& body3 = post_interceptor_->GetRequestBody(3); + EXPECT_THAT(body3, testing::HasSubstr(R"(installsource="sideload")")); + EXPECT_THAT(body3, testing::HasSubstr(R"(installedby="policy")")); } TEST_F(UpdateCheckerTest, ComponentDisabled) { @@ -669,110 +670,99 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { components[kUpdateItemId] = MakeComponent(); auto& component = components[kUpdateItemId]; - auto& crx_component = const_cast<CrxComponent&>(component->crx_component()); + auto* crx_component = const_cast<CrxComponent*>(component->crx_component()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(0).find("enabled=\"1\"")); - EXPECT_EQ(string::npos, - post_interceptor_->GetRequestBody(0).find("<disabled")); - crx_component.disabled_reasons = std::vector<int>(); + const auto& body0 = post_interceptor_->GetRequestBody(0); + EXPECT_THAT(body0, testing::HasSubstr(R"(enabled="1")")); + EXPECT_THAT(body0, testing::Not(testing::HasSubstr("<disabled"))); + + crx_component->disabled_reasons = {}; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(1).find("enabled=\"1\"")); - EXPECT_EQ(string::npos, - post_interceptor_->GetRequestBody(1).find("<disabled")); - crx_component.disabled_reasons = std::vector<int>({0}); + const auto& body1 = post_interceptor_->GetRequestBody(1); + EXPECT_THAT(body1, testing::HasSubstr(R"(enabled="1")")); + EXPECT_THAT(body1, testing::Not(testing::HasSubstr("<disabled"))); + + crx_component->disabled_reasons = {0}; EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(2).find("enabled=\"0\"")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(2).find( - "<disabled reason=\"0\"/>")); - crx_component.disabled_reasons = std::vector<int>({1}); + const auto& body2 = post_interceptor_->GetRequestBody(2); + EXPECT_THAT(body2, testing::HasSubstr(R"(enabled="0")")); + EXPECT_THAT(body2, testing::HasSubstr(R"(<disabled reason="0")")); + + crx_component->disabled_reasons = {1}; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(3).find("enabled=\"0\"")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(3).find( - "<disabled reason=\"1\"/>")); - crx_component.disabled_reasons = std::vector<int>({4, 8, 16}); + const auto& body3 = post_interceptor_->GetRequestBody(3); + EXPECT_THAT(body3, testing::HasSubstr(R"(enabled="0")")); + EXPECT_THAT(body3, testing::HasSubstr(R"(<disabled reason="1")")); + + crx_component->disabled_reasons = {4, 8, 16}; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(4).find("enabled=\"0\"")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(4).find( - "<disabled reason=\"4\"/>")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(4).find( - "<disabled reason=\"8\"/>")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(4).find( - "<disabled reason=\"16\"/>")); - - crx_component.disabled_reasons = std::vector<int>({0, 4, 8, 16}); + + const auto& body4 = post_interceptor_->GetRequestBody(4); + EXPECT_THAT(body4, testing::HasSubstr(R"(enabled="0")")); + EXPECT_THAT(body4, testing::HasSubstr(R"(<disabled reason="4")")); + EXPECT_THAT(body4, testing::HasSubstr(R"(<disabled reason="8")")); + EXPECT_THAT(body4, testing::HasSubstr(R"(<disabled reason="16")")); + + crx_component->disabled_reasons = {0, 4, 8, 16}; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, - post_interceptor_->GetRequestBody(5).find("enabled=\"0\"")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(5).find( - "<disabled reason=\"0\"/>")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(5).find( - "<disabled reason=\"4\"/>")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(5).find( - "<disabled reason=\"8\"/>")); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(5).find( - "<disabled reason=\"16\"/>")); + + const auto& body5 = post_interceptor_->GetRequestBody(5); + EXPECT_THAT(body5, testing::HasSubstr(R"(enabled="0")")); + EXPECT_THAT(body5, testing::HasSubstr(R"(<disabled reason="0")")); + EXPECT_THAT(body5, testing::HasSubstr(R"(<disabled reason="4")")); + EXPECT_THAT(body5, testing::HasSubstr(R"(<disabled reason="8")")); + EXPECT_THAT(body5, testing::HasSubstr(R"(<disabled reason="16")")); } TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { @@ -789,85 +779,82 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { // * the component updates are disabled. // Expects the group policy to be ignored and the update check to not // include the "updatedisabled" attribute. - EXPECT_FALSE( - component->crx_component_.supports_group_policy_enable_component_updates); + EXPECT_FALSE(component->crx_component_ + ->supports_group_policy_enable_component_updates); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(0).find( - std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" enabled=\"1\">" - "<updatecheck/>")); + EXPECT_THAT( + post_interceptor_->GetRequestBody(0), + testing::HasSubstr(std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" enabled="1"><updatecheck/>)")); // Tests the scenario where: // * the component supports group policies. // * the component updates are disabled. // Expects the update check to include the "updatedisabled" attribute. - component->crx_component_.supports_group_policy_enable_component_updates = + component->crx_component_->supports_group_policy_enable_component_updates = true; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", false, + update_context_->session_id, {kUpdateItemId}, components, "", false, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(1).find( - std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" enabled=\"1\">" - "<updatecheck updatedisabled=\"true\"/>")); + EXPECT_THAT( + post_interceptor_->GetRequestBody(1), + testing::HasSubstr(std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" enabled="1">)" + + R"(<updatecheck updatedisabled="true"/>)")); // Tests the scenario where: // * the component does not support group policies. // * the component updates are enabled. // Expects the update check to not include the "updatedisabled" attribute. - component->crx_component_.supports_group_policy_enable_component_updates = + component->crx_component_->supports_group_policy_enable_component_updates = false; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(2).find( - std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" enabled=\"1\">" - "<updatecheck/>")); + EXPECT_THAT( + post_interceptor_->GetRequestBody(2), + testing::HasSubstr(std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" enabled="1"><updatecheck/>)")); // Tests the scenario where: // * the component supports group policies. // * the component updates are enabled. // Expects the update check to not include the "updatedisabled" attribute. - component->crx_component_.supports_group_policy_enable_component_updates = + component->crx_component_->supports_group_policy_enable_component_updates = true; update_checker_ = UpdateChecker::Create(config_, metadata_.get()); EXPECT_TRUE(post_interceptor_->ExpectRequest( std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - EXPECT_NE(string::npos, post_interceptor_->GetRequestBody(3).find( - std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" enabled=\"1\">" - "<updatecheck/>")); + EXPECT_THAT( + post_interceptor_->GetRequestBody(3), + testing::HasSubstr(std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" enabled="1"><updatecheck/>)")); } TEST_F(UpdateCheckerTest, NoUpdateActionRun) { @@ -883,8 +870,7 @@ TEST_F(UpdateCheckerTest, NoUpdateActionRun) { auto& component = components[kUpdateItemId]; update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); @@ -915,19 +901,19 @@ TEST_F(UpdateCheckerTest, UpdatePauseResume) { components[kUpdateItemId] = MakeComponent(); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); RunThreads(); - const auto request = post_interceptor_->GetRequestBody(0); - EXPECT_NE(string::npos, - request.find(std::string("<app appid=\"") + kUpdateItemId + - "\" version=\"0.9\" brand=\"TEST\" enabled=\"1\">" - "<updatecheck/><ping r=\"-2\" ")); - EXPECT_NE(string::npos, - request.find("<packages><package fp=\"fp1\"/></packages></app>")); + const auto& request = post_interceptor_->GetRequestBody(0); + EXPECT_THAT(request, testing::HasSubstr( + std::string(R"(<app appid=")") + kUpdateItemId + + R"(" version="0.9" brand="TEST" enabled="1">)" + + R"(<updatecheck/><ping r="-2" )")); + EXPECT_THAT( + request, + testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); } // Tests that an update checker object and its underlying URLFetcher can @@ -949,8 +935,7 @@ TEST_F(UpdateCheckerTest, UpdateResetUpdateChecker) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( - update_context_->session_id, std::vector<std::string>{kUpdateItemId}, - components, "", true, + update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); runloop.Run(); diff --git a/chromium/components/update_client/update_client.h b/chromium/components/update_client/update_client.h index af0df043f0b..d8d546a16a2 100644 --- a/chromium/components/update_client/update_client.h +++ b/chromium/components/update_client/update_client.h @@ -258,6 +258,11 @@ struct CrxComponent { // For extension, this information is set from the update service, which // gets the install source from the update URL. std::string install_source; + + // Information about where the component/extension was loaded from. + // For extensions, this information is inferred from the extension + // registry. + std::string install_location; }; // Called when a non-blocking call of UpdateClient completes. @@ -270,8 +275,8 @@ using Callback = base::OnceCallback<void(Error error)>; class UpdateClient : public base::RefCounted<UpdateClient> { public: using CrxDataCallback = - base::OnceCallback<void(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components)>; + base::OnceCallback<std::vector<std::unique_ptr<CrxComponent>>( + const std::vector<std::string>& ids)>; // Defines an interface to observe the UpdateClient. It provides // notifications when state changes occur for the service itself or for the diff --git a/chromium/components/update_client/update_client_errors.h b/chromium/components/update_client/update_client_errors.h index 814f08a1d77..129d7aa9763 100644 --- a/chromium/components/update_client/update_client_errors.h +++ b/chromium/components/update_client/update_client_errors.h @@ -17,16 +17,18 @@ enum class Error { RETRY_LATER = 3, SERVICE_ERROR = 4, UPDATE_CHECK_ERROR = 5, + CRX_NOT_FOUND = 6, }; // These errors are sent in pings. Add new values only to the bottom of // the enums below; the order must be kept stable. enum class ErrorCategory { - kErrorNone = 0, - kNetworkError, - kUnpackError, - kInstallError, - kServiceError, // Runtime errors which occur in the service itself. + kNone = 0, + kDownload, + kUnpack, + kInstall, + kService, // Runtime errors which occur in the service itself. + kUpdateCheck, }; // These errors are returned with the |kNetworkError| error category. This @@ -43,7 +45,7 @@ enum class CrxDownloaderError { GENERIC_ERROR = -1 }; -// These errors are returned with the |kUnpackError| error category and +// These errors are returned with the |kUnpack| error category and // indicate unpacker or patcher error. enum class UnpackerError { kNone = 0, @@ -66,7 +68,7 @@ enum class UnpackerError { // kFingerprintWriteFailed = 17, // Deprecated. Don't use. }; -// These errors are returned with the |kServiceError| error category and +// These errors are returned with the |kService| error category and // are returned by the component installers. enum class InstallError { NONE = 0, @@ -83,7 +85,7 @@ enum class InstallError { CUSTOM_ERROR_BASE = 100, // Specific installer errors go above this value. }; -// These errors are returned with the |kInstallError| error category and +// These errors are returned with the |kInstall| error category and // indicate critical or configuration errors in the update service. enum class ServiceError { NONE = 0, diff --git a/chromium/components/update_client/update_client_unittest.cc b/chromium/components/update_client/update_client_unittest.cc index 4b9c0b1fdde..581cbf9e9b9 100644 --- a/chromium/components/update_client/update_client_unittest.cc +++ b/chromium/components/update_client/update_client_unittest.cc @@ -13,6 +13,7 @@ #include "base/memory/ref_counted.h" #include "base/path_service.h" #include "base/run_loop.h" +#include "base/stl_util.h" #include "base/task_scheduler/post_task.h" #include "base/task_scheduler/task_traits.h" #include "base/test/scoped_path_override.h" @@ -93,10 +94,10 @@ class MockPingManagerImpl : public PingManager { std::string id; base::Version previous_version; base::Version next_version; - int error_category = 0; + ErrorCategory error_category = ErrorCategory::kNone; int error_code = 0; int extra_code1 = 0; - int diff_error_category = 0; + ErrorCategory diff_error_category = ErrorCategory::kNone; int diff_error_code = 0; bool diff_update_failed = false; }; @@ -200,7 +201,7 @@ void UpdateClientTest::RunThreads() { base::FilePath UpdateClientTest::TestFilePath(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components") .AppendASCII("test") .AppendASCII("data") @@ -213,14 +214,16 @@ base::FilePath UpdateClientTest::TestFilePath(const char* file) { TEST_F(UpdateClientTest, OneCrxNoUpdate) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.9"); - crx.installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -319,22 +322,24 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx1; - crx1.name = "test_jebg"; - crx1.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx1.version = base::Version("0.9"); - crx1.installer = base::MakeRefCounted<TestInstaller>(); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx1 = std::make_unique<CrxComponent>(); + crx1->name = "test_jebg"; + crx1->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx1->version = base::Version("0.9"); + crx1->installer = base::MakeRefCounted<TestInstaller>(); - CrxComponent crx2; - crx2.name = "test_abag"; - crx2.pk_hash.assign(abag_hash, abag_hash + arraysize(abag_hash)); - crx2.version = base::Version("2.2"); - crx2.installer = base::MakeRefCounted<TestInstaller>(); + std::unique_ptr<CrxComponent> crx2 = std::make_unique<CrxComponent>(); + crx2->name = "test_abag"; + crx2->pk_hash.assign(abag_hash, abag_hash + base::size(abag_hash)); + crx2->version = base::Version("2.2"); + crx2->installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx1); - components->push_back(crx2); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx1)); + component.push_back(std::move(crx2)); + return component; } }; @@ -482,7 +487,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(0, ping_data[0].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(0, ping_data[0].error_code); } }; @@ -528,26 +533,26 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { update_client->RemoveObserver(&observer); } -// Tests the update check for two CRXs scenario. Both CRXs have updates. -TEST_F(UpdateClientTest, TwoCrxUpdate) { +// Tests the update check for two CRXs scenario when the second CRX does not +// provide a CrxComponent instance. In this case, the update is handled as +// if only one component were provided as an argument to the |Update| call +// with the exception that the second component still fires an event such as +// |COMPONENT_UPDATE_ERROR|. +TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentData) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx1; - crx1.name = "test_jebg"; - crx1.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx1.version = base::Version("0.9"); - crx1.installer = base::MakeRefCounted<TestInstaller>(); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); - CrxComponent crx2; - crx2.name = "test_ihfo"; - crx2.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash)); - crx2.version = base::Version("0.8"); - crx2.installer = base::MakeRefCounted<TestInstaller>(); - - components->push_back(crx1); - components->push_back(crx2); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + component.push_back(std::move(nullptr)); + return component; } }; @@ -592,25 +597,11 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { </manifest> </updatecheck> </app> - <app appid='ihfokbkgjpifnbbojhneepfflplebdkc'> - <updatecheck status='ok'> - <urls> - <url codebase='http://localhost/download/'/> - </urls> - <manifest version='1.0' prodversionmin='11.0.1.0'> - <packages> - <package name='ihfokbkgjpifnbbojhneepfflplebdkc_1.crx' - hash_sha256='813c59747e139a608b3b5fc49633affc6db574373f - 309f156ea6d27229c0b3f9'/> - </packages> - </manifest> - </updatecheck> - </app> </response> */ EXPECT_FALSE(session_id.empty()); EXPECT_TRUE(enabled_component_updates); - EXPECT_EQ(2u, ids_to_check.size()); + EXPECT_EQ(1u, ids_to_check.size()); { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; @@ -636,30 +627,6 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { EXPECT_FALSE(component->is_foreground()); } - { - const std::string id = "ihfokbkgjpifnbbojhneepfflplebdkc"; - EXPECT_EQ(id, ids_to_check[1]); - EXPECT_EQ(1u, components.count(id)); - - ProtocolParser::Result::Manifest::Package package; - package.name = "ihfokbkgjpifnbbojhneepfflplebdkc_1.crx"; - package.hash_sha256 = - "813c59747e139a608b3b5fc49633affc6db574373f309f156ea6d27229c0b3f9"; - - ProtocolParser::Result result; - result.extension_id = id; - result.status = "ok"; - result.crx_urls.push_back(GURL("http://localhost/download/")); - result.manifest.version = "1.0"; - result.manifest.browser_min_version = "11.0.1.0"; - result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); - - EXPECT_FALSE(component->is_foreground()); - } - base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } @@ -695,22 +662,6 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { result.response = path; result.downloaded_bytes = 1843; result.total_bytes = 1843; - } else if (url.path() == - "/download/ihfokbkgjpifnbbojhneepfflplebdkc_1.crx") { - download_metrics.url = url; - download_metrics.downloader = DownloadMetrics::kNone; - download_metrics.error = 0; - download_metrics.downloaded_bytes = 53638; - download_metrics.total_bytes = 53638; - download_metrics.download_time_ms = 2000; - - EXPECT_TRUE(MakeTestFile( - TestFilePath("ihfokbkgjpifnbbojhneepfflplebdkc_1.crx"), &path)); - - result.error = 0; - result.response = path; - result.downloaded_bytes = 53638; - result.total_bytes = 53638; } else { NOTREACHED(); } @@ -734,17 +685,12 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { protected: ~MockPingManager() override { const auto ping_data = MockPingManagerImpl::ping_data(); - EXPECT_EQ(2u, ping_data.size()); + EXPECT_EQ(1u, ping_data.size()); EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(0, ping_data[0].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(0, ping_data[0].error_code); - EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[1].id); - EXPECT_EQ(base::Version("0.8"), ping_data[1].previous_version); - EXPECT_EQ(base::Version("1.0"), ping_data[1].next_version); - EXPECT_EQ(0, ping_data[1].error_category); - EXPECT_EQ(0, ping_data[1].error_code); } }; @@ -770,19 +716,104 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { } { InSequence seq; - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, - "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1); - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_FOUND, - "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1); - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_WAIT, - "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1); - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_DOWNLOADING, + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, "ihfokbkgjpifnbbojhneepfflplebdkc")) - .Times(AtLeast(1)); - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_READY, - "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1); - EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATED, - "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1); + .Times(1); + } + + update_client->AddObserver(&observer); + + const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf", + "ihfokbkgjpifnbbojhneepfflplebdkc"}; + update_client->Update( + ids, base::BindOnce(&DataCallbackMock::Callback), false, + base::BindOnce(&CompletionCallbackMock::Callback, quit_closure())); + + RunThreads(); + + update_client->RemoveObserver(&observer); +} + +// Tests the update check for two CRXs scenario when no CrxComponent data is +// provided for either component. In this case, no update check occurs, and +// |COMPONENT_UPDATE_ERROR| event fires for both components. +TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentDataAtAll) { + class DataCallbackMock { + public: + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(nullptr)); + component.push_back(std::move(nullptr)); + return component; + } + }; + + class CompletionCallbackMock { + public: + static void Callback(base::OnceClosure quit_closure, Error error) { + EXPECT_EQ(Error::NONE, error); + std::move(quit_closure).Run(); + } + }; + + class MockUpdateChecker : public UpdateChecker { + public: + static std::unique_ptr<UpdateChecker> Create( + scoped_refptr<Configurator> config, + PersistedData* metadata) { + return std::make_unique<MockUpdateChecker>(); + } + + void CheckForUpdates(const std::string& session_id, + const std::vector<std::string>& ids_to_check, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) override { + NOTREACHED(); + } + }; + + class MockCrxDownloader : public CrxDownloader { + public: + static std::unique_ptr<CrxDownloader> Create( + bool is_background_download, + scoped_refptr<net::URLRequestContextGetter> context_getter) { + return std::make_unique<MockCrxDownloader>(); + } + + MockCrxDownloader() : CrxDownloader(nullptr) {} + + private: + void DoStartDownload(const GURL& url) override { NOTREACHED(); } + }; + + class MockPingManager : public MockPingManagerImpl { + public: + explicit MockPingManager(scoped_refptr<Configurator> config) + : MockPingManagerImpl(config) {} + + protected: + ~MockPingManager() override { + EXPECT_EQ(0u, MockPingManagerImpl::ping_data().size()); + } + }; + + scoped_refptr<UpdateClient> update_client = + base::MakeRefCounted<UpdateClientImpl>( + config(), base::MakeRefCounted<MockPingManager>(config()), + &MockUpdateChecker::Create, &MockCrxDownloader::Create); + + MockObserver observer; + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "ihfokbkgjpifnbbojhneepfflplebdkc")) + .Times(1); } update_client->AddObserver(&observer); @@ -804,22 +835,24 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx1; - crx1.name = "test_jebg"; - crx1.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx1.version = base::Version("0.9"); - crx1.installer = base::MakeRefCounted<TestInstaller>(); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx1 = std::make_unique<CrxComponent>(); + crx1->name = "test_jebg"; + crx1->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx1->version = base::Version("0.9"); + crx1->installer = base::MakeRefCounted<TestInstaller>(); - CrxComponent crx2; - crx2.name = "test_ihfo"; - crx2.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash)); - crx2.version = base::Version("0.8"); - crx2.installer = base::MakeRefCounted<TestInstaller>(); + std::unique_ptr<CrxComponent> crx2 = std::make_unique<CrxComponent>(); + crx2->name = "test_ihfo"; + crx2->pk_hash.assign(ihfo_hash, ihfo_hash + base::size(ihfo_hash)); + crx2->version = base::Version("0.8"); + crx2->installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx1); - components->push_back(crx2); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx1)); + component.push_back(std::move(crx2)); + return component; } }; @@ -1004,12 +1037,12 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(1, ping_data[0].error_category); + EXPECT_EQ(1, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(-118, ping_data[0].error_code); EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[1].id); EXPECT_EQ(base::Version("0.8"), ping_data[1].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[1].next_version); - EXPECT_EQ(0, ping_data[1].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[1].error_category)); EXPECT_EQ(0, ping_data[1].error_code); } }; @@ -1031,7 +1064,15 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { .Times(AtLeast(1)); EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, "jebgalgnebhfojomionfpkfelancnnkf")) - .Times(1); + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(1, static_cast<int>(item.error_category)); + EXPECT_EQ(-118, item.error_code); + EXPECT_EQ(0, item.extra_code1); + })); } { InSequence seq; @@ -1068,8 +1109,8 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { TEST_F(UpdateClientTest, OneCrxDiffUpdate) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { static int num_calls = 0; // Must use the same stateful installer object. @@ -1078,19 +1119,21 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { ++num_calls; - CrxComponent crx; - crx.name = "test_ihfo"; - crx.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash)); - crx.installer = installer; + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_ihfo"; + crx->pk_hash.assign(ihfo_hash, ihfo_hash + base::size(ihfo_hash)); + crx->installer = installer; if (num_calls == 1) { - crx.version = base::Version("0.8"); + crx->version = base::Version("0.8"); } else if (num_calls == 2) { - crx.version = base::Version("1.0"); + crx->version = base::Version("1.0"); } else { NOTREACHED(); } - components->push_back(crx); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -1295,15 +1338,15 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[0].id); EXPECT_EQ(base::Version("0.8"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(0, ping_data[0].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(0, ping_data[0].error_code); EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[1].id); EXPECT_EQ(base::Version("1.0"), ping_data[1].previous_version); EXPECT_EQ(base::Version("2.0"), ping_data[1].next_version); EXPECT_FALSE(ping_data[1].diff_update_failed); - EXPECT_EQ(0, ping_data[1].diff_error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[1].diff_error_category)); EXPECT_EQ(0, ping_data[1].diff_error_code); - EXPECT_EQ(0, ping_data[1].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[1].error_category)); EXPECT_EQ(0, ping_data[1].error_code); } }; @@ -1381,7 +1424,7 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { void Install(const base::FilePath& unpack_path, const std::string& public_key, - Callback callback) { + Callback callback) override { DoInstall(unpack_path, std::move(callback)); unpack_path_ = unpack_path; @@ -1408,8 +1451,8 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { scoped_refptr<MockInstaller> installer = base::MakeRefCounted<MockInstaller>(); @@ -1418,12 +1461,15 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { EXPECT_CALL(*installer, GetInstalledFile(_, _)).Times(0); EXPECT_CALL(*installer, Uninstall()).Times(0); - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.9"); - crx.installer = installer; - components->push_back(crx); + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = installer; + + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -1550,8 +1596,8 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(3, ping_data[0].error_category); // kInstallError. - EXPECT_EQ(9, ping_data[0].error_code); // kInstallerError. + EXPECT_EQ(3, static_cast<int>(ping_data[0].error_category)); // kInstall. + EXPECT_EQ(9, ping_data[0].error_code); // kInstallerError. } }; @@ -1593,8 +1639,8 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { static int num_calls = 0; // Must use the same stateful installer object. @@ -1603,19 +1649,21 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { ++num_calls; - CrxComponent crx; - crx.name = "test_ihfo"; - crx.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash)); - crx.installer = installer; + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_ihfo"; + crx->pk_hash.assign(ihfo_hash, ihfo_hash + base::size(ihfo_hash)); + crx->installer = installer; if (num_calls == 1) { - crx.version = base::Version("0.8"); + crx->version = base::Version("0.8"); } else if (num_calls == 2) { - crx.version = base::Version("1.0"); + crx->version = base::Version("1.0"); } else { NOTREACHED(); } - components->push_back(crx); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -1834,15 +1882,15 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[0].id); EXPECT_EQ(base::Version("0.8"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(0, ping_data[0].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(0, ping_data[0].error_code); EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[1].id); EXPECT_EQ(base::Version("1.0"), ping_data[1].previous_version); EXPECT_EQ(base::Version("2.0"), ping_data[1].next_version); - EXPECT_EQ(0, ping_data[1].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[1].error_category)); EXPECT_EQ(0, ping_data[1].error_code); EXPECT_TRUE(ping_data[1].diff_update_failed); - EXPECT_EQ(1, ping_data[1].diff_error_category); // kNetworkError. + EXPECT_EQ(1, static_cast<int>(ping_data[1].diff_error_category)); EXPECT_EQ(-1, ping_data[1].diff_error_code); } }; @@ -1911,14 +1959,16 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.9"); - crx.installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -2028,15 +2078,17 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { TEST_F(UpdateClientTest, OneCrxInstall) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.0"); - crx.installer = base::MakeRefCounted<TestInstaller>(); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.0"); + crx->installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -2172,7 +2224,7 @@ TEST_F(UpdateClientTest, OneCrxInstall) { EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.0"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(0, ping_data[0].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(0, ping_data[0].error_code); } }; @@ -2208,19 +2260,107 @@ TEST_F(UpdateClientTest, OneCrxInstall) { update_client->RemoveObserver(&observer); } +// Tests the install of one CRX when no component data is provided. This +// results in an install error. +TEST_F(UpdateClientTest, OneCrxInstallNoCrxComponentData) { + class DataCallbackMock { + public: + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(nullptr); + return component; + } + }; + + class CompletionCallbackMock { + public: + static void Callback(base::OnceClosure quit_closure, Error error) { + EXPECT_EQ(Error::NONE, error); + std::move(quit_closure).Run(); + } + }; + + class MockUpdateChecker : public UpdateChecker { + public: + static std::unique_ptr<UpdateChecker> Create( + scoped_refptr<Configurator> config, + PersistedData* metadata) { + return std::make_unique<MockUpdateChecker>(); + } + + void CheckForUpdates(const std::string& session_id, + const std::vector<std::string>& ids_to_check, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) override { + NOTREACHED(); + } + }; + + class MockCrxDownloader : public CrxDownloader { + public: + static std::unique_ptr<CrxDownloader> Create( + bool is_background_download, + scoped_refptr<net::URLRequestContextGetter> context_getter) { + return std::make_unique<MockCrxDownloader>(); + } + + MockCrxDownloader() : CrxDownloader(nullptr) {} + + private: + void DoStartDownload(const GURL& url) override { NOTREACHED(); } + }; + + class MockPingManager : public MockPingManagerImpl { + public: + explicit MockPingManager(scoped_refptr<Configurator> config) + : MockPingManagerImpl(config) {} + + protected: + ~MockPingManager() override { + EXPECT_EQ(0u, MockPingManagerImpl::ping_data().size()); + } + }; + + scoped_refptr<UpdateClient> update_client = + base::MakeRefCounted<UpdateClientImpl>( + config(), base::MakeRefCounted<MockPingManager>(config()), + &MockUpdateChecker::Create, &MockCrxDownloader::Create); + + MockObserver observer; + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + + update_client->AddObserver(&observer); + + update_client->Install( + std::string("jebgalgnebhfojomionfpkfelancnnkf"), + base::BindOnce(&DataCallbackMock::Callback), + base::BindOnce(&CompletionCallbackMock::Callback, quit_closure())); + + RunThreads(); + + update_client->RemoveObserver(&observer); +} + // Tests that overlapping installs of the same CRX result in an error. TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.0"); - crx.installer = base::MakeRefCounted<TestInstaller>(); - - components->push_back(crx); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.0"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -2337,8 +2477,10 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { TEST_F(UpdateClientTest, EmptyIdList) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) {} + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + return {}; + } }; class CompletionCallbackMock { @@ -2474,14 +2616,16 @@ TEST_F(UpdateClientTest, SendUninstallPing) { TEST_F(UpdateClientTest, RetryAfter) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.9"); - crx.installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -2666,23 +2810,25 @@ TEST_F(UpdateClientTest, RetryAfter) { TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx1; - crx1.name = "test_jebg"; - crx1.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx1.version = base::Version("0.9"); - crx1.installer = base::MakeRefCounted<TestInstaller>(); - crx1.supports_group_policy_enable_component_updates = true; + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx1 = std::make_unique<CrxComponent>(); + crx1->name = "test_jebg"; + crx1->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx1->version = base::Version("0.9"); + crx1->installer = base::MakeRefCounted<TestInstaller>(); + crx1->supports_group_policy_enable_component_updates = true; - CrxComponent crx2; - crx2.name = "test_ihfo"; - crx2.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash)); - crx2.version = base::Version("0.8"); - crx2.installer = base::MakeRefCounted<TestInstaller>(); + std::unique_ptr<CrxComponent> crx2 = std::make_unique<CrxComponent>(); + crx2->name = "test_ihfo"; + crx2->pk_hash.assign(ihfo_hash, ihfo_hash + base::size(ihfo_hash)); + crx2->version = base::Version("0.8"); + crx2->installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx1); - components->push_back(crx2); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx1)); + component.push_back(std::move(crx2)); + return component; } }; @@ -2858,12 +3004,12 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); - EXPECT_EQ(4, ping_data[0].error_category); + EXPECT_EQ(4, static_cast<int>(ping_data[0].error_category)); EXPECT_EQ(2, ping_data[0].error_code); EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_data[1].id); EXPECT_EQ(base::Version("0.8"), ping_data[1].previous_version); EXPECT_EQ(base::Version("1.0"), ping_data[1].next_version); - EXPECT_EQ(0, ping_data[1].error_category); + EXPECT_EQ(0, static_cast<int>(ping_data[1].error_category)); EXPECT_EQ(0, ping_data[1].error_code); } }; @@ -2924,14 +3070,16 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { class DataCallbackMock { public: - static void Callback(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_jebg"; - crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); - crx.version = base::Version("0.9"); - crx.installer = base::MakeRefCounted<TestInstaller>(); - components->push_back(crx); + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; } }; @@ -2963,9 +3111,11 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check.front()); EXPECT_EQ(1u, components.count(id)); - + constexpr int update_check_error = -1; + components.at(id)->set_update_check_error(update_check_error); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), -1, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), + update_check_error, 0)); } }; @@ -3009,6 +3159,9 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { CrxUpdateItem item; update_client->GetCrxUpdateState(id, &item); EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-1, item.error_code); + EXPECT_EQ(0, item.extra_code1); })); update_client->AddObserver(&observer); @@ -3170,14 +3323,15 @@ TEST_F(UpdateClientTest, ActionRun_Install) { // The action is a program which returns 1877345072 as a hardcoded value. update_client->Install( std::string("gjpmebpgbhcamgdgjcmnjfhggjpgcimm"), - base::BindOnce([](const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_niea"; - crx.pk_hash.assign(gjpm_hash, gjpm_hash + arraysize(gjpm_hash)); - crx.version = base::Version("0.0"); - crx.installer = base::MakeRefCounted<VersionedTestInstaller>(); - components->push_back(crx); + base::BindOnce([](const std::vector<std::string>& ids) { + auto crx = std::make_unique<CrxComponent>(); + crx->name = "test_niea"; + crx->pk_hash.assign(gjpm_hash, gjpm_hash + base::size(gjpm_hash)); + crx->version = base::Version("0.0"); + crx->installer = base::MakeRefCounted<VersionedTestInstaller>(); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; }), base::BindOnce( [](base::OnceClosure quit_closure, Error error) { @@ -3278,7 +3432,7 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { base::OnceClosure quit_closure = runloop.QuitClosure(); auto config = base::MakeRefCounted<TestConfigurator>(); - scoped_refptr<ComponentUnpacker> component_unpacker = new ComponentUnpacker( + auto component_unpacker = base::MakeRefCounted<ComponentUnpacker>( std::vector<uint8_t>(std::begin(gjpm_hash), std::end(gjpm_hash)), TestFilePath("runaction_test_win.crx3"), nullptr, config->CreateServiceManagerConnector()); @@ -3317,15 +3471,16 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { ids, base::BindOnce( [](const base::FilePath& unpack_path, - const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { - CrxComponent crx; - crx.name = "test_niea"; - crx.pk_hash.assign(gjpm_hash, gjpm_hash + arraysize(gjpm_hash)); - crx.version = base::Version("1.0"); - crx.installer = + const std::vector<std::string>& ids) { + auto crx = std::make_unique<CrxComponent>(); + crx->name = "test_niea"; + crx->pk_hash.assign(gjpm_hash, gjpm_hash + base::size(gjpm_hash)); + crx->version = base::Version("1.0"); + crx->installer = base::MakeRefCounted<ReadOnlyTestInstaller>(unpack_path); - components->push_back(crx); + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx)); + return component; }, unpack_path), false, diff --git a/chromium/components/update_client/update_engine.cc b/chromium/components/update_client/update_engine.cc index 1f33635b12b..351cb3893c2 100644 --- a/chromium/components/update_client/update_engine.cc +++ b/chromium/components/update_client/update_engine.cc @@ -97,31 +97,44 @@ void UpdateEngine::Update(bool is_foreground, // Calls out to get the corresponding CrxComponent data for the CRXs in this // update context. - DCHECK_EQ(ids.size(), update_context->ids.size()); - DCHECK_EQ(update_context->ids.size(), update_context->components.size()); - std::vector<CrxComponent> crx_components; - std::move(update_context->crx_data_callback) - .Run(update_context->ids, &crx_components); + std::vector<std::unique_ptr<CrxComponent>> crx_components = + std::move(update_context->crx_data_callback).Run(update_context->ids); DCHECK_EQ(update_context->ids.size(), crx_components.size()); for (size_t i = 0; i != update_context->ids.size(); ++i) { const auto& id = update_context->ids[i]; - const auto& crx_component = crx_components[i]; - DCHECK_EQ(id, GetCrxComponentID(crx_component)); - DCHECK_EQ(1u, update_context->components.count(id)); - DCHECK(update_context->components.at(id)); + DCHECK(update_context->components[id]->state() == ComponentState::kNew); + + auto& crx_component = crx_components[i]; + if (crx_component) { + // This component can be checked for updates. + DCHECK_EQ(id, GetCrxComponentID(*crx_component)); + auto& component = update_context->components[id]; + component->set_crx_component(std::move(crx_component)); + component->set_previous_version(component->crx_component()->version); + component->set_previous_fp(component->crx_component()->fingerprint); + update_context->components_to_check_for_updates.push_back(id); + } else { + // |CrxDataCallback| did not return a CrxComponent instance for this + // component, which most likely, has been uninstalled. This component + // is going to be transitioned to an error state when the its |Handle| + // method is called later on by the engine. + update_context->component_queue.push(id); + } + } - auto& component = *update_context->components.at(id); - component.set_crx_component(crx_component); - component.set_previous_version(crx_component.version); - component.set_previous_fp(crx_component.fingerprint); + if (update_context->components_to_check_for_updates.empty()) { + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&UpdateEngine::HandleComponent, + base::Unretained(this), update_context)); + return; + } - // Handle |kNew| state. This will transition the components to |kChecking|. - component.Handle( + for (const auto& id : update_context->components_to_check_for_updates) + update_context->components[id]->Handle( base::BindOnce(&UpdateEngine::ComponentCheckingForUpdatesStart, base::Unretained(this), update_context, id)); - } } void UpdateEngine::ComponentCheckingForUpdatesStart( @@ -141,7 +154,7 @@ void UpdateEngine::ComponentCheckingForUpdatesStart( ++update_context->num_components_ready_to_check; if (update_context->num_components_ready_to_check < - update_context->ids.size()) { + update_context->components_to_check_for_updates.size()) { return; } @@ -158,7 +171,8 @@ void UpdateEngine::DoUpdateCheck(scoped_refptr<UpdateContext> update_context) { update_checker_factory_(config_, metadata_.get()); update_context->update_checker->CheckForUpdates( - update_context->session_id, update_context->ids, + update_context->session_id, + update_context->components_to_check_for_updates, update_context->components, config_->ExtraRequestParams(), update_context->enabled_component_updates, base::BindOnce(&UpdateEngine::UpdateCheckDone, base::Unretained(this), @@ -188,7 +202,7 @@ void UpdateEngine::UpdateCheckDone(scoped_refptr<UpdateContext> update_context, update_context->update_check_error = error; - for (const auto& id : update_context->ids) { + for (const auto& id : update_context->components_to_check_for_updates) { DCHECK_EQ(1u, update_context->components.count(id)); DCHECK(update_context->components.at(id)); @@ -203,7 +217,8 @@ void UpdateEngine::ComponentCheckingForUpdatesComplete( DCHECK(update_context); ++update_context->num_components_checked; - if (update_context->num_components_checked < update_context->ids.size()) { + if (update_context->num_components_checked < + update_context->components_to_check_for_updates.size()) { return; } @@ -217,7 +232,7 @@ void UpdateEngine::UpdateCheckComplete( DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(update_context); - for (const auto& id : update_context->ids) + for (const auto& id : update_context->components_to_check_for_updates) update_context->component_queue.push(id); base::ThreadTaskRunnerHandle::Get()->PostTask( @@ -314,7 +329,7 @@ bool UpdateEngine::GetUpdateState(const std::string& id, for (const auto& context : update_contexts_) { const auto& components = context.second->components; const auto it = components.find(id); - if (it != components.end()) { + if (it != components.end() && it->second->crx_component()) { *update_item = it->second->GetCrxUpdateItem(); return true; } diff --git a/chromium/components/update_client/update_engine.h b/chromium/components/update_client/update_engine.h index 3bc8f383721..2cfe9da0465 100644 --- a/chromium/components/update_client/update_engine.h +++ b/chromium/components/update_client/update_engine.h @@ -157,10 +157,20 @@ struct UpdateContext : public base::RefCounted<UpdateContext> { // The time in seconds to wait until doing further update checks. int retry_after_sec = 0; + // Contains the ids of the components to check for updates. It is possible + // for a component to be uninstalled after it has been added in this context + // but before an update check is made. When this happens, the component won't + // have a CrxComponent instance, therefore, it can't be included in an + // update check. + std::vector<std::string> components_to_check_for_updates; + + // The error reported by the update checker. int update_check_error = 0; + size_t num_components_ready_to_check = 0; size_t num_components_checked = 0; + // Contains the ids of the components that the state machine must handle. base::queue<std::string> component_queue; // The time to wait before handling the update for a component. diff --git a/chromium/components/update_client/url_fetcher_downloader.cc b/chromium/components/update_client/url_fetcher_downloader.cc index 7db726b4bd9..d181daa0202 100644 --- a/chromium/components/update_client/url_fetcher_downloader.cc +++ b/chromium/components/update_client/url_fetcher_downloader.cc @@ -142,6 +142,7 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) { net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DISABLE_CACHE); url_fetcher_->SetAutomaticallyRetryOn5xx(false); + url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(3); url_fetcher_->SaveResponseToFileAtPath( response, base::CreateSequencedTaskRunnerWithTraits(kTaskTraits)); data_use_measurement::DataUseUserData::AttachToFetcher( diff --git a/chromium/components/update_client/url_request_post_interceptor.cc b/chromium/components/update_client/url_request_post_interceptor.cc index 2bf72e1c05b..b177f533287 100644 --- a/chromium/components/update_client/url_request_post_interceptor.cc +++ b/chromium/components/update_client/url_request_post_interceptor.cc @@ -10,6 +10,7 @@ #include "base/files/file_util.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/sequenced_task_runner.h" #include "base/strings/stringprintf.h" #include "components/update_client/test_configurator.h" #include "net/base/upload_bytes_element_reader.h" diff --git a/chromium/components/update_client/utils.cc b/chromium/components/update_client/utils.cc index 19b93a09688..1213c1f5b57 100644 --- a/chromium/components/update_client/utils.cc +++ b/chromium/components/update_client/utils.cc @@ -86,6 +86,7 @@ std::unique_ptr<net::URLFetcher> SendProtocolRequest( net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DISABLE_CACHE); url_fetcher->SetAutomaticallyRetryOn5xx(false); + url_fetcher->SetAutomaticallyRetryOnNetworkChanges(3); for (const auto& header : protocol_request_extra_headers) url_fetcher->AddExtraRequestHeader(base::StringPrintf( "%s: %s", header.first.c_str(), header.second.c_str())); diff --git a/chromium/components/update_client/utils.h b/chromium/components/update_client/utils.h index 54eb873a774..e2840f9dcbb 100644 --- a/chromium/components/update_client/utils.h +++ b/chromium/components/update_client/utils.h @@ -97,6 +97,18 @@ CrxInstaller::Result InstallFunctionWrapper( std::unique_ptr<base::DictionaryValue> ReadManifest( const base::FilePath& unpack_path); +// Converts a custom, specific installer error (and optionally extended error) +// to an installer result. +template <typename T> +CrxInstaller::Result ToInstallerResult(const T& error, int extended_error = 0) { + static_assert(std::is_enum<T>::value, + "Use an enum class to define custom installer errors"); + return CrxInstaller::Result( + static_cast<int>(update_client::InstallError::CUSTOM_ERROR_BASE) + + static_cast<int>(error), + extended_error); +} + } // namespace update_client #endif // COMPONENTS_UPDATE_CLIENT_UTILS_H_ diff --git a/chromium/components/update_client/utils_unittest.cc b/chromium/components/update_client/utils_unittest.cc index 77ea7ae3b51..e2670e9ba9d 100644 --- a/chromium/components/update_client/utils_unittest.cc +++ b/chromium/components/update_client/utils_unittest.cc @@ -15,7 +15,7 @@ namespace { base::FilePath MakeTestFilePath(const char* file) { base::FilePath path; - PathService::Get(base::DIR_SOURCE_ROOT, &path); + base::PathService::Get(base::DIR_SOURCE_ROOT, &path); return path.AppendASCII("components/test/data/update_client") .AppendASCII(file); } @@ -160,4 +160,32 @@ TEST(UpdateClientUtils, RemoveUnsecureUrls) { EXPECT_EQ(0u, urls.size()); } +TEST(UpdateClientUtils, ToInstallerResult) { + enum EnumA { + ENTRY0 = 10, + ENTRY1 = 20, + }; + + enum class EnumB { + ENTRY0 = 0, + ENTRY1, + }; + + const auto result1 = ToInstallerResult(EnumA::ENTRY0); + EXPECT_EQ(110, result1.error); + EXPECT_EQ(0, result1.extended_error); + + const auto result2 = ToInstallerResult(ENTRY1, 10000); + EXPECT_EQ(120, result2.error); + EXPECT_EQ(10000, result2.extended_error); + + const auto result3 = ToInstallerResult(EnumB::ENTRY0); + EXPECT_EQ(100, result3.error); + EXPECT_EQ(0, result3.extended_error); + + const auto result4 = ToInstallerResult(EnumB::ENTRY1, 20000); + EXPECT_EQ(101, result4.error); + EXPECT_EQ(20000, result4.extended_error); +} + } // namespace update_client |