diff options
Diffstat (limited to 'chromium/components/update_client')
50 files changed, 910 insertions, 887 deletions
diff --git a/chromium/components/update_client/BUILD.gn b/chromium/components/update_client/BUILD.gn index d33cc6c7320..675c7be083c 100644 --- a/chromium/components/update_client/BUILD.gn +++ b/chromium/components/update_client/BUILD.gn @@ -24,7 +24,6 @@ static_library("update_client") { "crx_downloader.cc", "crx_downloader.h", "crx_update_item.h", - "out_of_process_patcher.h", "persisted_data.cc", "persisted_data.h", "ping_manager.cc", @@ -68,6 +67,7 @@ static_library("update_client") { "//components/client_update_protocol", "//components/crx_file", "//components/data_use_measurement/core", + "//components/patch_service/public/cpp", "//components/prefs", "//components/version_info:version_info", "//courgette:courgette_lib", @@ -99,8 +99,10 @@ static_library("test_support") { ] deps = [ "//base", + "//components/patch_service:lib", "//components/prefs", "//net:test_support", + "//services/service_manager/public/cpp/test:test_support", "//testing/gmock", "//testing/gtest", "//url", @@ -158,11 +160,14 @@ source_set("unit_tests") { ":update_client", "//base", "//components/crx_file", + "//components/patch_service:lib", "//components/prefs", "//components/prefs:test_support", "//components/version_info:version_info", "//courgette:courgette_lib", "//net:test_support", + "//services/service_manager/public/cpp", + "//services/service_manager/public/cpp/test:test_support", "//testing/gmock", "//testing/gtest", "//third_party/libxml", diff --git a/chromium/components/update_client/DEPS b/chromium/components/update_client/DEPS index ca6755e33e6..30e7de9951f 100644 --- a/chromium/components/update_client/DEPS +++ b/chromium/components/update_client/DEPS @@ -2,12 +2,14 @@ include_rules = [ "+components/client_update_protocol", "+components/crx_file", "+components/data_use_measurement/core", + "+components/patch_service", "+components/prefs", "+components/version_info", "+courgette", "+crypto", "+libxml", "+net", + "+services/service_manager/public", "+third_party/libxml", "+third_party/zlib", ] diff --git a/chromium/components/update_client/action_runner.cc b/chromium/components/update_client/action_runner.cc index 6d382bee24e..058750b1fdc 100644 --- a/chromium/components/update_client/action_runner.cc +++ b/chromium/components/update_client/action_runner.cc @@ -13,9 +13,7 @@ #include "base/files/file_util.h" #include "base/location.h" #include "base/logging.h" -#include "base/single_thread_task_runner.h" -#include "base/threading/sequenced_task_runner_handle.h" -#include "base/threading/thread_task_runner_handle.h" +#include "base/task_scheduler/post_task.h" #include "components/update_client/component.h" #include "components/update_client/task_traits.h" #include "components/update_client/update_client.h" @@ -32,13 +30,14 @@ ActionRunner::~ActionRunner() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); } -void ActionRunner::Run(const Callback& run_complete) { +void ActionRunner::Run(Callback run_complete) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - run_complete_ = run_complete; + run_complete_ = std::move(run_complete); - base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(&ActionRunner::Unpack, base::Unretained(this))); + base::CreateSequencedTaskRunnerWithTraits(kTaskTraits) + ->PostTask(FROM_HERE, + base::BindOnce(&ActionRunner::Unpack, base::Unretained(this))); } void ActionRunner::Unpack() { @@ -50,7 +49,7 @@ void ActionRunner::Unpack() { auto unpacker = base::MakeRefCounted<ComponentUnpacker>(key_hash_, file_path, installer, nullptr); unpacker->Unpack( - base::Bind(&ActionRunner::UnpackComplete, base::Unretained(this))); + base::BindOnce(&ActionRunner::UnpackComplete, base::Unretained(this))); } void ActionRunner::UnpackComplete(const ComponentUnpacker::Result& result) { @@ -59,8 +58,8 @@ void ActionRunner::UnpackComplete(const ComponentUnpacker::Result& result) { main_task_runner_->PostTask( FROM_HERE, - base::BindOnce(run_complete_, false, static_cast<int>(result.error), - result.extended_error)); + base::BindOnce(std::move(run_complete_), false, + static_cast<int>(result.error), result.extended_error)); return; } @@ -75,8 +74,8 @@ void ActionRunner::UnpackComplete(const ComponentUnpacker::Result& result) { void ActionRunner::RunCommand(const base::CommandLine& cmdline) { base::DeleteFile(unpack_path_, true); - main_task_runner_->PostTask(FROM_HERE, - base::BindOnce(run_complete_, false, -1, 0)); + main_task_runner_->PostTask( + FROM_HERE, base::BindOnce(std::move(run_complete_), false, -1, 0)); } base::CommandLine ActionRunner::MakeCommandLine( diff --git a/chromium/components/update_client/action_runner.h b/chromium/components/update_client/action_runner.h index 2364284d23e..075507d7d37 100644 --- a/chromium/components/update_client/action_runner.h +++ b/chromium/components/update_client/action_runner.h @@ -30,13 +30,13 @@ class Component; class ActionRunner { public: using Callback = - base::Callback<void(bool succeeded, int error_code, int extra_code1)>; + base::OnceCallback<void(bool succeeded, int error_code, int extra_code1)>; ActionRunner(const Component& component, const std::vector<uint8_t>& key_hash); ~ActionRunner(); - void Run(const Callback& run_complete); + void Run(Callback run_complete); private: void Unpack(); diff --git a/chromium/components/update_client/action_runner_win.cc b/chromium/components/update_client/action_runner_win.cc index 2ea7eaa3f40..00084944365 100644 --- a/chromium/components/update_client/action_runner_win.cc +++ b/chromium/components/update_client/action_runner_win.cc @@ -43,7 +43,8 @@ void ActionRunner::WaitForCommand(base::Process process) { process.WaitForExitWithTimeout(kMaxWaitTime, &exit_code); base::DeleteFile(unpack_path_, true); main_task_runner_->PostTask( - FROM_HERE, base::BindOnce(run_complete_, succeeded, exit_code, 0)); + FROM_HERE, + base::BindOnce(std::move(run_complete_), succeeded, exit_code, 0)); } base::CommandLine ActionRunner::MakeCommandLine( diff --git a/chromium/components/update_client/background_downloader_win.cc b/chromium/components/update_client/background_downloader_win.cc index cee9c0dca93..53cf6a759fe 100644 --- a/chromium/components/update_client/background_downloader_win.cc +++ b/chromium/components/update_client/background_downloader_win.cc @@ -35,8 +35,8 @@ #include "components/update_client/utils.h" #include "url/gurl.h" -using base::win::ScopedCoMem; using Microsoft::WRL::ComPtr; +using base::win::ScopedCoMem; // The class BackgroundDownloader in this module is an adapter between // the CrxDownloader interface and the BITS service interfaces. diff --git a/chromium/components/update_client/component.cc b/chromium/components/update_client/component.cc index 608b7ccd00c..907554146bb 100644 --- a/chromium/components/update_client/component.cc +++ b/chromium/components/update_client/component.cc @@ -63,12 +63,12 @@ namespace update_client { namespace { -using InstallOnBlockingTaskRunnerCompleteCallback = - base::Callback<void(int error_category, int error_code, int extra_code1)>; +using InstallOnBlockingTaskRunnerCompleteCallback = base::OnceCallback< + void(int error_category, int error_code, int extra_code1)>; void InstallComplete( const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner, - const InstallOnBlockingTaskRunnerCompleteCallback& callback, + InstallOnBlockingTaskRunnerCompleteCallback callback, const base::FilePath& unpack_path, const CrxInstaller::Result& result) { base::PostTaskWithTraits( @@ -76,7 +76,7 @@ void InstallComplete( base::BindOnce( [](const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner, - const InstallOnBlockingTaskRunnerCompleteCallback& callback, + InstallOnBlockingTaskRunnerCompleteCallback callback, const base::FilePath& unpack_path, const CrxInstaller::Result& result) { base::DeleteFile(unpack_path, true); @@ -84,20 +84,21 @@ void InstallComplete( result.error ? ErrorCategory::kInstallError : ErrorCategory::kErrorNone; main_task_runner->PostTask( - FROM_HERE, - base::BindOnce(callback, static_cast<int>(error_category), - static_cast<int>(result.error), - result.extended_error)); + FROM_HERE, base::BindOnce(std::move(callback), + static_cast<int>(error_category), + static_cast<int>(result.error), + result.extended_error)); }, - main_task_runner, callback, unpack_path, result)); + main_task_runner, std::move(callback), unpack_path, result)); } void InstallOnBlockingTaskRunner( const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner, const base::FilePath& unpack_path, + const std::string& public_key, const std::string& fingerprint, const scoped_refptr<CrxInstaller>& installer, - const InstallOnBlockingTaskRunnerCompleteCallback& callback) { + InstallOnBlockingTaskRunnerCompleteCallback callback) { DCHECK(base::DirectoryExists(unpack_path)); // Acquire the ownership of the |unpack_path|. @@ -111,24 +112,16 @@ void InstallOnBlockingTaskRunner( const CrxInstaller::Result result(InstallError::FINGERPRINT_WRITE_FAILED); main_task_runner->PostTask( FROM_HERE, - base::BindOnce(callback, static_cast<int>(ErrorCategory::kInstallError), + base::BindOnce(std::move(callback), + static_cast<int>(ErrorCategory::kInstallError), static_cast<int>(result.error), result.extended_error)); return; } - std::unique_ptr<base::DictionaryValue> manifest = ReadManifest(unpack_path); - if (!manifest) { - const CrxInstaller::Result result(InstallError::BAD_MANIFEST); - main_task_runner->PostTask( - FROM_HERE, - base::BindOnce(callback, static_cast<int>(ErrorCategory::kInstallError), - static_cast<int>(result.error), result.extended_error)); - return; - } - - installer->Install(std::move(manifest), unpack_path, - base::Bind(&InstallComplete, main_task_runner, callback, - unpack_path_owner.Take())); + installer->Install( + unpack_path, public_key, + base::BindOnce(&InstallComplete, main_task_runner, std::move(callback), + unpack_path_owner.Take())); } void UnpackCompleteOnBlockingTaskRunner( @@ -136,14 +129,15 @@ void UnpackCompleteOnBlockingTaskRunner( const base::FilePath& crx_path, const std::string& fingerprint, const scoped_refptr<CrxInstaller>& installer, - const InstallOnBlockingTaskRunnerCompleteCallback& callback, + InstallOnBlockingTaskRunnerCompleteCallback callback, const ComponentUnpacker::Result& result) { update_client::DeleteFileAndEmptyParentDirectory(crx_path); if (result.error != UnpackerError::kNone) { main_task_runner->PostTask( FROM_HERE, - base::BindOnce(callback, static_cast<int>(ErrorCategory::kUnpackError), + base::BindOnce(std::move(callback), + static_cast<int>(ErrorCategory::kUnpackError), static_cast<int>(result.error), result.extended_error)); return; } @@ -151,7 +145,8 @@ void UnpackCompleteOnBlockingTaskRunner( base::PostTaskWithTraits( FROM_HERE, kTaskTraits, base::BindOnce(&InstallOnBlockingTaskRunner, main_task_runner, - result.unpack_path, fingerprint, installer, callback)); + result.unpack_path, result.public_key, fingerprint, + installer, std::move(callback))); } void StartInstallOnBlockingTaskRunner( @@ -160,14 +155,14 @@ void StartInstallOnBlockingTaskRunner( const base::FilePath& crx_path, const std::string& fingerprint, const scoped_refptr<CrxInstaller>& installer, - const scoped_refptr<OutOfProcessPatcher>& oop_patcher, - const InstallOnBlockingTaskRunnerCompleteCallback& callback) { + std::unique_ptr<service_manager::Connector> connector, + InstallOnBlockingTaskRunnerCompleteCallback callback) { auto unpacker = base::MakeRefCounted<ComponentUnpacker>( - pk_hash, crx_path, installer, oop_patcher); + pk_hash, crx_path, installer, std::move(connector)); - unpacker->Unpack(base::Bind(&UnpackCompleteOnBlockingTaskRunner, - main_task_runner, crx_path, fingerprint, - installer, callback)); + unpacker->Unpack(base::BindOnce(&UnpackCompleteOnBlockingTaskRunner, + main_task_runner, crx_path, fingerprint, + installer, std::move(callback))); } } // namespace @@ -183,9 +178,10 @@ void Component::Handle(CallbackHandleComplete callback) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(state_); - callback_handle_complete_ = callback; + callback_handle_complete_ = std::move(callback); - state_->Handle(base::Bind(&Component::ChangeState, base::Unretained(this))); + state_->Handle( + base::BindOnce(&Component::ChangeState, base::Unretained(this))); } void Component::ChangeState(std::unique_ptr<State> next_state) { @@ -195,8 +191,8 @@ void Component::ChangeState(std::unique_ptr<State> next_state) { if (next_state) state_ = std::move(next_state); - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - callback_handle_complete_); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, std::move(callback_handle_complete_)); } CrxUpdateItem Component::GetCrxUpdateItem() const { @@ -256,13 +252,13 @@ void Component::Uninstall(const base::Version& version, int reason) { state_ = base::MakeUnique<StateUninstalled>(this); } -void Component::UpdateCheckComplete() const { +void Component::UpdateCheckComplete() { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_EQ(ComponentState::kChecking, state()); - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - update_check_complete_); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, std::move(update_check_complete_)); } bool Component::CanDoBackgroundDownload() const { @@ -301,7 +297,7 @@ Component::State::~State() {} void Component::State::Handle(CallbackNextState callback) { DCHECK(thread_checker_.CalledOnValidThread()); - callback_ = callback; + callback_ = std::move(callback); DCHECK(!is_final_); DoHandle(); @@ -312,7 +308,8 @@ void Component::State::TransitionState(std::unique_ptr<State> next_state) { is_final_ = true; base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback(), base::Passed(&next_state))); + FROM_HERE, + base::BindOnce(std::move(callback_), base::Passed(&next_state))); } Component::StateNew::StateNew(Component* component) @@ -349,7 +346,7 @@ void Component::StateChecking::DoHandle() { auto& component = State::component(); component.last_check_ = base::TimeTicks::Now(); - component.update_check_complete_ = base::Bind( + component.update_check_complete_ = base::BindOnce( &Component::StateChecking::UpdateCheckComplete, base::Unretained(this)); component.NotifyObservers(Events::COMPONENT_CHECKING_FOR_UPDATES); @@ -476,8 +473,8 @@ void Component::StateDownloadingDiff::DoHandle() { base::Unretained(this), id)); crx_downloader_->StartDownload( component.crx_diffurls_, component.hashdiff_sha256_, - base::Bind(&Component::StateDownloadingDiff::DownloadComplete, - base::Unretained(this), id)); + base::BindOnce(&Component::StateDownloadingDiff::DownloadComplete, + base::Unretained(this), id)); component.NotifyObservers(Events::COMPONENT_UPDATE_DOWNLOADING); } @@ -543,8 +540,8 @@ void Component::StateDownloading::DoHandle() { base::Unretained(this), id)); crx_downloader_->StartDownload( component.crx_urls_, component.hash_sha256_, - base::Bind(&Component::StateDownloading::DownloadComplete, - base::Unretained(this), id)); + base::BindOnce(&Component::StateDownloading::DownloadComplete, + base::Unretained(this), id)); component.NotifyObservers(Events::COMPONENT_UPDATE_DOWNLOADING); } @@ -601,16 +598,21 @@ void Component::StateUpdatingDiff::DoHandle() { component.NotifyObservers(Events::COMPONENT_UPDATE_READY); + // Create a fresh connector that can be used on the other task runner. + std::unique_ptr<service_manager::Connector> connector = + update_context.config->CreateServiceManagerConnector(); + base::CreateSequencedTaskRunnerWithTraits(kTaskTraits) - ->PostTask(FROM_HERE, - base::BindOnce( - &update_client::StartInstallOnBlockingTaskRunner, - base::ThreadTaskRunnerHandle::Get(), - component.crx_component_.pk_hash, component.crx_path_, - component.next_fp_, component.crx_component_.installer, - update_context.config->CreateOutOfProcessPatcher(), - base::Bind(&Component::StateUpdatingDiff::InstallComplete, - base::Unretained(this)))); + ->PostTask( + FROM_HERE, + base::BindOnce( + &update_client::StartInstallOnBlockingTaskRunner, + base::ThreadTaskRunnerHandle::Get(), + component.crx_component_.pk_hash, component.crx_path_, + component.next_fp_, component.crx_component_.installer, + base::Passed(&connector), + base::BindOnce(&Component::StateUpdatingDiff::InstallComplete, + base::Unretained(this)))); } void Component::StateUpdatingDiff::InstallComplete(int error_category, @@ -660,15 +662,19 @@ void Component::StateUpdating::DoHandle() { component.NotifyObservers(Events::COMPONENT_UPDATE_READY); + // Create a fresh connector that can be used on the other task runner. + std::unique_ptr<service_manager::Connector> connector = + update_context.config->CreateServiceManagerConnector(); + base::CreateSequencedTaskRunnerWithTraits(kTaskTraits) - ->PostTask( - FROM_HERE, - base::BindOnce(&update_client::StartInstallOnBlockingTaskRunner, - base::ThreadTaskRunnerHandle::Get(), - component.crx_component_.pk_hash, component.crx_path_, - component.next_fp_, component.crx_component_.installer, - update_context.config->CreateOutOfProcessPatcher(), - base::Bind(&Component::StateUpdating::InstallComplete, + ->PostTask(FROM_HERE, + base::BindOnce( + &update_client::StartInstallOnBlockingTaskRunner, + base::ThreadTaskRunnerHandle::Get(), + component.crx_component_.pk_hash, component.crx_path_, + component.next_fp_, component.crx_component_.installer, + base::Passed(&connector), + base::BindOnce(&Component::StateUpdating::InstallComplete, base::Unretained(this)))); } @@ -754,7 +760,7 @@ void Component::StateRun::DoHandle() { component, component.update_context_.config->GetRunActionKeyHash()); action_runner_->Run( - base::Bind(&StateRun::ActionRunComplete, base::Unretained(this))); + base::BindOnce(&StateRun::ActionRunComplete, base::Unretained(this))); } void Component::StateRun::ActionRunComplete(bool succeeded, diff --git a/chromium/components/update_client/component.h b/chromium/components/update_client/component.h index 5b85bd89fb0..34cde414e2a 100644 --- a/chromium/components/update_client/component.h +++ b/chromium/components/update_client/component.h @@ -37,7 +37,7 @@ class Component { public: using Events = UpdateClient::Observer::Events; - using CallbackHandleComplete = base::Callback<void()>; + using CallbackHandleComplete = base::OnceCallback<void()>; Component(const UpdateContext& update_context, const std::string& id); ~Component(); @@ -55,7 +55,7 @@ class Component { void Uninstall(const base::Version& cur_version, int reason); // Called by the UpdateEngine when an update check for this component is done. - void UpdateCheckComplete() const; + void UpdateCheckComplete(); // Returns true if the component has reached a final state and no further // handling and state transitions are possible. @@ -139,7 +139,7 @@ class Component { class State { public: using CallbackNextState = - base::Callback<void(std::unique_ptr<State> next_state)>; + base::OnceCallback<void(std::unique_ptr<State> next_state)>; State(Component* component, ComponentState state); virtual ~State(); @@ -160,8 +160,6 @@ class Component { Component& component() { return component_; } const Component& component() const { return component_; } - CallbackNextState callback() const { return callback_; } - base::ThreadChecker thread_checker_; const ComponentState state_; @@ -432,7 +430,7 @@ class Component { std::unique_ptr<State> state_; const UpdateContext& update_context_; - base::Closure update_check_complete_; + base::OnceClosure update_check_complete_; ComponentState previous_state_ = ComponentState::kLastStatus; diff --git a/chromium/components/update_client/component_patcher.cc b/chromium/components/update_client/component_patcher.cc index a69860c5b85..81aa9f557cc 100644 --- a/chromium/components/update_client/component_patcher.cc +++ b/chromium/components/update_client/component_patcher.cc @@ -5,6 +5,7 @@ #include "components/update_client/component_patcher.h" #include <string> +#include <utility> #include <vector> #include "base/bind.h" @@ -19,6 +20,7 @@ #include "components/update_client/component_patcher_operation.h" #include "components/update_client/update_client.h" #include "components/update_client/update_client_errors.h" +#include "services/service_manager/public/cpp/connector.h" namespace update_client { @@ -30,14 +32,15 @@ base::ListValue* ReadCommands(const base::FilePath& unpack_path) { const base::FilePath commands = unpack_path.Append(FILE_PATH_LITERAL("commands.json")); if (!base::PathExists(commands)) - return NULL; + return nullptr; JSONFileValueDeserializer deserializer(commands); - std::unique_ptr<base::Value> root = deserializer.Deserialize(NULL, NULL); + std::unique_ptr<base::Value> root = + deserializer.Deserialize(nullptr, nullptr); - return (root.get() && root->IsType(base::Value::Type::LIST)) + return (root.get() && root->is_list()) ? static_cast<base::ListValue*>(root.release()) - : NULL; + : nullptr; } } // namespace @@ -46,17 +49,17 @@ ComponentPatcher::ComponentPatcher( const base::FilePath& input_dir, const base::FilePath& unpack_dir, scoped_refptr<CrxInstaller> installer, - scoped_refptr<OutOfProcessPatcher> out_of_process_patcher) + std::unique_ptr<service_manager::Connector> connector) : input_dir_(input_dir), unpack_dir_(unpack_dir), installer_(installer), - out_of_process_patcher_(out_of_process_patcher) {} + connector_(std::move(connector)) {} ComponentPatcher::~ComponentPatcher() { } -void ComponentPatcher::Start(const Callback& callback) { - callback_ = callback; +void ComponentPatcher::Start(Callback callback) { + callback_ = std::move(callback); base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&ComponentPatcher::StartPatching, scoped_refptr<ComponentPatcher>(this))); @@ -85,17 +88,17 @@ void ComponentPatcher::PatchNextFile() { std::string operation; if (command_args->GetString(kOp, &operation)) { - current_operation_ = - CreateDeltaUpdateOp(operation, out_of_process_patcher_); + current_operation_ = CreateDeltaUpdateOp(operation, connector_.get()); } if (!current_operation_.get()) { DonePatching(UnpackerError::kDeltaUnsupportedCommand, 0); return; } - current_operation_->Run(command_args, input_dir_, unpack_dir_, installer_, - base::Bind(&ComponentPatcher::DonePatchingFile, - scoped_refptr<ComponentPatcher>(this))); + current_operation_->Run( + command_args, input_dir_, unpack_dir_, installer_, + base::BindOnce(&ComponentPatcher::DonePatchingFile, + scoped_refptr<ComponentPatcher>(this))); } void ComponentPatcher::DonePatchingFile(UnpackerError error, @@ -109,10 +112,9 @@ void ComponentPatcher::DonePatchingFile(UnpackerError error, } void ComponentPatcher::DonePatching(UnpackerError error, int extended_error) { - current_operation_ = NULL; + current_operation_ = nullptr; base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback_, error, extended_error)); - callback_.Reset(); + FROM_HERE, base::BindOnce(std::move(callback_), error, extended_error)); } } // namespace update_client diff --git a/chromium/components/update_client/component_patcher.h b/chromium/components/update_client/component_patcher.h index 2179778ad94..75eebe58f59 100644 --- a/chromium/components/update_client/component_patcher.h +++ b/chromium/components/update_client/component_patcher.h @@ -39,11 +39,14 @@ namespace base { class FilePath; } +namespace service_manager { +class Connector; +} + namespace update_client { class CrxInstaller; class DeltaUpdateOp; -class OutOfProcessPatcher; enum class UnpackerError; // The type of a patch file. @@ -56,7 +59,7 @@ enum PatchType { // Encapsulates a task for applying a differential update to a component. class ComponentPatcher : public base::RefCountedThreadSafe<ComponentPatcher> { public: - using Callback = base::Callback<void(UnpackerError, int)>; + using Callback = base::OnceCallback<void(UnpackerError, int)>; // Takes an unpacked differential CRX (|input_dir|) and a component installer, // and sets up the class to create a new (non-differential) unpacked CRX. @@ -66,13 +69,13 @@ class ComponentPatcher : public base::RefCountedThreadSafe<ComponentPatcher> { ComponentPatcher(const base::FilePath& input_dir, const base::FilePath& unpack_dir, scoped_refptr<CrxInstaller> installer, - scoped_refptr<OutOfProcessPatcher> out_of_process_patcher); + std::unique_ptr<service_manager::Connector> connector); // Starts patching files. This member function returns immediately, after // posting a task to do the patching. When patching has been completed, // |callback| will be called with the error codes if any error codes were // encountered. - void Start(const Callback& callback); + void Start(Callback callback); private: friend class base::RefCountedThreadSafe<ComponentPatcher>; @@ -90,7 +93,7 @@ class ComponentPatcher : public base::RefCountedThreadSafe<ComponentPatcher> { const base::FilePath input_dir_; const base::FilePath unpack_dir_; scoped_refptr<CrxInstaller> installer_; - scoped_refptr<OutOfProcessPatcher> out_of_process_patcher_; + std::unique_ptr<service_manager::Connector> connector_; Callback callback_; std::unique_ptr<base::ListValue> commands_; base::ListValue::const_iterator next_command_; diff --git a/chromium/components/update_client/component_patcher_operation.cc b/chromium/components/update_client/component_patcher_operation.cc index 6fbde6b7aef..99906e495ad 100644 --- a/chromium/components/update_client/component_patcher_operation.cc +++ b/chromium/components/update_client/component_patcher_operation.cc @@ -5,6 +5,7 @@ #include "components/update_client/component_patcher_operation.h" #include <stdint.h> +#include <utility> #include <vector> #include "base/bind.h" @@ -14,7 +15,7 @@ #include "base/strings/string_number_conversions.h" #include "base/task_scheduler/post_task.h" #include "base/threading/sequenced_task_runner_handle.h" -#include "components/update_client/out_of_process_patcher.h" +#include "components/patch_service/public/cpp/patch.h" #include "components/update_client/update_client.h" #include "components/update_client/update_client_errors.h" #include "components/update_client/utils.h" @@ -40,17 +41,16 @@ const char kCourgette[] = "courgette"; const char kInput[] = "input"; const char kPatch[] = "patch"; -DeltaUpdateOp* CreateDeltaUpdateOp( - const std::string& operation, - const scoped_refptr<OutOfProcessPatcher>& out_of_process_patcher) { +DeltaUpdateOp* CreateDeltaUpdateOp(const std::string& operation, + service_manager::Connector* connector) { if (operation == "copy") { return new DeltaUpdateOpCopy(); } else if (operation == "create") { return new DeltaUpdateOpCreate(); } else if (operation == "bsdiff" || operation == "courgette") { - return new DeltaUpdateOpPatch(operation, out_of_process_patcher); + return new DeltaUpdateOpPatch(operation, connector); } - return NULL; + return nullptr; } DeltaUpdateOp::DeltaUpdateOp() { @@ -63,8 +63,8 @@ void DeltaUpdateOp::Run(const base::DictionaryValue* command_args, const base::FilePath& input_dir, const base::FilePath& unpack_dir, const scoped_refptr<CrxInstaller>& installer, - const ComponentPatcher::Callback& callback) { - callback_ = callback; + ComponentPatcher::Callback callback) { + callback_ = std::move(callback); std::string output_rel_path; if (!command_args->GetString(kOutput, &output_rel_path) || !command_args->GetString(kSha256, &output_sha256_)) { @@ -89,16 +89,15 @@ void DeltaUpdateOp::Run(const base::DictionaryValue* command_args, } } - DoRun(base::Bind(&DeltaUpdateOp::DoneRunning, - scoped_refptr<DeltaUpdateOp>(this))); + DoRun(base::BindOnce(&DeltaUpdateOp::DoneRunning, + scoped_refptr<DeltaUpdateOp>(this))); } void DeltaUpdateOp::DoneRunning(UnpackerError error, int extended_error) { if (error == UnpackerError::kNone) error = CheckHash(); base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback_, error, extended_error)); - callback_.Reset(); + FROM_HERE, base::BindOnce(std::move(callback_), error, extended_error)); } // Uses the hash as a checksum to confirm that the file now residing in the @@ -129,11 +128,11 @@ UnpackerError DeltaUpdateOpCopy::DoParseArguments( return UnpackerError::kNone; } -void DeltaUpdateOpCopy::DoRun(const ComponentPatcher::Callback& callback) { +void DeltaUpdateOpCopy::DoRun(ComponentPatcher::Callback callback) { if (!base::CopyFile(input_abs_path_, output_abs_path_)) - callback.Run(UnpackerError::kDeltaOperationFailure, 0); + std::move(callback).Run(UnpackerError::kDeltaOperationFailure, 0); else - callback.Run(UnpackerError::kNone, 0); + std::move(callback).Run(UnpackerError::kNone, 0); } DeltaUpdateOpCreate::DeltaUpdateOpCreate() { @@ -156,17 +155,16 @@ UnpackerError DeltaUpdateOpCreate::DoParseArguments( return UnpackerError::kNone; } -void DeltaUpdateOpCreate::DoRun(const ComponentPatcher::Callback& callback) { +void DeltaUpdateOpCreate::DoRun(ComponentPatcher::Callback callback) { if (!base::Move(patch_abs_path_, output_abs_path_)) - callback.Run(UnpackerError::kDeltaOperationFailure, 0); + std::move(callback).Run(UnpackerError::kDeltaOperationFailure, 0); else - callback.Run(UnpackerError::kNone, 0); + std::move(callback).Run(UnpackerError::kNone, 0); } -DeltaUpdateOpPatch::DeltaUpdateOpPatch( - const std::string& operation, - const scoped_refptr<OutOfProcessPatcher>& out_of_process_patcher) - : operation_(operation), out_of_process_patcher_(out_of_process_patcher) { +DeltaUpdateOpPatch::DeltaUpdateOpPatch(const std::string& operation, + service_manager::Connector* connector) + : operation_(operation), connector_(connector) { DCHECK(operation == kBsdiff || operation == kCourgette); } @@ -192,44 +190,28 @@ UnpackerError DeltaUpdateOpPatch::DoParseArguments( return UnpackerError::kNone; } -void DeltaUpdateOpPatch::DoRun(const ComponentPatcher::Callback& callback) { - if (out_of_process_patcher_.get()) { - out_of_process_patcher_->Patch( - operation_, input_abs_path_, patch_abs_path_, output_abs_path_, - base::Bind(&DeltaUpdateOpPatch::DonePatching, this, callback)); - return; - } - - if (operation_ == kBsdiff) { - DonePatching(callback, - bsdiff::ApplyBinaryPatch(input_abs_path_, patch_abs_path_, - output_abs_path_)); - } else if (operation_ == kCourgette) { - DonePatching(callback, courgette::ApplyEnsemblePatch( - input_abs_path_.value().c_str(), - patch_abs_path_.value().c_str(), - output_abs_path_.value().c_str())); - } else { - NOTREACHED(); - } +void DeltaUpdateOpPatch::DoRun(ComponentPatcher::Callback callback) { + patch::Patch(connector_, operation_, input_abs_path_, patch_abs_path_, + output_abs_path_, + base::Bind(&DeltaUpdateOpPatch::DonePatching, this, + base::Passed(&callback))); } -void DeltaUpdateOpPatch::DonePatching( - const ComponentPatcher::Callback& callback, - int result) { +void DeltaUpdateOpPatch::DonePatching(ComponentPatcher::Callback callback, + int result) { if (operation_ == kBsdiff) { if (result == bsdiff::OK) { - callback.Run(UnpackerError::kNone, 0); + std::move(callback).Run(UnpackerError::kNone, 0); } else { - callback.Run(UnpackerError::kDeltaOperationFailure, - result + kBsdiffErrorOffset); + std::move(callback).Run(UnpackerError::kDeltaOperationFailure, + result + kBsdiffErrorOffset); } } else if (operation_ == kCourgette) { if (result == courgette::C_OK) { - callback.Run(UnpackerError::kNone, 0); + std::move(callback).Run(UnpackerError::kNone, 0); } else { - callback.Run(UnpackerError::kDeltaOperationFailure, - result + kCourgetteErrorOffset); + std::move(callback).Run(UnpackerError::kDeltaOperationFailure, + result + kCourgetteErrorOffset); } } else { NOTREACHED(); diff --git a/chromium/components/update_client/component_patcher_operation.h b/chromium/components/update_client/component_patcher_operation.h index 553f76f8d49..1f99ca17228 100644 --- a/chromium/components/update_client/component_patcher_operation.h +++ b/chromium/components/update_client/component_patcher_operation.h @@ -18,6 +18,10 @@ namespace base { class DictionaryValue; } // namespace base +namespace service_manager { +class Connector; +} + namespace update_client { extern const char kOp[]; @@ -27,7 +31,6 @@ extern const char kInput[]; extern const char kPatch[]; class CrxInstaller; -class OutOfProcessPatcher; enum class UnpackerError; class DeltaUpdateOp : public base::RefCountedThreadSafe<DeltaUpdateOp> { @@ -40,7 +43,7 @@ class DeltaUpdateOp : public base::RefCountedThreadSafe<DeltaUpdateOp> { const base::FilePath& input_dir, const base::FilePath& unpack_dir, const scoped_refptr<CrxInstaller>& installer, - const ComponentPatcher::Callback& callback); + ComponentPatcher::Callback callback); protected: virtual ~DeltaUpdateOp(); @@ -64,7 +67,7 @@ class DeltaUpdateOp : public base::RefCountedThreadSafe<DeltaUpdateOp> { // Subclasses must override DoRun to actually perform the patching operation. // They must call the provided callback when they have completed their // operations. In practice, the provided callback is always for "DoneRunning". - virtual void DoRun(const ComponentPatcher::Callback& callback) = 0; + virtual void DoRun(ComponentPatcher::Callback callback) = 0; // Callback given to subclasses for when they complete their operation. // Validates the output, and posts a task to the patching operation's @@ -92,7 +95,7 @@ class DeltaUpdateOpCopy : public DeltaUpdateOp { const base::FilePath& input_dir, const scoped_refptr<CrxInstaller>& installer) override; - void DoRun(const ComponentPatcher::Callback& callback) override; + void DoRun(ComponentPatcher::Callback callback) override; base::FilePath input_abs_path_; @@ -116,7 +119,7 @@ class DeltaUpdateOpCreate : public DeltaUpdateOp { const base::FilePath& input_dir, const scoped_refptr<CrxInstaller>& installer) override; - void DoRun(const ComponentPatcher::Callback& callback) override; + void DoRun(ComponentPatcher::Callback callback) override; base::FilePath patch_abs_path_; @@ -129,10 +132,8 @@ class DeltaUpdateOpCreate : public DeltaUpdateOp { // unpacking directory. class DeltaUpdateOpPatch : public DeltaUpdateOp { public: - // |out_of_process_patcher| may be NULL. - DeltaUpdateOpPatch( - const std::string& operation, - const scoped_refptr<OutOfProcessPatcher>& out_of_process_patcher); + DeltaUpdateOpPatch(const std::string& operation, + service_manager::Connector* connector); private: ~DeltaUpdateOpPatch() override; @@ -143,23 +144,22 @@ class DeltaUpdateOpPatch : public DeltaUpdateOp { const base::FilePath& input_dir, const scoped_refptr<CrxInstaller>& installer) override; - void DoRun(const ComponentPatcher::Callback& callback) override; + void DoRun(ComponentPatcher::Callback callback) override; // |success_code| is the code that indicates a successful patch. // |result| is the code the patching operation returned. - void DonePatching(const ComponentPatcher::Callback& callback, int result); + void DonePatching(ComponentPatcher::Callback callback, int result); std::string operation_; - const scoped_refptr<OutOfProcessPatcher> out_of_process_patcher_; + service_manager::Connector* connector_; base::FilePath patch_abs_path_; base::FilePath input_abs_path_; DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOpPatch); }; -DeltaUpdateOp* CreateDeltaUpdateOp( - const std::string& operation, - const scoped_refptr<OutOfProcessPatcher>& out_of_process_patcher); +DeltaUpdateOp* CreateDeltaUpdateOp(const std::string& operation, + service_manager::Connector* connector); } // namespace update_client diff --git a/chromium/components/update_client/component_patcher_unittest.cc b/chromium/components/update_client/component_patcher_unittest.cc index b6dfe2cbfed..af431818652 100644 --- a/chromium/components/update_client/component_patcher_unittest.cc +++ b/chromium/components/update_client/component_patcher_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "components/update_client/component_patcher.h" #include "base/base_paths.h" #include "base/bind.h" #include "base/bind_helpers.h" @@ -12,13 +13,14 @@ #include "base/path_service.h" #include "base/run_loop.h" #include "base/values.h" -#include "components/update_client/component_patcher.h" +#include "components/patch_service/patch_service.h" #include "components/update_client/component_patcher_operation.h" #include "components/update_client/component_patcher_unittest.h" #include "components/update_client/test_installer.h" #include "components/update_client/update_client_errors.h" #include "courgette/courgette.h" #include "courgette/third_party/bsdiff/bsdiff.h" +#include "services/service_manager/public/cpp/test/test_connector_factory.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -81,17 +83,18 @@ TEST_F(ComponentPatcherOperationTest, CheckCreateOperation) { test_file("binary_output.bin"), input_dir_.GetPath().Append(FILE_PATH_LITERAL("binary_output.bin")))); - std::unique_ptr<base::DictionaryValue> command_args( - new base::DictionaryValue()); + std::unique_ptr<base::DictionaryValue> command_args = + std::make_unique<base::DictionaryValue>(); command_args->SetString("output", "output.bin"); command_args->SetString("sha256", binary_output_hash); command_args->SetString("op", "create"); command_args->SetString("patch", "binary_output.bin"); TestCallback callback; - scoped_refptr<DeltaUpdateOp> op = new DeltaUpdateOpCreate(); - op->Run(command_args.get(), input_dir_.GetPath(), unpack_dir_.GetPath(), NULL, - base::Bind(&TestCallback::Set, base::Unretained(&callback))); + scoped_refptr<DeltaUpdateOp> op = base::MakeRefCounted<DeltaUpdateOpCreate>(); + op->Run(command_args.get(), input_dir_.GetPath(), unpack_dir_.GetPath(), + nullptr, + base::BindOnce(&TestCallback::Set, base::Unretained(&callback))); scoped_task_environment_.RunUntilIdle(); EXPECT_EQ(true, callback.called_); @@ -108,18 +111,18 @@ TEST_F(ComponentPatcherOperationTest, CheckCopyOperation) { test_file("binary_output.bin"), installed_dir_.GetPath().Append(FILE_PATH_LITERAL("binary_output.bin")))); - std::unique_ptr<base::DictionaryValue> command_args( - new base::DictionaryValue()); + std::unique_ptr<base::DictionaryValue> command_args = + std::make_unique<base::DictionaryValue>(); command_args->SetString("output", "output.bin"); command_args->SetString("sha256", binary_output_hash); command_args->SetString("op", "copy"); command_args->SetString("input", "binary_output.bin"); TestCallback callback; - scoped_refptr<DeltaUpdateOp> op = new DeltaUpdateOpCopy(); + scoped_refptr<DeltaUpdateOp> op = base::MakeRefCounted<DeltaUpdateOpCopy>(); op->Run(command_args.get(), input_dir_.GetPath(), unpack_dir_.GetPath(), installer_.get(), - base::Bind(&TestCallback::Set, base::Unretained(&callback))); + base::BindOnce(&TestCallback::Set, base::Unretained(&callback))); scoped_task_environment_.RunUntilIdle(); EXPECT_EQ(true, callback.called_); @@ -139,20 +142,26 @@ TEST_F(ComponentPatcherOperationTest, CheckCourgetteOperation) { input_dir_.GetPath().Append(FILE_PATH_LITERAL( "binary_courgette_patch.bin")))); - std::unique_ptr<base::DictionaryValue> command_args( - new base::DictionaryValue()); + std::unique_ptr<base::DictionaryValue> command_args = + std::make_unique<base::DictionaryValue>(); command_args->SetString("output", "output.bin"); command_args->SetString("sha256", binary_output_hash); command_args->SetString("op", "courgette"); command_args->SetString("input", "binary_input.bin"); command_args->SetString("patch", "binary_courgette_patch.bin"); + // The operation needs a connector to access the PatchService. + service_manager::TestConnectorFactory connector_factory( + std::make_unique<patch::PatchService>()); + std::unique_ptr<service_manager::Connector> connector = + connector_factory.CreateConnector(); + TestCallback callback; scoped_refptr<DeltaUpdateOp> op = - CreateDeltaUpdateOp("courgette", NULL /* out_of_process_patcher */); + CreateDeltaUpdateOp("courgette", connector.get()); op->Run(command_args.get(), input_dir_.GetPath(), unpack_dir_.GetPath(), installer_.get(), - base::Bind(&TestCallback::Set, base::Unretained(&callback))); + base::BindOnce(&TestCallback::Set, base::Unretained(&callback))); scoped_task_environment_.RunUntilIdle(); EXPECT_EQ(true, callback.called_); @@ -172,20 +181,26 @@ TEST_F(ComponentPatcherOperationTest, CheckBsdiffOperation) { input_dir_.GetPath().Append(FILE_PATH_LITERAL( "binary_bsdiff_patch.bin")))); - std::unique_ptr<base::DictionaryValue> command_args( - new base::DictionaryValue()); + std::unique_ptr<base::DictionaryValue> command_args = + std::make_unique<base::DictionaryValue>(); command_args->SetString("output", "output.bin"); command_args->SetString("sha256", binary_output_hash); command_args->SetString("op", "courgette"); command_args->SetString("input", "binary_input.bin"); command_args->SetString("patch", "binary_bsdiff_patch.bin"); + // The operation needs a connector to access the PatchService. + service_manager::TestConnectorFactory connector_factory( + std::make_unique<patch::PatchService>()); + std::unique_ptr<service_manager::Connector> connector = + connector_factory.CreateConnector(); + TestCallback callback; scoped_refptr<DeltaUpdateOp> op = - CreateDeltaUpdateOp("bsdiff", NULL /* out_of_process_patcher */); + CreateDeltaUpdateOp("bsdiff", connector.get()); op->Run(command_args.get(), input_dir_.GetPath(), unpack_dir_.GetPath(), installer_.get(), - base::Bind(&TestCallback::Set, base::Unretained(&callback))); + base::BindOnce(&TestCallback::Set, base::Unretained(&callback))); scoped_task_environment_.RunUntilIdle(); EXPECT_EQ(true, callback.called_); diff --git a/chromium/components/update_client/component_unpacker.cc b/chromium/components/update_client/component_unpacker.cc index f9ef280666d..c642af3ed7c 100644 --- a/chromium/components/update_client/component_unpacker.cc +++ b/chromium/components/update_client/component_unpacker.cc @@ -6,13 +6,13 @@ #include <stdint.h> #include <string> +#include <utility> #include <vector> #include "base/bind.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_file.h" -#include "base/json/json_file_value_serializer.h" #include "base/location.h" #include "base/logging.h" #include "base/macros.h" @@ -20,7 +20,6 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/threading/sequenced_task_runner_handle.h" -#include "base/values.h" #include "components/crx_file/crx_verifier.h" #include "components/update_client/component_patcher.h" #include "components/update_client/update_client.h" @@ -29,38 +28,18 @@ namespace update_client { -// TODO(cpu): add a specific attribute check to a component json that the -// extension unpacker will reject, so that a component cannot be installed -// as an extension. -std::unique_ptr<base::DictionaryValue> ReadManifest( - const base::FilePath& unpack_path) { - base::FilePath manifest = - unpack_path.Append(FILE_PATH_LITERAL("manifest.json")); - if (!base::PathExists(manifest)) - return std::unique_ptr<base::DictionaryValue>(); - JSONFileValueDeserializer deserializer(manifest); - std::string error; - std::unique_ptr<base::Value> root = deserializer.Deserialize(NULL, &error); - if (!root.get()) - return std::unique_ptr<base::DictionaryValue>(); - if (!root->IsType(base::Value::Type::DICTIONARY)) - return std::unique_ptr<base::DictionaryValue>(); - return std::unique_ptr<base::DictionaryValue>( - static_cast<base::DictionaryValue*>(root.release())); -} - ComponentUnpacker::Result::Result() {} ComponentUnpacker::ComponentUnpacker( const std::vector<uint8_t>& pk_hash, const base::FilePath& path, const scoped_refptr<CrxInstaller>& installer, - const scoped_refptr<OutOfProcessPatcher>& oop_patcher) + std::unique_ptr<service_manager::Connector> connector) : pk_hash_(pk_hash), path_(path), is_delta_(false), installer_(installer), - oop_patcher_(oop_patcher), + connector_(std::move(connector)), error_(UnpackerError::kNone), extended_error_(0) {} @@ -70,8 +49,8 @@ bool ComponentUnpacker::UnpackInternal() { return Verify() && Unzip() && BeginPatching(); } -void ComponentUnpacker::Unpack(const Callback& callback) { - callback_ = callback; +void ComponentUnpacker::Unpack(Callback callback) { + callback_ = std::move(callback); if (!UnpackInternal()) EndUnpacking(); } @@ -83,9 +62,9 @@ bool ComponentUnpacker::Verify() { return false; } const std::vector<std::vector<uint8_t>> required_keys = {pk_hash_}; - const crx_file::VerifierResult result = - crx_file::Verify(path_, crx_file::VerifierFormat::CRX2_OR_CRX3, - required_keys, std::vector<uint8_t>(), nullptr, nullptr); + const crx_file::VerifierResult result = crx_file::Verify( + path_, crx_file::VerifierFormat::CRX2_OR_CRX3, required_keys, + std::vector<uint8_t>(), &public_key_, nullptr); if (result != crx_file::VerifierResult::OK_FULL && result != crx_file::VerifierResult::OK_DELTA) { error_ = UnpackerError::kInvalidFile; @@ -125,12 +104,12 @@ bool ComponentUnpacker::BeginPatching() { return false; } patcher_ = base::MakeRefCounted<ComponentPatcher>( - unpack_diff_path_, unpack_path_, installer_, oop_patcher_); + unpack_diff_path_, unpack_path_, installer_, std::move(connector_)); base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&ComponentPatcher::Start, patcher_, - base::Bind(&ComponentUnpacker::EndPatching, - scoped_refptr<ComponentUnpacker>(this)))); + base::BindOnce(&ComponentUnpacker::EndPatching, + scoped_refptr<ComponentUnpacker>(this)))); } else { base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&ComponentUnpacker::EndPatching, @@ -143,7 +122,7 @@ bool ComponentUnpacker::BeginPatching() { void ComponentUnpacker::EndPatching(UnpackerError error, int extended_error) { error_ = error; extended_error_ = extended_error; - patcher_ = NULL; + patcher_ = nullptr; EndUnpacking(); } @@ -157,11 +136,13 @@ void ComponentUnpacker::EndUnpacking() { Result result; result.error = error_; result.extended_error = extended_error_; - if (error_ == UnpackerError::kNone) + if (error_ == UnpackerError::kNone) { result.unpack_path = unpack_path_; + result.public_key = public_key_; + } base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback_, result)); + FROM_HERE, base::BindOnce(std::move(callback_), result)); } } // namespace update_client diff --git a/chromium/components/update_client/component_unpacker.h b/chromium/components/update_client/component_unpacker.h index d0b5259fb5c..7a6fc76a550 100644 --- a/chromium/components/update_client/component_unpacker.h +++ b/chromium/components/update_client/component_unpacker.h @@ -13,21 +13,16 @@ #include "base/callback.h" #include "base/files/file_path.h" -#include "base/json/json_file_value_serializer.h" #include "base/macros.h" #include "base/memory/ref_counted.h" -#include "components/update_client/out_of_process_patcher.h" #include "components/update_client/update_client_errors.h" +#include "services/service_manager/public/cpp/connector.h" namespace update_client { class CrxInstaller; class ComponentPatcher; -// Deserializes the CRX manifest. The top level must be a dictionary. -std::unique_ptr<base::DictionaryValue> ReadManifest( - const base::FilePath& unpack_path); - // In charge of unpacking the component CRX package and verifying that it is // well formed and the cryptographic signature is correct. // @@ -76,9 +71,12 @@ class ComponentUnpacker : public base::RefCountedThreadSafe<ComponentUnpacker> { // Path of the unpacked files if the unpacking was successful. base::FilePath unpack_path; + + // The extracted public key of the package if the unpacking was successful. + std::string public_key; }; - using Callback = base::Callback<void(const Result& result)>; + using Callback = base::OnceCallback<void(const Result& result)>; // Constructs an unpacker for a specific component unpacking operation. // |pk_hash| is the expected/ public key SHA256 hash. |path| is the current @@ -86,12 +84,12 @@ class ComponentUnpacker : public base::RefCountedThreadSafe<ComponentUnpacker> { ComponentUnpacker(const std::vector<uint8_t>& pk_hash, const base::FilePath& path, const scoped_refptr<CrxInstaller>& installer, - const scoped_refptr<OutOfProcessPatcher>& oop_patcher); + std::unique_ptr<service_manager::Connector> connector); // Begins the actual unpacking of the files. May invoke a patcher and the // component installer if the package is a differential update. // Calls |callback| with the result. - void Unpack(const Callback& callback); + void Unpack(Callback callback); private: friend class base::RefCountedThreadSafe<ComponentUnpacker>; @@ -129,9 +127,10 @@ class ComponentUnpacker : public base::RefCountedThreadSafe<ComponentUnpacker> { scoped_refptr<ComponentPatcher> patcher_; scoped_refptr<CrxInstaller> installer_; Callback callback_; - scoped_refptr<OutOfProcessPatcher> oop_patcher_; + std::unique_ptr<service_manager::Connector> connector_; UnpackerError error_; int extended_error_; + std::string public_key_; DISALLOW_COPY_AND_ASSIGN(ComponentUnpacker); }; diff --git a/chromium/components/update_client/component_unpacker_unittest.cc b/chromium/components/update_client/component_unpacker_unittest.cc index 65eca6d7049..3ba343c8b91 100644 --- a/chromium/components/update_client/component_unpacker_unittest.cc +++ b/chromium/components/update_client/component_unpacker_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include <iterator> +#include <utility> #include <vector> #include "base/bind.h" @@ -79,7 +80,7 @@ class ComponentUnpackerTest : public testing::Test { const scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_ = base::ThreadTaskRunnerHandle::Get(); base::RunLoop runloop_; - const base::Closure quit_closure_ = runloop_.QuitClosure(); + base::OnceClosure quit_closure_ = runloop_.QuitClosure(); ComponentUnpacker::Result result_; }; @@ -95,7 +96,7 @@ void ComponentUnpackerTest::RunThreads() { void ComponentUnpackerTest::UnpackComplete( const ComponentUnpacker::Result& result) { result_ = result; - main_thread_task_runner_->PostTask(FROM_HERE, quit_closure_); + main_thread_task_runner_->PostTask(FROM_HERE, std::move(quit_closure_)); } TEST_F(ComponentUnpackerTest, UnpackFullCrx) { @@ -103,8 +104,8 @@ TEST_F(ComponentUnpackerTest, UnpackFullCrx) { base::MakeRefCounted<ComponentUnpacker>( std::vector<uint8_t>(std::begin(jebg_hash), std::end(jebg_hash)), test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"), nullptr, nullptr); - component_unpacker->Unpack(base::Bind(&ComponentUnpackerTest::UnpackComplete, - base::Unretained(this))); + component_unpacker->Unpack(base::BindOnce( + &ComponentUnpackerTest::UnpackComplete, base::Unretained(this))); RunThreads(); EXPECT_EQ(UnpackerError::kNone, result_.error); @@ -113,6 +114,7 @@ TEST_F(ComponentUnpackerTest, UnpackFullCrx) { base::FilePath unpack_path = result_.unpack_path; EXPECT_FALSE(unpack_path.empty()); EXPECT_TRUE(base::DirectoryExists(unpack_path)); + EXPECT_EQ(jebg_public_key, result_.public_key); int64_t file_size = 0; EXPECT_TRUE( @@ -133,8 +135,8 @@ TEST_F(ComponentUnpackerTest, UnpackFileNotFound) { base::MakeRefCounted<ComponentUnpacker>( std::vector<uint8_t>(std::begin(jebg_hash), std::end(jebg_hash)), test_file("file-not-found.crx"), nullptr, nullptr); - component_unpacker->Unpack(base::Bind(&ComponentUnpackerTest::UnpackComplete, - base::Unretained(this))); + component_unpacker->Unpack(base::BindOnce( + &ComponentUnpackerTest::UnpackComplete, base::Unretained(this))); RunThreads(); EXPECT_EQ(UnpackerError::kInvalidFile, result_.error); @@ -150,8 +152,8 @@ TEST_F(ComponentUnpackerTest, UnpackFileHashMismatch) { base::MakeRefCounted<ComponentUnpacker>( std::vector<uint8_t>(std::begin(abag_hash), std::end(abag_hash)), test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"), nullptr, nullptr); - component_unpacker->Unpack(base::Bind(&ComponentUnpackerTest::UnpackComplete, - base::Unretained(this))); + component_unpacker->Unpack(base::BindOnce( + &ComponentUnpackerTest::UnpackComplete, base::Unretained(this))); RunThreads(); EXPECT_EQ(UnpackerError::kInvalidFile, result_.error); diff --git a/chromium/components/update_client/configurator.h b/chromium/components/update_client/configurator.h index 99ebcbc18c1..fd29c79c801 100644 --- a/chromium/components/update_client/configurator.h +++ b/chromium/components/update_client/configurator.h @@ -22,10 +22,13 @@ namespace net { class URLRequestContextGetter; } +namespace service_manager { +class Connector; +} + namespace update_client { class ActivityDataService; -class OutOfProcessPatcher; // Controls the component updater behavior. // TODO(sorin): this class will be split soon in two. One class controls @@ -95,10 +98,10 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> { // The source of contexts for all the url requests. virtual net::URLRequestContextGetter* RequestContext() const = 0; - // Returns a new out of process patcher. May be NULL for implementations - // that patch in-process. - virtual scoped_refptr<update_client::OutOfProcessPatcher> - CreateOutOfProcessPatcher() const = 0; + // Returns a new connector to the service manager. That connector is not bound + // to any thread yet. + virtual std::unique_ptr<service_manager::Connector> + CreateServiceManagerConnector() const = 0; // True means that this client can handle delta updates. virtual bool EnabledDeltas() const = 0; diff --git a/chromium/components/update_client/crx_downloader.cc b/chromium/components/update_client/crx_downloader.cc index ee80fc98a31..a73bfc50cc7 100644 --- a/chromium/components/update_client/crx_downloader.cc +++ b/chromium/components/update_client/crx_downloader.cc @@ -82,18 +82,17 @@ CrxDownloader::download_metrics() const { return retval; } -void CrxDownloader::StartDownloadFromUrl( - const GURL& url, - const std::string& expected_hash, - const DownloadCallback& download_callback) { +void CrxDownloader::StartDownloadFromUrl(const GURL& url, + const std::string& expected_hash, + DownloadCallback download_callback) { std::vector<GURL> urls; urls.push_back(url); - StartDownload(urls, expected_hash, download_callback); + StartDownload(urls, expected_hash, std::move(download_callback)); } void CrxDownloader::StartDownload(const std::vector<GURL>& urls, const std::string& expected_hash, - const DownloadCallback& download_callback) { + DownloadCallback download_callback) { DCHECK(thread_checker_.CalledOnValidThread()); auto error = CrxDownloaderError::NONE; @@ -106,15 +105,15 @@ void CrxDownloader::StartDownload(const std::vector<GURL>& urls, if (error != CrxDownloaderError::NONE) { Result result; result.error = static_cast<int>(error); - main_task_runner()->PostTask(FROM_HERE, - base::BindOnce(download_callback, result)); + main_task_runner()->PostTask( + FROM_HERE, base::BindOnce(std::move(download_callback), result)); return; } urls_ = urls; expected_hash_ = expected_hash; current_url_ = urls_.begin(); - download_callback_ = download_callback; + download_callback_ = std::move(download_callback); DoStartDownload(*current_url_); } @@ -157,8 +156,8 @@ void CrxDownloader::VerifyResponse(bool is_handled, if (VerifyFileHash256(result.response, expected_hash_)) { download_metrics_.push_back(download_metrics); - main_task_runner()->PostTask(FROM_HERE, - base::BindOnce(download_callback_, result)); + main_task_runner()->PostTask( + FROM_HERE, base::BindOnce(std::move(download_callback_), result)); return; } @@ -206,14 +205,15 @@ void CrxDownloader::HandleDownloadError( // Try downloading using the next downloader. if (successor_ && !urls_.empty()) { - successor_->StartDownload(urls_, expected_hash_, download_callback_); + successor_->StartDownload(urls_, expected_hash_, + std::move(download_callback_)); return; } // The download ends here since there is no url nor downloader to handle this // download request further. - main_task_runner()->PostTask(FROM_HERE, - base::BindOnce(download_callback_, result)); + main_task_runner()->PostTask( + FROM_HERE, base::BindOnce(std::move(download_callback_), result)); } } // namespace update_client diff --git a/chromium/components/update_client/crx_downloader.h b/chromium/components/update_client/crx_downloader.h index 6c27ca77ac7..544a91514d1 100644 --- a/chromium/components/update_client/crx_downloader.h +++ b/chromium/components/update_client/crx_downloader.h @@ -78,7 +78,7 @@ class CrxDownloader { // download. The callback interface can be extended if needed to provide // more visibility into how the download has been handled, including // specific error codes and download metrics. - using DownloadCallback = base::Callback<void(const Result& result)>; + using DownloadCallback = base::OnceCallback<void(const Result& result)>; // The callback may fire 0 or many times during a download. Since this // class implements a chain of responsibility, the callback can fire for @@ -108,10 +108,10 @@ class CrxDownloader { // the download payload, represented as a hexadecimal string. void StartDownloadFromUrl(const GURL& url, const std::string& expected_hash, - const DownloadCallback& download_callback); + DownloadCallback download_callback); void StartDownload(const std::vector<GURL>& urls, const std::string& expected_hash, - const DownloadCallback& download_callback); + DownloadCallback download_callback); const std::vector<DownloadMetrics> download_metrics() const; diff --git a/chromium/components/update_client/crx_downloader_unittest.cc b/chromium/components/update_client/crx_downloader_unittest.cc index 40440085ab1..b87257d5378 100644 --- a/chromium/components/update_client/crx_downloader_unittest.cc +++ b/chromium/components/update_client/crx_downloader_unittest.cc @@ -5,6 +5,7 @@ #include "components/update_client/crx_downloader.h" #include <memory> +#include <utility> #include "base/bind.h" #include "base/bind_helpers.h" @@ -87,15 +88,15 @@ class CrxDownloaderTest : public testing::Test { private: base::test::ScopedTaskEnvironment scoped_task_environment_; scoped_refptr<net::TestURLRequestContextGetter> context_; - base::Closure quit_closure_; + base::OnceClosure quit_closure_; }; const int CrxDownloaderTest::kExpectedContext; CrxDownloaderTest::CrxDownloaderTest() - : callback_(base::Bind(&CrxDownloaderTest::DownloadComplete, - base::Unretained(this), - kExpectedContext)), + : callback_(base::BindOnce(&CrxDownloaderTest::DownloadComplete, + base::Unretained(this), + kExpectedContext)), progress_callback_(base::Bind(&CrxDownloaderTest::DownloadProgress, base::Unretained(this), kExpectedContext)), @@ -104,11 +105,11 @@ CrxDownloaderTest::CrxDownloaderTest() num_progress_calls_(0), scoped_task_environment_( base::test::ScopedTaskEnvironment::MainThreadType::IO), - context_(new net::TestURLRequestContextGetter( + context_(base::MakeRefCounted<net::TestURLRequestContextGetter>( base::ThreadTaskRunnerHandle::Get())) {} CrxDownloaderTest::~CrxDownloaderTest() { - context_ = NULL; + context_ = nullptr; // The GetInterceptor requires the message loop to run to destruct correctly. get_interceptor_.reset(); @@ -122,12 +123,11 @@ void CrxDownloaderTest::SetUp() { download_progress_result_ = CrxDownloader::Result(); // Do not use the background downloader in these tests. - crx_downloader_.reset(CrxDownloader::Create(false, context_.get()).release()); + crx_downloader_ = CrxDownloader::Create(false, context_.get()); crx_downloader_->set_progress_callback(progress_callback_); - get_interceptor_.reset( - new GetInterceptor(base::ThreadTaskRunnerHandle::Get(), - base::ThreadTaskRunnerHandle::Get())); + get_interceptor_ = std::make_unique<GetInterceptor>( + base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get()); } void CrxDownloaderTest::TearDown() { @@ -136,7 +136,7 @@ void CrxDownloaderTest::TearDown() { void CrxDownloaderTest::Quit() { if (!quit_closure_.is_null()) - quit_closure_.Run(); + std::move(quit_closure_).Run(); } void CrxDownloaderTest::DownloadComplete(int crx_context, @@ -173,7 +173,8 @@ void CrxDownloaderTest::RunThreadsUntilIdle() { // Tests that starting a download without a url results in an error. TEST_F(CrxDownloaderTest, NoUrl) { std::vector<GURL> urls; - crx_downloader_->StartDownload(urls, std::string("abcd"), callback_); + crx_downloader_->StartDownload(urls, std::string("abcd"), + std::move(callback_)); RunThreadsUntilIdle(); EXPECT_EQ(1, num_download_complete_calls_); @@ -190,7 +191,7 @@ TEST_F(CrxDownloaderTest, NoUrl) { TEST_F(CrxDownloaderTest, NoHash) { std::vector<GURL> urls(1, GURL("http://somehost/somefile")); - crx_downloader_->StartDownload(urls, std::string(), callback_); + crx_downloader_->StartDownload(urls, std::string(), std::move(callback_)); RunThreadsUntilIdle(); EXPECT_EQ(1, num_download_complete_calls_); @@ -211,8 +212,8 @@ TEST_F(CrxDownloaderTest, OneUrl) { const base::FilePath test_file(MakeTestFilePath(kTestFileName)); get_interceptor_->SetResponse(expected_crx_url, test_file); - crx_downloader_->StartDownloadFromUrl(expected_crx_url, - std::string(hash_jebg), callback_); + crx_downloader_->StartDownloadFromUrl( + expected_crx_url, std::string(hash_jebg), std::move(callback_)); RunThreads(); EXPECT_EQ(1, get_interceptor_->GetHitCount()); @@ -245,7 +246,7 @@ TEST_F(CrxDownloaderTest, OneUrlBadHash) { expected_crx_url, std::string( "813c59747e139a608b3b5fc49633affc6db574373f309f156ea6d27229c0b3f9"), - callback_); + std::move(callback_)); RunThreads(); EXPECT_EQ(1, get_interceptor_->GetHitCount()); @@ -282,7 +283,8 @@ TEST_F(CrxDownloaderTest, MAYBE_TwoUrls) { urls.push_back(expected_crx_url); urls.push_back(expected_crx_url); - crx_downloader_->StartDownload(urls, std::string(hash_jebg), callback_); + crx_downloader_->StartDownload(urls, std::string(hash_jebg), + std::move(callback_)); RunThreads(); EXPECT_EQ(1, get_interceptor_->GetHitCount()); @@ -313,7 +315,7 @@ TEST_F(CrxDownloaderTest, OneUrl_InvalidHost) { crx_downloader_->StartDownloadFromUrl( GURL("http://no.such.host" "/download/jebgalgnebhfojomionfpkfelancnnkf.crx"), - std::string(hash_jebg), callback_); + std::string(hash_jebg), std::move(callback_)); RunThreads(); EXPECT_EQ(0, get_interceptor_->GetHitCount()); @@ -333,7 +335,8 @@ TEST_F(CrxDownloaderTest, OneUrl_InvalidPath) { get_interceptor_->SetResponse(expected_crx_url, test_file); crx_downloader_->StartDownloadFromUrl(GURL("http://localhost/no/such/file"), - std::string(hash_jebg), callback_); + std::string(hash_jebg), + std::move(callback_)); RunThreads(); EXPECT_EQ(0, get_interceptor_->GetHitCount()); @@ -362,7 +365,8 @@ TEST_F(CrxDownloaderTest, MAYBE_TwoUrls_FirstInvalid) { urls.push_back(GURL("http://localhost/no/such/file")); urls.push_back(expected_crx_url); - crx_downloader_->StartDownload(urls, std::string(hash_jebg), callback_); + crx_downloader_->StartDownload(urls, std::string(hash_jebg), + std::move(callback_)); RunThreads(); EXPECT_EQ(1, get_interceptor_->GetHitCount()); @@ -395,7 +399,8 @@ TEST_F(CrxDownloaderTest, TwoUrls_SecondInvalid) { urls.push_back(expected_crx_url); urls.push_back(GURL("http://localhost/no/such/file")); - crx_downloader_->StartDownload(urls, std::string(hash_jebg), callback_); + crx_downloader_->StartDownload(urls, std::string(hash_jebg), + std::move(callback_)); RunThreads(); EXPECT_EQ(1, get_interceptor_->GetHitCount()); @@ -429,7 +434,8 @@ TEST_F(CrxDownloaderTest, TwoUrls_BothInvalid) { "http://no.such.host/" "/download/jebgalgnebhfojomionfpkfelancnnkf.crx")); - crx_downloader_->StartDownload(urls, std::string(hash_jebg), callback_); + crx_downloader_->StartDownload(urls, std::string(hash_jebg), + std::move(callback_)); RunThreads(); EXPECT_EQ(0, get_interceptor_->GetHitCount()); diff --git a/chromium/components/update_client/out_of_process_patcher.h b/chromium/components/update_client/out_of_process_patcher.h deleted file mode 100644 index df0ee37ff79..00000000000 --- a/chromium/components/update_client/out_of_process_patcher.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2017 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_OUT_OF_PROCESS_PATCHER_H_ -#define COMPONENTS_UPDATE_CLIENT_OUT_OF_PROCESS_PATCHER_H_ - -#include <string> - -#include "base/callback_forward.h" -#include "base/memory/ref_counted.h" - -namespace base { -class FilePath; -} - -namespace update_client { - -// An interface an embedder can implement to enable out-of-process patching. -class OutOfProcessPatcher - : public base::RefCountedThreadSafe<OutOfProcessPatcher> { - public: - virtual void Patch( - const std::string& operation, - const base::FilePath& input_abs_path, - const base::FilePath& patch_abs_path, - const base::FilePath& output_abs_path, - const base::Callback<void(int result)>& callback) = 0; - - protected: - friend class base::RefCountedThreadSafe<OutOfProcessPatcher>; - - virtual ~OutOfProcessPatcher() {} -}; - -} // namespace update_client - -#endif // COMPONENTS_UPDATE_CLIENT_OUT_OF_PROCESS_PATCHER_H_ diff --git a/chromium/components/update_client/ping_manager.cc b/chromium/components/update_client/ping_manager.cc index 50d7b2cbe8a..3b148dc3666 100644 --- a/chromium/components/update_client/ping_manager.cc +++ b/chromium/components/update_client/ping_manager.cc @@ -77,9 +77,9 @@ bool PingSender::SendPing(const Component& component) { return false; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send( - false, BuildEventPingRequest(*config_, component), urls, - base::Bind(&PingSender::OnRequestSenderComplete, base::Unretained(this))); + request_sender_->Send(false, BuildEventPingRequest(*config_, component), urls, + base::BindOnce(&PingSender::OnRequestSenderComplete, + base::Unretained(this))); return true; } diff --git a/chromium/components/update_client/protocol_parser.cc b/chromium/components/update_client/protocol_parser.cc index 74527d4b967..4fd3c994748 100644 --- a/chromium/components/update_client/protocol_parser.cc +++ b/chromium/components/update_client/protocol_parser.cc @@ -64,7 +64,7 @@ static bool TagNameEquals(const xmlNode* node, const char* expected_name) { // Returns child nodes of |root| with name |name|. static std::vector<xmlNode*> GetChildren(xmlNode* root, const char* name) { std::vector<xmlNode*> result; - for (xmlNode* child = root->children; child != NULL; child = child->next) { + for (xmlNode* child = root->children; child != nullptr; child = child->next) { if (!TagNameEquals(child, name)) { continue; } @@ -76,7 +76,7 @@ static std::vector<xmlNode*> GetChildren(xmlNode* root, const char* name) { // Returns the value of a named attribute, or the empty string. static std::string GetAttribute(xmlNode* node, const char* attribute_name) { const xmlChar* name = reinterpret_cast<const xmlChar*>(attribute_name); - for (xmlAttr* attr = node->properties; attr != NULL; attr = attr->next) { + for (xmlAttr* attr = node->properties; attr != nullptr; attr = attr->next) { if (!xmlStrcmp(attr->name, name) && attr->children && attr->children->content) { return std::string( @@ -91,7 +91,7 @@ static std::unique_ptr<std::string> GetAttributePtr( xmlNode* node, const char* attribute_name) { const xmlChar* name = reinterpret_cast<const xmlChar*>(attribute_name); - for (xmlAttr* attr = node->properties; attr != NULL; attr = attr->next) { + for (xmlAttr* attr = node->properties; attr != nullptr; attr = attr->next) { if (!xmlStrcmp(attr->name, name) && attr->children && attr->children->content) { return base::MakeUnique<std::string>( diff --git a/chromium/components/update_client/request_sender.cc b/chromium/components/update_client/request_sender.cc index af2b87b5fe7..a1d46ea8095 100644 --- a/chromium/components/update_client/request_sender.cc +++ b/chromium/components/update_client/request_sender.cc @@ -5,6 +5,7 @@ #include "components/update_client/request_sender.h" #include <algorithm> +#include <utility> #include "base/base64.h" #include "base/bind.h" @@ -61,13 +62,13 @@ RequestSender::~RequestSender() { void RequestSender::Send(bool use_signing, const std::string& request_body, const std::vector<GURL>& urls, - const RequestSenderCallback& request_sender_callback) { + RequestSenderCallback request_sender_callback) { DCHECK(thread_checker_.CalledOnValidThread()); use_signing_ = use_signing; request_body_ = request_body; urls_ = urls; - request_sender_callback_ = request_sender_callback; + request_sender_callback_ = std::move(request_sender_callback); if (urls_.empty()) { return HandleSendError(-1, 0); @@ -116,8 +117,8 @@ void RequestSender::SendInternalComplete(int error, if (!error) { if (!use_signing_) { base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(request_sender_callback_, 0, response_body, - retry_after_sec)); + FROM_HERE, base::BindOnce(std::move(request_sender_callback_), 0, + response_body, retry_after_sec)); return; } @@ -125,8 +126,8 @@ void RequestSender::SendInternalComplete(int error, DCHECK(signer_.get()); if (signer_->ValidateResponse(response_body, response_etag)) { base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(request_sender_callback_, 0, response_body, - retry_after_sec)); + FROM_HERE, base::BindOnce(std::move(request_sender_callback_), 0, + response_body, retry_after_sec)); return; } @@ -176,8 +177,8 @@ void RequestSender::OnURLFetchComplete(const net::URLFetcher* source) { void RequestSender::HandleSendError(int error, int retry_after_sec) { base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(request_sender_callback_, error, std::string(), - retry_after_sec)); + FROM_HERE, base::BindOnce(std::move(request_sender_callback_), error, + std::string(), retry_after_sec)); } std::string RequestSender::GetKey(const char* key_bytes_base64) { diff --git a/chromium/components/update_client/request_sender.h b/chromium/components/update_client/request_sender.h index e6ecbda9035..bbba898682b 100644 --- a/chromium/components/update_client/request_sender.h +++ b/chromium/components/update_client/request_sender.h @@ -42,7 +42,7 @@ class RequestSender : public net::URLFetcherDelegate { // range for this value is [-1, 86400]. If |retry_after_sec| is -1 it means // that the header could not be found, or trusted, or had an invalid value. // The upper bound represents a delay of one day. - using RequestSenderCallback = base::Callback< + using RequestSenderCallback = base::OnceCallback< void(int error, const std::string& response, int retry_after_sec)>; // This value is chosen not to conflict with network errors defined by @@ -59,7 +59,7 @@ class RequestSender : public net::URLFetcherDelegate { void Send(bool use_signing, const std::string& request_body, const std::vector<GURL>& urls, - const RequestSenderCallback& request_sender_callback); + RequestSenderCallback request_sender_callback); private: // Combines the |url| and |query_params| parameters. diff --git a/chromium/components/update_client/request_sender_unittest.cc b/chromium/components/update_client/request_sender_unittest.cc index 67ac7dd2844..4ed3d25cf67 100644 --- a/chromium/components/update_client/request_sender_unittest.cc +++ b/chromium/components/update_client/request_sender_unittest.cc @@ -5,6 +5,7 @@ #include "components/update_client/request_sender.h" #include <memory> +#include <utility> #include "base/macros.h" #include "base/memory/ptr_util.h" @@ -72,7 +73,7 @@ class RequestSenderTest : public testing::Test { std::string response_; private: - base::Closure quit_closure_; + base::OnceClosure quit_closure_; DISALLOW_COPY_AND_ASSIGN(RequestSenderTest); }; @@ -119,7 +120,7 @@ void RequestSenderTest::RunThreads() { void RequestSenderTest::Quit() { if (!quit_closure_.is_null()) - quit_closure_.Run(); + std::move(quit_closure_).Run(); } void RequestSenderTest::RequestSenderComplete(int error, @@ -139,9 +140,10 @@ TEST_F(RequestSenderTest, RequestSendSuccess) { const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)}; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send(false, "test", urls, - base::Bind(&RequestSenderTest::RequestSenderComplete, - base::Unretained(this))); + request_sender_->Send( + false, "test", urls, + base::BindOnce(&RequestSenderTest::RequestSenderComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) @@ -176,9 +178,10 @@ TEST_F(RequestSenderTest, RequestSendSuccessWithFallback) { const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)}; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send(false, "test", urls, - base::Bind(&RequestSenderTest::RequestSenderComplete, - base::Unretained(this))); + request_sender_->Send( + false, "test", urls, + base::BindOnce(&RequestSenderTest::RequestSenderComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) @@ -204,9 +207,10 @@ TEST_F(RequestSenderTest, RequestSendFailed) { const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)}; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send(false, "test", urls, - base::Bind(&RequestSenderTest::RequestSenderComplete, - base::Unretained(this))); + request_sender_->Send( + false, "test", urls, + base::BindOnce(&RequestSenderTest::RequestSenderComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) @@ -227,9 +231,10 @@ TEST_F(RequestSenderTest, RequestSendFailed) { TEST_F(RequestSenderTest, RequestSendFailedNoUrls) { std::vector<GURL> urls; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send(false, "test", urls, - base::Bind(&RequestSenderTest::RequestSenderComplete, - base::Unretained(this))); + request_sender_->Send( + false, "test", urls, + base::BindOnce(&RequestSenderTest::RequestSenderComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(-1, error_); @@ -242,9 +247,10 @@ TEST_F(RequestSenderTest, RequestSendCupError) { const std::vector<GURL> urls = {GURL(kUrl1)}; request_sender_ = base::MakeUnique<RequestSender>(config_); - request_sender_->Send(true, "test", urls, - base::Bind(&RequestSenderTest::RequestSenderComplete, - base::Unretained(this))); + request_sender_->Send( + true, "test", urls, + base::BindOnce(&RequestSenderTest::RequestSenderComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) diff --git a/chromium/components/update_client/task_send_uninstall_ping.cc b/chromium/components/update_client/task_send_uninstall_ping.cc index 70130ed3795..45a11644924 100644 --- a/chromium/components/update_client/task_send_uninstall_ping.cc +++ b/chromium/components/update_client/task_send_uninstall_ping.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include "components/update_client/task_send_uninstall_ping.h" +#include <utility> + #include "base/bind.h" #include "base/bind_helpers.h" #include "base/location.h" @@ -17,12 +19,12 @@ TaskSendUninstallPing::TaskSendUninstallPing(UpdateEngine* update_engine, const std::string& id, const base::Version& version, int reason, - const Callback& callback) + Callback callback) : update_engine_(update_engine), id_(id), version_(version), reason_(reason), - callback_(callback) {} + callback_(std::move(callback)) {} TaskSendUninstallPing::~TaskSendUninstallPing() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -38,7 +40,8 @@ void TaskSendUninstallPing::Run() { update_engine_->SendUninstallPing( id_, version_, reason_, - base::Bind(&TaskSendUninstallPing::TaskComplete, base::Unretained(this))); + base::BindOnce(&TaskSendUninstallPing::TaskComplete, + base::Unretained(this))); } void TaskSendUninstallPing::Cancel() { @@ -55,7 +58,7 @@ void TaskSendUninstallPing::TaskComplete(Error error) { DCHECK(thread_checker_.CalledOnValidThread()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback_, this, error)); + FROM_HERE, base::BindOnce(std::move(callback_), this, error)); } } // namespace update_client diff --git a/chromium/components/update_client/task_send_uninstall_ping.h b/chromium/components/update_client/task_send_uninstall_ping.h index 33dbc4941a8..5af65777129 100644 --- a/chromium/components/update_client/task_send_uninstall_ping.h +++ b/chromium/components/update_client/task_send_uninstall_ping.h @@ -26,7 +26,7 @@ enum class Error; // Defines a specialized task for sending the uninstall ping. class TaskSendUninstallPing : public Task { public: - using Callback = base::Callback<void(Task* task, Error error)>; + using Callback = base::OnceCallback<void(Task* task, Error error)>; // |update_engine| is injected here to handle the task. // |id| represents the CRX to send the ping for. @@ -36,7 +36,7 @@ class TaskSendUninstallPing : public Task { const std::string& id, const base::Version& version, int reason, - const Callback& callback); + Callback callback); ~TaskSendUninstallPing() override; void Run() override; @@ -56,7 +56,7 @@ class TaskSendUninstallPing : public Task { const std::string id_; const base::Version version_; int reason_; - const Callback callback_; + Callback callback_; DISALLOW_COPY_AND_ASSIGN(TaskSendUninstallPing); }; diff --git a/chromium/components/update_client/task_update.cc b/chromium/components/update_client/task_update.cc index b9fb47b1f1b..050173648c2 100644 --- a/chromium/components/update_client/task_update.cc +++ b/chromium/components/update_client/task_update.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include "components/update_client/task_update.h" +#include <utility> + #include "base/bind.h" #include "base/bind_helpers.h" #include "base/location.h" @@ -15,14 +17,13 @@ namespace update_client { TaskUpdate::TaskUpdate(UpdateEngine* update_engine, bool is_foreground, const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, - const Callback& callback) + UpdateClient::CrxDataCallback crx_data_callback, + Callback callback) : update_engine_(update_engine), is_foreground_(is_foreground), ids_(ids), - crx_data_callback_(crx_data_callback), - callback_(callback) { -} + crx_data_callback_(std::move(crx_data_callback)), + callback_(std::move(callback)) {} TaskUpdate::~TaskUpdate() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -37,8 +38,8 @@ void TaskUpdate::Run() { } update_engine_->Update( - is_foreground_, ids_, crx_data_callback_, - base::Bind(&TaskUpdate::TaskComplete, base::Unretained(this))); + is_foreground_, ids_, std::move(crx_data_callback_), + base::BindOnce(&TaskUpdate::TaskComplete, base::Unretained(this))); } void TaskUpdate::Cancel() { @@ -55,7 +56,7 @@ void TaskUpdate::TaskComplete(Error error) { DCHECK(thread_checker_.CalledOnValidThread()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback_, this, error)); + FROM_HERE, base::BindOnce(std::move(callback_), this, error)); } } // namespace update_client diff --git a/chromium/components/update_client/task_update.h b/chromium/components/update_client/task_update.h index 9d8235c3d47..d6862c2853f 100644 --- a/chromium/components/update_client/task_update.h +++ b/chromium/components/update_client/task_update.h @@ -22,7 +22,7 @@ enum class Error; // Defines a specialized task for updating a group of CRXs. class TaskUpdate : public Task { public: - using Callback = base::Callback<void(Task* task, Error error)>; + using Callback = base::OnceCallback<void(Task* task, Error error)>; // |update_engine| is injected here to handle the task. // |is_foreground| is true when the update task is initiated by the user, @@ -34,8 +34,8 @@ class TaskUpdate : public Task { TaskUpdate(UpdateEngine* update_engine, bool is_foreground, const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, - const Callback& callback); + UpdateClient::CrxDataCallback crx_data_callback, + Callback callback); ~TaskUpdate() override; void Run() override; @@ -55,8 +55,8 @@ class TaskUpdate : public Task { const bool is_foreground_; const std::vector<std::string> ids_; - const UpdateClient::CrxDataCallback crx_data_callback_; - const Callback callback_; + UpdateClient::CrxDataCallback crx_data_callback_; + Callback callback_; DISALLOW_COPY_AND_ASSIGN(TaskUpdate); }; diff --git a/chromium/components/update_client/test_configurator.cc b/chromium/components/update_client/test_configurator.cc index 4dd09d0d53e..ba1dc7e9b6f 100644 --- a/chromium/components/update_client/test_configurator.cc +++ b/chromium/components/update_client/test_configurator.cc @@ -8,10 +8,13 @@ #include "base/threading/thread_task_runner_handle.h" #include "base/version.h" +#include "components/patch_service/file_patcher_impl.h" +#include "components/patch_service/patch_service.h" +#include "components/patch_service/public/interfaces/file_patcher.mojom.h" #include "components/prefs/pref_service.h" #include "components/update_client/activity_data_service.h" -#include "components/update_client/out_of_process_patcher.h" #include "net/url_request/url_request_test_util.h" +#include "services/service_manager/public/cpp/connector.h" #include "url/gurl.h" namespace update_client { @@ -33,6 +36,8 @@ TestConfigurator::TestConfigurator() ondemand_time_(0), enabled_cup_signing_(false), enabled_component_updates_(true), + connector_factory_(std::make_unique<patch::PatchService>()), + connector_(connector_factory_.CreateConnector()), context_(base::MakeRefCounted<net::TestURLRequestContextGetter>( base::ThreadTaskRunnerHandle::Get())) {} @@ -106,9 +111,9 @@ net::URLRequestContextGetter* TestConfigurator::RequestContext() const { return context_.get(); } -scoped_refptr<OutOfProcessPatcher> TestConfigurator::CreateOutOfProcessPatcher() - const { - return NULL; +std::unique_ptr<service_manager::Connector> +TestConfigurator::CreateServiceManagerConnector() const { + return connector_->Clone(); } bool TestConfigurator::EnabledDeltas() const { diff --git a/chromium/components/update_client/test_configurator.h b/chromium/components/update_client/test_configurator.h index 83686f445a2..0b3b4bc0522 100644 --- a/chromium/components/update_client/test_configurator.h +++ b/chromium/components/update_client/test_configurator.h @@ -14,6 +14,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "components/update_client/configurator.h" +#include "services/service_manager/public/cpp/test/test_connector_factory.h" #include "url/gurl.h" class PrefService; @@ -23,6 +24,10 @@ class TestURLRequestContextGetter; class URLRequestContextGetter; } // namespace net +namespace service_manager { +class Connector; +} + namespace update_client { class ActivityDataService; @@ -37,6 +42,11 @@ const uint8_t jebg_hash[] = {0x94, 0x16, 0x0b, 0x6d, 0x41, 0x75, 0xe9, 0xec, 0x8e, 0xd5, 0xfa, 0x54, 0xb0, 0xd2, 0xdd, 0xa5, 0x6e, 0x05, 0x6b, 0xe8, 0x73, 0x47, 0xf6, 0xc4, 0x11, 0x9f, 0xbc, 0xb3, 0x09, 0xb3, 0x5b, 0x40}; +// component 1 public key (base64 encoded): +const std::string jebg_public_key = + "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC68bW8i/RzSaeXOcNLuBw0SP9+1bdo5ysLqH" + "qfLqZs6XyJWEyL0U6f1axPR6LwViku21kgdc6PI524eb8Cr+a/iXGgZ8SdvZTcfQ/g/ukwlblF" + "mtqYfDoVpz03U8rDQ9b6DxeJBF4r48TNlFORggrAiNR26qbf1i178Au12AzWtwIDAQAB"; // component 2 has extension id "abagagagagagagagagagagagagagagag", and // the RSA public key the following hash: const uint8_t abag_hash[] = {0x01, 0x06, 0x06, 0x06, 0x06, 0x06, 0x06, 0x06, @@ -76,7 +86,8 @@ class TestConfigurator : public Configurator { std::string ExtraRequestParams() const override; std::string GetDownloadPreference() const override; net::URLRequestContextGetter* RequestContext() const override; - scoped_refptr<OutOfProcessPatcher> CreateOutOfProcessPatcher() const override; + std::unique_ptr<service_manager::Connector> CreateServiceManagerConnector() + const override; bool EnabledDeltas() const override; bool EnabledComponentUpdates() const override; bool EnabledBackgroundDownloader() const override; @@ -99,6 +110,8 @@ class TestConfigurator : public Configurator { friend class base::RefCountedThreadSafe<TestConfigurator>; ~TestConfigurator() override; + class TestPatchService; + std::string brand_; int initial_time_; int ondemand_time_; @@ -108,6 +121,8 @@ class TestConfigurator : public Configurator { GURL update_check_url_; GURL ping_url_; + service_manager::TestConnectorFactory connector_factory_; + std::unique_ptr<service_manager::Connector> connector_; scoped_refptr<net::TestURLRequestContextGetter> context_; DISALLOW_COPY_AND_ASSIGN(TestConfigurator); diff --git a/chromium/components/update_client/test_installer.cc b/chromium/components/update_client/test_installer.cc index 76a7ce70644..22eef68c64e 100644 --- a/chromium/components/update_client/test_installer.cc +++ b/chromium/components/update_client/test_installer.cc @@ -12,6 +12,7 @@ #include "base/task_scheduler/task_traits.h" #include "base/values.h" #include "components/update_client/update_client_errors.h" +#include "components/update_client/utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace update_client { @@ -31,19 +32,19 @@ void TestInstaller::OnUpdateError(int error) { error_ = error; } -void TestInstaller::Install(std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) { +void TestInstaller::Install(const base::FilePath& unpack_path, + const std::string& /*public_key*/, + Callback callback) { ++install_count_; unpack_path_ = unpack_path; - InstallComplete(callback, Result(InstallError::NONE)); + InstallComplete(std::move(callback), Result(InstallError::NONE)); } -void TestInstaller::InstallComplete(const Callback& callback, +void TestInstaller::InstallComplete(Callback callback, const Result& result) const { base::PostTaskWithTraits(FROM_HERE, {base::MayBlock()}, - base::BindOnce(callback, result)); + base::BindOnce(std::move(callback), result)); } bool TestInstaller::GetInstalledFile(const std::string& file, @@ -76,31 +77,31 @@ VersionedTestInstaller::~VersionedTestInstaller() { base::DeleteFile(install_directory_, true); } -void VersionedTestInstaller::Install( - std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) { +void VersionedTestInstaller::Install(const base::FilePath& unpack_path, + const std::string& public_key, + Callback callback) { + const auto manifest = update_client::ReadManifest(unpack_path); std::string version_string; manifest->GetStringASCII("version", &version_string); - base::Version version(version_string.c_str()); + const base::Version version(version_string.c_str()); - base::FilePath path; - path = install_directory_.AppendASCII(version.GetString()); + const base::FilePath path = + install_directory_.AppendASCII(version.GetString()); base::CreateDirectory(path.DirName()); if (!base::Move(unpack_path, path)) { - InstallComplete(callback, Result(InstallError::GENERIC_ERROR)); + InstallComplete(std::move(callback), Result(InstallError::GENERIC_ERROR)); return; } current_version_ = version; ++install_count_; - InstallComplete(callback, Result(InstallError::NONE)); + InstallComplete(std::move(callback), Result(InstallError::NONE)); } bool VersionedTestInstaller::GetInstalledFile(const std::string& file, base::FilePath* installed_file) { - base::FilePath path; - path = install_directory_.AppendASCII(current_version_.GetString()); + const base::FilePath path = + install_directory_.AppendASCII(current_version_.GetString()); *installed_file = path.Append(base::FilePath::FromUTF8Unsafe(file)); return true; } diff --git a/chromium/components/update_client/test_installer.h b/chromium/components/update_client/test_installer.h index 151cf7b7da4..a12baf9bb7c 100644 --- a/chromium/components/update_client/test_installer.h +++ b/chromium/components/update_client/test_installer.h @@ -7,14 +7,11 @@ #include <memory> #include <string> +#include <utility> #include "base/files/file_path.h" #include "components/update_client/update_client.h" -namespace base { -class DictionaryValue; -} - namespace update_client { // TODO(sorin): consider reducing the number of the installer mocks. @@ -26,9 +23,9 @@ class TestInstaller : public CrxInstaller { void OnUpdateError(int error) override; - void Install(std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) override; + void Install(const base::FilePath& unpack_path, + const std::string& public_key, + Callback callback) override; bool GetInstalledFile(const std::string& file, base::FilePath* installed_file) override; @@ -42,7 +39,7 @@ class TestInstaller : public CrxInstaller { protected: ~TestInstaller() override; - void InstallComplete(const Callback& callback, const Result& result) const; + void InstallComplete(Callback callback, const Result& result) const; int error_; int install_count_; @@ -73,9 +70,9 @@ class VersionedTestInstaller : public TestInstaller { public: VersionedTestInstaller(); - void Install(std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) override; + void Install(const base::FilePath& unpack_path, + const std::string& public_key, + Callback callback) override; bool GetInstalledFile(const std::string& file, base::FilePath* installed_file) override; diff --git a/chromium/components/update_client/update_checker.cc b/chromium/components/update_client/update_checker.cc index 82c2013817e..fbac4bbf1ea 100644 --- a/chromium/components/update_client/update_checker.cc +++ b/chromium/components/update_client/update_checker.cc @@ -8,6 +8,7 @@ #include <memory> #include <string> +#include <utility> #include <vector> #include "base/bind.h" @@ -54,12 +55,11 @@ class UpdateCheckerImpl : public UpdateChecker { ~UpdateCheckerImpl() override; // Overrides for UpdateChecker. - void CheckForUpdates( - const std::vector<std::string>& ids_checked, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override; + void CheckForUpdates(const std::vector<std::string>& ids_checked, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) override; private: void ReadUpdaterStateAttributes(); @@ -102,11 +102,11 @@ void UpdateCheckerImpl::CheckForUpdates( const IdToComponentPtrMap& components, const std::string& additional_attributes, bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) { + UpdateCheckCallback update_check_callback) { DCHECK(thread_checker_.CalledOnValidThread()); ids_checked_ = ids_checked; - update_check_callback_ = update_check_callback; + update_check_callback_ = std::move(update_check_callback); base::PostTaskWithTraitsAndReply( FROM_HERE, kTaskTraits, @@ -140,8 +140,8 @@ void UpdateCheckerImpl::CheckForUpdatesHelper( additional_attributes, enabled_component_updates, updater_state_attributes_), urls, - base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete, - base::Unretained(this), base::ConstRef(components))); + base::BindOnce(&UpdateCheckerImpl::OnRequestSenderComplete, + base::Unretained(this), base::ConstRef(components))); } void UpdateCheckerImpl::OnRequestSenderComplete( @@ -199,7 +199,8 @@ void UpdateCheckerImpl::UpdateCheckSucceeded( } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback_, 0, retry_after_sec)); + FROM_HERE, + base::BindOnce(std::move(update_check_callback_), 0, retry_after_sec)); } void UpdateCheckerImpl::UpdateCheckFailed(const IdToComponentPtrMap& components, @@ -214,8 +215,8 @@ void UpdateCheckerImpl::UpdateCheckFailed(const IdToComponentPtrMap& components, } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::BindOnce(update_check_callback_, error, retry_after_sec)); + FROM_HERE, base::BindOnce(std::move(update_check_callback_), error, + retry_after_sec)); } } // namespace diff --git a/chromium/components/update_client/update_checker.h b/chromium/components/update_client/update_checker.h index c38bf61e790..0f2cb3edfab 100644 --- a/chromium/components/update_client/update_checker.h +++ b/chromium/components/update_client/update_checker.h @@ -23,7 +23,7 @@ class PersistedData; class UpdateChecker { public: using UpdateCheckCallback = - base::Callback<void(int error, int retry_after_sec)>; + base::OnceCallback<void(int error, int retry_after_sec)>; using Factory = std::unique_ptr<UpdateChecker> (*)( const scoped_refptr<Configurator>& config, @@ -37,12 +37,11 @@ class UpdateChecker { // XML attribute string. // On completion, the state of |components| is mutated as required by the // server response received. - virtual void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) = 0; + virtual void CheckForUpdates(const std::vector<std::string>& ids_to_check, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) = 0; static std::unique_ptr<UpdateChecker> Create( const scoped_refptr<Configurator>& config, diff --git a/chromium/components/update_client/update_checker_unittest.cc b/chromium/components/update_client/update_checker_unittest.cc index f384f0e080c..d1dff9caf8d 100644 --- a/chromium/components/update_client/update_checker_unittest.cc +++ b/chromium/components/update_client/update_checker_unittest.cc @@ -141,7 +141,7 @@ class UpdateCheckerTest : public testing::Test { std::unique_ptr<UpdateContext> MakeFakeUpdateContext() const; base::test::ScopedTaskEnvironment scoped_task_environment_; - base::Closure quit_closure_; + base::OnceClosure quit_closure_; DISALLOW_COPY_AND_ASSIGN(UpdateCheckerTest); }; @@ -199,7 +199,7 @@ void UpdateCheckerTest::RunThreads() { void UpdateCheckerTest::Quit() { if (!quit_closure_.is_null()) - quit_closure_.Run(); + std::move(quit_closure_).Run(); } void UpdateCheckerTest::UpdateCheckComplete(int error, int retry_after_sec) { @@ -220,7 +220,7 @@ 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 = NULL; + crx_component.installer = nullptr; crx_component.version = base::Version("0.9"); crx_component.fingerprint = "fp1"; @@ -246,8 +246,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_->GetHitCount()) @@ -313,8 +313,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckInvalidAp) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); @@ -339,8 +339,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccessNoBrand) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); @@ -367,8 +367,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckError) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_->GetHitCount()) @@ -394,8 +394,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); @@ -421,8 +421,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckCupError) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); @@ -461,8 +461,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckRequiresEncryptionError) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(-1, error_); @@ -487,16 +487,16 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastRollCall) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(2, post_interceptor_->GetHitCount()) @@ -527,8 +527,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); // The active bit should be reset. @@ -539,8 +539,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); // The active bit should be reset. @@ -550,8 +550,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckLastActive) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "extra=\"params\"", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_FALSE(metadata_->GetActiveBit(kUpdateItemId)); @@ -582,8 +582,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[0].find("enabled=\"1\"")); @@ -594,8 +594,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[1].find("enabled=\"1\"")); @@ -605,8 +605,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { crx_component.disabled_reasons = std::vector<int>({0}); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[2].find("enabled=\"0\"")); @@ -617,8 +617,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[3].find("enabled=\"0\"")); @@ -629,8 +629,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[4].find("enabled=\"0\"")); @@ -645,8 +645,8 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[5].find("enabled=\"0\"")); @@ -682,8 +682,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[0].find( std::string("<app appid=\"") + kUpdateItemId + @@ -699,8 +699,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", false, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[1].find( std::string("<app appid=\"") + kUpdateItemId + @@ -716,8 +716,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[2].find( std::string("<app appid=\"") + kUpdateItemId + @@ -733,8 +733,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) { update_checker_ = UpdateChecker::Create(config_, metadata_.get()); update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_NE(string::npos, post_interceptor_->GetRequests()[3].find( std::string("<app appid=\"") + kUpdateItemId + @@ -756,8 +756,8 @@ TEST_F(UpdateCheckerTest, NoUpdateActionRun) { update_checker_->CheckForUpdates( std::vector<std::string>{kUpdateItemId}, components, "", true, - base::Bind(&UpdateCheckerTest::UpdateCheckComplete, - base::Unretained(this))); + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); RunThreads(); EXPECT_EQ(1, post_interceptor_->GetHitCount()) diff --git a/chromium/components/update_client/update_client.cc b/chromium/components/update_client/update_client.cc index 3ec3e3cebc4..e50b0c3388a 100644 --- a/chromium/components/update_client/update_client.cc +++ b/chromium/components/update_client/update_client.cc @@ -87,12 +87,12 @@ UpdateClientImpl::~UpdateClientImpl() { } void UpdateClientImpl::Install(const std::string& id, - const CrxDataCallback& crx_data_callback, - const Callback& callback) { + CrxDataCallback crx_data_callback, + Callback callback) { DCHECK(thread_checker_.CalledOnValidThread()); if (IsUpdating(id)) { - callback.Run(Error::UPDATE_IN_PROGRESS); + std::move(callback).Run(Error::UPDATE_IN_PROGRESS); return; } @@ -100,22 +100,22 @@ void UpdateClientImpl::Install(const std::string& id, // Partially applies |callback| to OnTaskComplete, so this argument is // available when the task completes, along with the task itself. - std::unique_ptr<TaskUpdate> task = base::MakeUnique<TaskUpdate>( - update_engine_.get(), true, ids, crx_data_callback, - base::Bind(&UpdateClientImpl::OnTaskComplete, this, callback)); - // Install tasks are run concurrently and never queued up. - RunTask(std::move(task)); + RunTask(std::make_unique<TaskUpdate>( + update_engine_.get(), true, ids, std::move(crx_data_callback), + base::BindOnce(&UpdateClientImpl::OnTaskComplete, this, + std::move(callback)))); } void UpdateClientImpl::Update(const std::vector<std::string>& ids, - const CrxDataCallback& crx_data_callback, - const Callback& callback) { + CrxDataCallback crx_data_callback, + Callback callback) { DCHECK(thread_checker_.CalledOnValidThread()); - std::unique_ptr<TaskUpdate> task = base::MakeUnique<TaskUpdate>( - update_engine_.get(), false, ids, crx_data_callback, - base::Bind(&UpdateClientImpl::OnTaskComplete, this, callback)); + auto task = std::make_unique<TaskUpdate>( + update_engine_.get(), false, ids, std::move(crx_data_callback), + base::BindOnce(&UpdateClientImpl::OnTaskComplete, this, + std::move(callback))); // If no other tasks are running at the moment, run this update task. // Otherwise, queue the task up. @@ -133,14 +133,14 @@ void UpdateClientImpl::RunTask(std::unique_ptr<Task> task) { tasks_.insert(task.release()); } -void UpdateClientImpl::OnTaskComplete(const Callback& callback, +void UpdateClientImpl::OnTaskComplete(Callback callback, Task* task, Error error) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(task); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback, error)); + FROM_HERE, base::BindOnce(std::move(callback), error)); // Remove the task from the set of the running tasks. Only tasks handled by // the update engine can be in this data structure. @@ -231,15 +231,13 @@ void UpdateClientImpl::Stop() { void UpdateClientImpl::SendUninstallPing(const std::string& id, const base::Version& version, int reason, - const Callback& callback) { + Callback callback) { DCHECK(thread_checker_.CalledOnValidThread()); - std::unique_ptr<TaskSendUninstallPing> task = - base::MakeUnique<TaskSendUninstallPing>( - update_engine_.get(), id, version, reason, - base::Bind(&UpdateClientImpl::OnTaskComplete, base::Unretained(this), - callback)); - RunTask(std::move(task)); + RunTask(std::make_unique<TaskSendUninstallPing>( + update_engine_.get(), id, version, reason, + base::BindOnce(&UpdateClientImpl::OnTaskComplete, base::Unretained(this), + std::move(callback)))); } scoped_refptr<UpdateClient> UpdateClientFactory( diff --git a/chromium/components/update_client/update_client.h b/chromium/components/update_client/update_client.h index 3d6f84d2793..10e9cd18e5d 100644 --- a/chromium/components/update_client/update_client.h +++ b/chromium/components/update_client/update_client.h @@ -55,7 +55,7 @@ // update_client->AddObserver(&observer); // std::vector<std::string> ids; // ids.push_back(...)); -// update_client->Update(ids, base::Bind(...), base::Bind(...)); +// update_client->Update(ids, base::BindOnce(...), base::BindOnce(...)); // // UpdateClient::Update takes two callbacks as parameters. First callback // allows the client of this code to provide an instance of CrxComponent @@ -133,7 +133,6 @@ class PrefRegistrySimple; namespace base { -class DictionaryValue; class FilePath; } @@ -173,7 +172,7 @@ class CrxInstaller : public base::RefCountedThreadSafe<CrxInstaller> { int extended_error = 0; }; - using Callback = base::Callback<void(const Result& result)>; + using Callback = base::OnceCallback<void(const Result& result)>; // Called on the main thread when there was a problem unpacking or // verifying the CRX. |error| is a non-zero value which is only meaningful @@ -181,14 +180,14 @@ class CrxInstaller : public base::RefCountedThreadSafe<CrxInstaller> { virtual void OnUpdateError(int error) = 0; // Called by the update service when a CRX has been unpacked - // and it is ready to be installed. |manifest| contains the CRX manifest - // as a json dictionary.|unpack_path| contains the temporary directory - // with all the unpacked CRX files. The caller must invoke the |callback| - // when the install flow has completed. + // and it is ready to be installed. |unpack_path| contains the + // temporary directory with all the unpacked CRX files. |pubkey| contains the + // public key of the CRX in the PEM format, without the header and the footer. + // The caller must invoke the |callback| when the install flow has completed. // This method may be called from a thread other than the main thread. - virtual void Install(std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) = 0; + virtual void Install(const base::FilePath& unpack_path, + const std::string& public_key, + Callback callback) = 0; // Sets |installed_file| to the full path to the installed |file|. |file| is // the filename of the file in this CRX. Returns false if this is @@ -257,7 +256,7 @@ struct CrxComponent { }; // Called when a non-blocking call of UpdateClient completes. -using Callback = base::Callback<void(Error error)>; +using Callback = base::OnceCallback<void(Error error)>; // All methods are safe to call only from the browser's main thread. Once an // instance of this class is created, the reference to it must be released @@ -266,8 +265,8 @@ using Callback = base::Callback<void(Error error)>; class UpdateClient : public base::RefCounted<UpdateClient> { public: using CrxDataCallback = - base::Callback<void(const std::vector<std::string>& ids, - std::vector<CrxComponent>* components)>; + base::OnceCallback<void(const std::vector<std::string>& ids, + std::vector<CrxComponent>* components)>; // Defines an interface to observe the UpdateClient. It provides // notifications when state changes occur for the service itself or for the @@ -331,8 +330,8 @@ class UpdateClient : public base::RefCounted<UpdateClient> { // scenarios, which are triggered by user actions. Installs are never // queued up. virtual void Install(const std::string& id, - const CrxDataCallback& crx_data_callback, - const Callback& callback) = 0; + CrxDataCallback crx_data_callback, + Callback callback) = 0; // Updates the specified CRXs. Calls back on |crx_data_callback| before the // update is attempted to give the caller the opportunity to provide the @@ -342,8 +341,8 @@ class UpdateClient : public base::RefCounted<UpdateClient> { // of each call is serialized. In addition, updates are always queued up when // installs are running. virtual void Update(const std::vector<std::string>& ids, - const CrxDataCallback& crx_data_callback, - const Callback& callback) = 0; + CrxDataCallback crx_data_callback, + Callback callback) = 0; // Sends an uninstall ping for the CRX identified by |id| and |version|. The // |reason| parameter is defined by the caller. The current implementation of @@ -353,7 +352,7 @@ class UpdateClient : public base::RefCounted<UpdateClient> { virtual void SendUninstallPing(const std::string& id, const base::Version& version, int reason, - const Callback& callback) = 0; + Callback callback) = 0; // Returns status details about a CRX update. The function returns true in // case of success and false in case of errors, such as |id| was diff --git a/chromium/components/update_client/update_client_internal.h b/chromium/components/update_client/update_client_internal.h index 04efc56d220..6684b7eebca 100644 --- a/chromium/components/update_client/update_client_internal.h +++ b/chromium/components/update_client/update_client_internal.h @@ -38,11 +38,11 @@ class UpdateClientImpl : public UpdateClient { void AddObserver(Observer* observer) override; void RemoveObserver(Observer* observer) override; void Install(const std::string& id, - const CrxDataCallback& crx_data_callback, - const Callback& callback) override; + CrxDataCallback crx_data_callback, + Callback callback) override; void Update(const std::vector<std::string>& ids, - const CrxDataCallback& crx_data_callback, - const Callback& callback) override; + CrxDataCallback crx_data_callback, + Callback callback) override; bool GetCrxUpdateState(const std::string& id, CrxUpdateItem* update_item) const override; bool IsUpdating(const std::string& id) const override; @@ -50,13 +50,13 @@ class UpdateClientImpl : public UpdateClient { void SendUninstallPing(const std::string& id, const base::Version& version, int reason, - const Callback& callback) override; + Callback callback) override; private: ~UpdateClientImpl() override; void RunTask(std::unique_ptr<Task> task); - void OnTaskComplete(const Callback& callback, Task* task, Error error); + void OnTaskComplete(Callback callback, Task* task, Error error); void NotifyObservers(Observer::Events event, const std::string& id); diff --git a/chromium/components/update_client/update_client_unittest.cc b/chromium/components/update_client/update_client_unittest.cc index 65b6f717c72..05434bc2cfc 100644 --- a/chromium/components/update_client/update_client_unittest.cc +++ b/chromium/components/update_client/update_client_unittest.cc @@ -166,14 +166,13 @@ class UpdateClientTest : public testing::Test { scoped_refptr<update_client::TestConfigurator> config() { return config_; } update_client::PersistedData* metadata() { return metadata_.get(); } - base::Closure quit_closure() { return quit_closure_; } + base::OnceClosure quit_closure() { return runloop_.QuitClosure(); } private: static constexpr int kNumWorkerThreads_ = 2; base::test::ScopedTaskEnvironment scoped_task_environment_; base::RunLoop runloop_; - const base::Closure quit_closure_ = runloop_.QuitClosure(); scoped_refptr<update_client::TestConfigurator> config_ = base::MakeRefCounted<TestConfigurator>(); @@ -227,9 +226,9 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -241,12 +240,11 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(1u, ids_to_check.size()); const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; @@ -263,7 +261,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -304,8 +302,8 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -338,9 +336,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -352,12 +350,11 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -423,7 +420,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -516,8 +513,8 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf", "abagagagagagagagagagagagagagagag"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -549,9 +546,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -563,12 +560,11 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -656,7 +652,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -715,9 +711,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { base::Unretained(this), result)); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::Bind(&FakeCrxDownloader::OnDownloadComplete, - base::Unretained(this), true, result, download_metrics)); + FROM_HERE, base::BindOnce(&FakeCrxDownloader::OnDownloadComplete, + base::Unretained(this), true, result, + download_metrics)); } }; @@ -783,8 +779,8 @@ TEST_F(UpdateClientTest, TwoCrxUpdate) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf", "ihfokbkgjpifnbbojhneepfflplebdkc"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -818,9 +814,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -832,12 +828,11 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -922,7 +917,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -1045,8 +1040,8 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { "ihfokbkgjpifnbbojhneepfflplebdkc"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -1085,9 +1080,9 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -1099,12 +1094,11 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { static int num_call = 0; ++num_call; @@ -1205,7 +1199,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -1330,17 +1324,17 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { const std::vector<std::string> ids = {"ihfokbkgjpifnbbojhneepfflplebdkc"}; { base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } { base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } @@ -1354,31 +1348,24 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { TEST_F(UpdateClientTest, OneCrxInstallError) { class MockInstaller : public CrxInstaller { public: - // gMock does not support mocking functions with parameters which have - // move semantics. This function is a shim to work around it. - void Install(std::unique_ptr<base::DictionaryValue> manifest, - const base::FilePath& unpack_path, - const Callback& callback) { - return Install_(manifest, unpack_path, callback); - } - MOCK_METHOD1(OnUpdateError, void(int error)); - MOCK_METHOD3(Install_, - void(const std::unique_ptr<base::DictionaryValue>& manifest, - const base::FilePath& unpack_path, + MOCK_METHOD2(DoInstall, + void(const base::FilePath& unpack_path, const Callback& callback)); MOCK_METHOD2(GetInstalledFile, bool(const std::string& file, base::FilePath* installed_file)); MOCK_METHOD0(Uninstall, bool()); - void OnInstall(const std::unique_ptr<base::DictionaryValue>& manifest, - const base::FilePath& unpack_path, - const Callback& callback) { + void Install(const base::FilePath& unpack_path, + const std::string& public_key, + Callback callback) { + DoInstall(unpack_path, std::move(callback)); + unpack_path_ = unpack_path; EXPECT_TRUE(base::DirectoryExists(unpack_path_)); base::PostTaskWithTraits( FROM_HERE, {base::MayBlock()}, - base::BindOnce(callback, + base::BindOnce(std::move(callback), CrxInstaller::Result(InstallError::GENERIC_ERROR))); } @@ -1404,8 +1391,7 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { base::MakeRefCounted<MockInstaller>(); EXPECT_CALL(*installer, OnUpdateError(_)).Times(0); - EXPECT_CALL(*installer, Install_(_, _, _)) - .WillOnce(Invoke(installer.get(), &MockInstaller::OnInstall)); + EXPECT_CALL(*installer, DoInstall(_, _)).Times(1); EXPECT_CALL(*installer, GetInstalledFile(_, _)).Times(0); EXPECT_CALL(*installer, Uninstall()).Times(0); @@ -1420,9 +1406,9 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -1434,12 +1420,11 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -1482,7 +1467,7 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -1517,8 +1502,8 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { result.total_bytes = 1843; base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(&FakeCrxDownloader::OnDownloadProgress, - base::Unretained(this), result)); + FROM_HERE, base::BindOnce(&FakeCrxDownloader::OnDownloadProgress, + base::Unretained(this), result)); base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&FakeCrxDownloader::OnDownloadComplete, @@ -1567,8 +1552,8 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -1607,9 +1592,9 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -1621,12 +1606,11 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { static int num_call = 0; ++num_call; @@ -1729,7 +1713,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -1868,17 +1852,17 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { { base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } { base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } @@ -1904,14 +1888,14 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { static int num_call = 0; ++num_call; EXPECT_EQ(Error::NONE, error); if (num_call == 2) - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -1923,12 +1907,11 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(1u, ids_to_check.size()); const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; @@ -1945,7 +1928,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -1992,11 +1975,11 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -2021,9 +2004,9 @@ TEST_F(UpdateClientTest, OneCrxInstall) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -2035,12 +2018,11 @@ TEST_F(UpdateClientTest, OneCrxInstall) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -2089,7 +2071,7 @@ TEST_F(UpdateClientTest, OneCrxInstall) { EXPECT_TRUE(component->on_demand()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -2176,8 +2158,8 @@ TEST_F(UpdateClientTest, OneCrxInstall) { update_client->Install( std::string("jebgalgnebhfojomionfpkfelancnnkf"), - base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -2202,7 +2184,7 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { static int num_call = 0; ++num_call; @@ -2214,7 +2196,7 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { } if (num_call == 2) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } } }; @@ -2227,12 +2209,11 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(1u, ids_to_check.size()); const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; @@ -2250,7 +2231,7 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { EXPECT_TRUE(component->on_demand()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -2294,13 +2275,13 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { update_client->Install( std::string("jebgalgnebhfojomionfpkfelancnnkf"), - base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); update_client->Install( std::string("jebgalgnebhfojomionfpkfelancnnkf"), - base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -2318,9 +2299,9 @@ TEST_F(UpdateClientTest, EmptyIdList) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { DCHECK_EQ(Error::INVALID_ARGUMENT, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -2332,12 +2313,11 @@ TEST_F(UpdateClientTest, EmptyIdList) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override {} + void CheckForUpdates(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 {} }; class FakeCrxDownloader : public CrxDownloader { @@ -2361,16 +2341,16 @@ TEST_F(UpdateClientTest, EmptyIdList) { const std::vector<std::string> empty_id_list; update_client->Update( - empty_id_list, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + empty_id_list, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); } TEST_F(UpdateClientTest, SendUninstallPing) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { - quit_closure.Run(); + static void Callback(base::OnceClosure quit_closure, Error error) { + std::move(quit_closure).Run(); } }; @@ -2382,12 +2362,11 @@ TEST_F(UpdateClientTest, SendUninstallPing) { return nullptr; } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override {} + void CheckForUpdates(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 {} }; class FakeCrxDownloader : public CrxDownloader { @@ -2426,7 +2405,7 @@ TEST_F(UpdateClientTest, SendUninstallPing) { update_client->SendUninstallPing( "jebgalgnebhfojomionfpkfelancnnkf", base::Version("1.0"), 10, - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); } @@ -2447,7 +2426,7 @@ TEST_F(UpdateClientTest, RetryAfter) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { static int num_call = 0; ++num_call; @@ -2469,7 +2448,7 @@ TEST_F(UpdateClientTest, RetryAfter) { EXPECT_EQ(Error::NONE, error); } - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -2481,12 +2460,11 @@ TEST_F(UpdateClientTest, RetryAfter) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { static int num_call = 0; ++num_call; @@ -2511,7 +2489,8 @@ TEST_F(UpdateClientTest, RetryAfter) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, retry_after_sec)); + FROM_HERE, + base::BindOnce(std::move(update_check_callback), 0, retry_after_sec)); } }; @@ -2570,9 +2549,9 @@ TEST_F(UpdateClientTest, RetryAfter) { // The engine handles this Update call but responds with a valid // |retry_after_sec|, which causes subsequent calls to fail. base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } @@ -2580,9 +2559,9 @@ TEST_F(UpdateClientTest, RetryAfter) { // This call will result in a completion callback invoked with // Error::ERROR_UPDATE_RETRY_LATER. base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } @@ -2590,19 +2569,19 @@ TEST_F(UpdateClientTest, RetryAfter) { // The Install call is handled, and the throttling is reset due to // the value of |retry_after_sec| in the completion callback. base::RunLoop runloop; - update_client->Install( - std::string("jebgalgnebhfojomionfpkfelancnnkf"), - base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Install(std::string("jebgalgnebhfojomionfpkfelancnnkf"), + base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } { // This call succeeds. base::RunLoop runloop; - update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure())); + update_client->Update(ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, + runloop.QuitClosure())); runloop.Run(); } @@ -2640,9 +2619,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -2654,12 +2633,11 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -2749,7 +2727,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -2862,8 +2840,8 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf", "ihfokbkgjpifnbbojhneepfflplebdkc"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -2887,9 +2865,9 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { class CompletionCallbackFake { public: - static void Callback(const base::Closure& quit_closure, Error error) { + static void Callback(base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::UPDATE_CHECK_ERROR, error); - quit_closure.Run(); + std::move(quit_closure).Run(); } }; @@ -2901,12 +2879,11 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(1u, ids_to_check.size()); const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; @@ -2914,7 +2891,7 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { EXPECT_EQ(1u, components.count(id)); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, -1, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), -1, 0)); } }; @@ -2962,8 +2939,8 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf"}; update_client->Update( - ids, base::Bind(&DataCallbackFake::Callback), - base::Bind(&CompletionCallbackFake::Callback, quit_closure())); + ids, base::BindOnce(&DataCallbackFake::Callback), + base::BindOnce(&CompletionCallbackFake::Callback, quit_closure())); RunThreads(); @@ -2982,12 +2959,11 @@ TEST_F(UpdateClientTest, ActionRun_Install) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -3036,7 +3012,7 @@ TEST_F(UpdateClientTest, ActionRun_Install) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -3111,8 +3087,8 @@ TEST_F(UpdateClientTest, ActionRun_Install) { // The action is a program which returns 1877345072 as a hardcoded value. update_client->Install( std::string("gjpmebpgbhcamgdgjcmnjfhggjpgcimm"), - base::Bind([](const std::vector<std::string>& ids, - std::vector<CrxComponent>* components) { + 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)); @@ -3120,10 +3096,10 @@ TEST_F(UpdateClientTest, ActionRun_Install) { crx.installer = base::MakeRefCounted<VersionedTestInstaller>(); components->push_back(crx); }), - base::Bind( - [](const base::Closure& quit_closure, Error error) { + base::BindOnce( + [](base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); }, quit_closure())); @@ -3141,12 +3117,11 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { return base::MakeUnique<FakeUpdateChecker>(); } - void CheckForUpdates( - const std::vector<std::string>& ids_to_check, - const IdToComponentPtrMap& components, - const std::string& additional_attributes, - bool enabled_component_updates, - const UpdateCheckCallback& update_check_callback) override { + void CheckForUpdates(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 { /* Fake the following response: @@ -3176,7 +3151,7 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { component->SetParseResult(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(update_check_callback, 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); } }; @@ -3213,19 +3188,19 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { base::FilePath unpack_path; { base::RunLoop runloop; - base::Closure quit_closure = runloop.QuitClosure(); + base::OnceClosure quit_closure = runloop.QuitClosure(); scoped_refptr<ComponentUnpacker> component_unpacker = new ComponentUnpacker( std::vector<uint8_t>(std::begin(gjpm_hash), std::end(gjpm_hash)), TestFilePath("runaction_test_win.crx3"), nullptr, nullptr); - component_unpacker->Unpack(base::Bind( - [](base::FilePath* unpack_path, const base::Closure& quit_closure, + component_unpacker->Unpack(base::BindOnce( + [](base::FilePath* unpack_path, base::OnceClosure quit_closure, const ComponentUnpacker::Result& result) { EXPECT_EQ(UnpackerError::kNone, result.error); EXPECT_EQ(0, result.extended_error); *unpack_path = result.unpack_path; - quit_closure.Run(); + std::move(quit_closure).Run(); }, &unpack_path, runloop.QuitClosure())); @@ -3251,7 +3226,7 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { const std::vector<std::string> ids = {"gjpmebpgbhcamgdgjcmnjfhggjpgcimm"}; update_client->Update( ids, - base::Bind( + base::BindOnce( [](const base::FilePath& unpack_path, const std::vector<std::string>& ids, std::vector<CrxComponent>* components) { @@ -3264,10 +3239,10 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { components->push_back(crx); }, unpack_path), - base::Bind( - [](const base::Closure& quit_closure, Error error) { + base::BindOnce( + [](base::OnceClosure quit_closure, Error error) { EXPECT_EQ(Error::NONE, error); - quit_closure.Run(); + std::move(quit_closure).Run(); }, quit_closure())); diff --git a/chromium/components/update_client/update_engine.cc b/chromium/components/update_client/update_engine.cc index 664e74e8b28..4bb2d3ade9e 100644 --- a/chromium/components/update_client/update_engine.cc +++ b/chromium/components/update_client/update_engine.cc @@ -29,17 +29,17 @@ UpdateContext::UpdateContext( const scoped_refptr<Configurator>& config, bool is_foreground, const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, + UpdateClient::CrxDataCallback crx_data_callback, const UpdateEngine::NotifyObserversCallback& notify_observers_callback, - const UpdateEngine::Callback& callback, + UpdateEngine::Callback callback, CrxDownloader::Factory crx_downloader_factory) : config(config), is_foreground(is_foreground), enabled_component_updates(config->EnabledComponentUpdates()), ids(ids), - crx_data_callback(crx_data_callback), + crx_data_callback(std::move(crx_data_callback)), notify_observers_callback(notify_observers_callback), - callback(callback), + callback(std::move(callback)), crx_downloader_factory(crx_downloader_factory) { for (const auto& id : ids) components.insert( @@ -67,26 +67,26 @@ UpdateEngine::~UpdateEngine() { DCHECK(thread_checker_.CalledOnValidThread()); } -void UpdateEngine::Update( - bool is_foreground, - const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, - const Callback& callback) { +void UpdateEngine::Update(bool is_foreground, + const std::vector<std::string>& ids, + UpdateClient::CrxDataCallback crx_data_callback, + Callback callback) { DCHECK(thread_checker_.CalledOnValidThread()); if (IsThrottled(is_foreground)) { base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(callback, Error::RETRY_LATER)); + FROM_HERE, base::BindOnce(std::move(callback), Error::RETRY_LATER)); return; } const auto result = update_contexts_.insert(base::MakeUnique<UpdateContext>( - config_, is_foreground, ids, crx_data_callback, - notify_observers_callback_, callback, crx_downloader_factory_)); + config_, is_foreground, ids, std::move(crx_data_callback), + notify_observers_callback_, std::move(callback), + crx_downloader_factory_)); DCHECK(result.second); - const auto& it = result.first; + const auto it = result.first; const auto& update_context = *it; DCHECK(update_context); @@ -95,7 +95,8 @@ void UpdateEngine::Update( DCHECK_EQ(ids.size(), update_context->ids.size()); DCHECK_EQ(update_context->ids.size(), update_context->components.size()); std::vector<CrxComponent> crx_components; - update_context->crx_data_callback.Run(update_context->ids, &crx_components); + std::move(update_context->crx_data_callback) + .Run(update_context->ids, &crx_components); DCHECK_EQ(update_context->ids.size(), crx_components.size()); for (size_t i = 0; i != update_context->ids.size(); ++i) { @@ -113,29 +114,28 @@ void UpdateEngine::Update( component.set_previous_fp(crx_component.fingerprint); // Handle |kNew| state. This will transition the components to |kChecking|. - component.Handle(base::Bind(&UpdateEngine::ComponentCheckingForUpdatesStart, - base::Unretained(this), it, - base::ConstRef(component))); + component.Handle( + base::BindOnce(&UpdateEngine::ComponentCheckingForUpdatesStart, + base::Unretained(this), it, id)); } } void UpdateEngine::ComponentCheckingForUpdatesStart( - const UpdateContextIterator& it, - const Component& component) { + const UpdateContextIterator it, + const std::string& id) { DCHECK(thread_checker_.CalledOnValidThread()); const auto& update_context = *it; DCHECK(update_context); - const auto id = component.id(); DCHECK_EQ(1u, update_context->components.count(id)); DCHECK(update_context->components.at(id)); // Handle |kChecking| state. - auto& mutable_component = *update_context->components.at(id); - mutable_component.Handle(base::Bind( - &UpdateEngine::ComponentCheckingForUpdatesComplete, - base::Unretained(this), it, base::ConstRef(mutable_component))); + auto& component = *update_context->components.at(id); + component.Handle( + base::BindOnce(&UpdateEngine::ComponentCheckingForUpdatesComplete, + base::Unretained(this), it)); ++update_context->num_components_ready_to_check; if (update_context->num_components_ready_to_check < @@ -148,10 +148,10 @@ void UpdateEngine::ComponentCheckingForUpdatesStart( base::BindOnce(&UpdateEngine::DoUpdateCheck, base::Unretained(this), it)); } -void UpdateEngine::DoUpdateCheck(const UpdateContextIterator& it) { +void UpdateEngine::DoUpdateCheck(const UpdateContextIterator it) { DCHECK(thread_checker_.CalledOnValidThread()); - auto& update_context = *it; + const auto& update_context = *it; DCHECK(update_context); update_context->update_checker = @@ -160,15 +160,16 @@ void UpdateEngine::DoUpdateCheck(const UpdateContextIterator& it) { update_context->update_checker->CheckForUpdates( update_context->ids, update_context->components, config_->ExtraRequestParams(), update_context->enabled_component_updates, - base::Bind(&UpdateEngine::UpdateCheckDone, base::Unretained(this), it)); + base::BindOnce(&UpdateEngine::UpdateCheckDone, base::Unretained(this), + it)); } -void UpdateEngine::UpdateCheckDone(const UpdateContextIterator& it, +void UpdateEngine::UpdateCheckDone(const UpdateContextIterator it, int error, int retry_after_sec) { DCHECK(thread_checker_.CalledOnValidThread()); - auto& update_context = *it; + const auto& update_context = *it; DCHECK(update_context); update_context->retry_after_sec = retry_after_sec; @@ -198,8 +199,7 @@ void UpdateEngine::UpdateCheckDone(const UpdateContextIterator& it, } void UpdateEngine::ComponentCheckingForUpdatesComplete( - const UpdateContextIterator& it, - const Component& component) { + const UpdateContextIterator it) { DCHECK(thread_checker_.CalledOnValidThread()); const auto& update_context = *it; @@ -215,7 +215,7 @@ void UpdateEngine::ComponentCheckingForUpdatesComplete( base::Unretained(this), it)); } -void UpdateEngine::UpdateCheckComplete(const UpdateContextIterator& it) { +void UpdateEngine::UpdateCheckComplete(const UpdateContextIterator it) { DCHECK(thread_checker_.CalledOnValidThread()); const auto& update_context = *it; @@ -229,10 +229,10 @@ void UpdateEngine::UpdateCheckComplete(const UpdateContextIterator& it) { base::Unretained(this), it)); } -void UpdateEngine::HandleComponent(const UpdateContextIterator& it) { +void UpdateEngine::HandleComponent(const UpdateContextIterator it) { DCHECK(thread_checker_.CalledOnValidThread()); - auto& update_context = *it; + const auto& update_context = *it; DCHECK(update_context); auto& queue = update_context->component_queue; @@ -241,9 +241,10 @@ void UpdateEngine::HandleComponent(const UpdateContextIterator& it) { const Error error = update_context->update_check_error ? Error::UPDATE_CHECK_ERROR : Error::NONE; + base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(&UpdateEngine::UpdateComplete, - base::Unretained(this), it, error)); + FROM_HERE, base::BindOnce(&UpdateEngine::UpdateComplete, + base::Unretained(this), it, error)); return; } @@ -259,7 +260,6 @@ void UpdateEngine::HandleComponent(const UpdateContextIterator& it) { base::BindOnce(&UpdateEngine::HandleComponent, base::Unretained(this), it), next_update_delay); - next_update_delay = base::TimeDelta(); notify_observers_callback_.Run( @@ -267,14 +267,14 @@ void UpdateEngine::HandleComponent(const UpdateContextIterator& it) { return; } - component->Handle(base::Bind(&UpdateEngine::HandleComponentComplete, - base::Unretained(this), it)); + component->Handle(base::BindOnce(&UpdateEngine::HandleComponentComplete, + base::Unretained(this), it)); } -void UpdateEngine::HandleComponentComplete(const UpdateContextIterator& it) { +void UpdateEngine::HandleComponentComplete(const UpdateContextIterator it) { DCHECK(thread_checker_.CalledOnValidThread()); - auto& update_context = *it; + const auto& update_context = *it; DCHECK(update_context); auto& queue = update_context->component_queue; @@ -286,7 +286,7 @@ void UpdateEngine::HandleComponentComplete(const UpdateContextIterator& it) { DCHECK(component); if (component->IsHandled()) { - (*it)->next_update_delay = component->GetUpdateDuration(); + update_context->next_update_delay = component->GetUpdateDuration(); if (!component->events().empty()) { ping_manager_->SendPing(*component); @@ -300,15 +300,14 @@ void UpdateEngine::HandleComponentComplete(const UpdateContextIterator& it) { base::Unretained(this), it)); } -void UpdateEngine::UpdateComplete(const UpdateContextIterator& it, - Error error) { +void UpdateEngine::UpdateComplete(const UpdateContextIterator it, Error error) { DCHECK(thread_checker_.CalledOnValidThread()); - auto& update_context = *it; + const auto& update_context = *it; DCHECK(update_context); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(update_context->callback, error)); + FROM_HERE, base::BindOnce(std::move(update_context->callback), error)); update_contexts_.erase(it); } @@ -343,17 +342,17 @@ bool UpdateEngine::IsThrottled(bool is_foreground) const { void UpdateEngine::SendUninstallPing(const std::string& id, const base::Version& version, int reason, - const Callback& callback) { + Callback callback) { DCHECK(thread_checker_.CalledOnValidThread()); const auto result = update_contexts_.insert(base::MakeUnique<UpdateContext>( config_, false, std::vector<std::string>{id}, UpdateClient::CrxDataCallback(), UpdateEngine::NotifyObserversCallback(), - callback, nullptr)); + std::move(callback), nullptr)); DCHECK(result.second); - const auto& it = result.first; + const auto it = result.first; const auto& update_context = *it; DCHECK(update_context); DCHECK_EQ(1u, update_context->ids.size()); diff --git a/chromium/components/update_client/update_engine.h b/chromium/components/update_client/update_engine.h index 64d220e7e15..9e3789259eb 100644 --- a/chromium/components/update_client/update_engine.h +++ b/chromium/components/update_client/update_engine.h @@ -40,7 +40,7 @@ struct UpdateContext; // applied one at a time. class UpdateEngine { public: - using Callback = base::Callback<void(Error error)>; + using Callback = base::OnceCallback<void(Error error)>; using NotifyObserversCallback = base::Callback<void(UpdateClient::Observer::Events event, const std::string& id)>; @@ -57,33 +57,32 @@ class UpdateEngine { void Update(bool is_foreground, const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, - const Callback& update_callback); + UpdateClient::CrxDataCallback crx_data_callback, + Callback update_callback); void SendUninstallPing(const std::string& id, const base::Version& version, int reason, - const Callback& update_callback); + Callback update_callback); private: using UpdateContexts = std::set<std::unique_ptr<UpdateContext>>; using UpdateContextIterator = UpdateContexts::iterator; - void UpdateComplete(const UpdateContextIterator& it, Error error); + void UpdateComplete(UpdateContextIterator it, Error error); - void ComponentCheckingForUpdatesStart(const UpdateContextIterator& it, - const Component& component); - void ComponentCheckingForUpdatesComplete(const UpdateContextIterator& it, - const Component& component); - void UpdateCheckComplete(const UpdateContextIterator& it); + void ComponentCheckingForUpdatesStart(UpdateContextIterator it, + const std::string& id); + void ComponentCheckingForUpdatesComplete(UpdateContextIterator it); + void UpdateCheckComplete(UpdateContextIterator it); - void DoUpdateCheck(const UpdateContextIterator& it); - void UpdateCheckDone(const UpdateContextIterator& it, + void DoUpdateCheck(UpdateContextIterator it); + void UpdateCheckDone(UpdateContextIterator it, int error, int retry_after_sec); - void HandleComponent(const UpdateContextIterator& it); - void HandleComponentComplete(const UpdateContextIterator& it); + void HandleComponent(UpdateContextIterator it); + void HandleComponentComplete(UpdateContextIterator it); // Returns true if the update engine rejects this update call because it // occurs too soon. @@ -124,9 +123,9 @@ struct UpdateContext { const scoped_refptr<Configurator>& config, bool is_foreground, const std::vector<std::string>& ids, - const UpdateClient::CrxDataCallback& crx_data_callback, + UpdateClient::CrxDataCallback crx_data_callback, const UpdateEngine::NotifyObserversCallback& notify_observers_callback, - const UpdateEngine::Callback& callback, + UpdateEngine::Callback callback, CrxDownloader::Factory crx_downloader_factory); ~UpdateContext(); @@ -143,13 +142,13 @@ struct UpdateContext { const std::vector<std::string> ids; // Called before an update check, when update metadata is needed. - const UpdateEngine::CrxDataCallback& crx_data_callback; + UpdateEngine::CrxDataCallback crx_data_callback; // Called when there is a state change for any update in this context. const UpdateEngine::NotifyObserversCallback notify_observers_callback; // Called when the all updates associated with this context have completed. - const UpdateEngine::Callback callback; + UpdateEngine::Callback callback; // Creates instances of CrxDownloader; CrxDownloader::Factory crx_downloader_factory; diff --git a/chromium/components/update_client/update_query_params.cc b/chromium/components/update_client/update_query_params.cc index 65d53ba79ee..319d59ea685 100644 --- a/chromium/components/update_client/update_query_params.cc +++ b/chromium/components/update_client/update_query_params.cc @@ -66,7 +66,7 @@ const char kChromeCrx[] = "chromecrx"; const char kChromiumCrx[] = "chromiumcrx"; #endif // defined(GOOGLE_CHROME_BUILD) -UpdateQueryParamsDelegate* g_delegate = NULL; +UpdateQueryParamsDelegate* g_delegate = nullptr; } // namespace diff --git a/chromium/components/update_client/url_fetcher_downloader.cc b/chromium/components/update_client/url_fetcher_downloader.cc index 2213f483fff..77dd764b09a 100644 --- a/chromium/components/update_client/url_fetcher_downloader.cc +++ b/chromium/components/update_client/url_fetcher_downloader.cc @@ -32,10 +32,32 @@ constexpr base::TaskTraits kTaskTraits = { namespace update_client { +UrlFetcherDownloader::URLFetcherDelegate::URLFetcherDelegate( + UrlFetcherDownloader* downloader) + : downloader_(downloader) {} + +UrlFetcherDownloader::URLFetcherDelegate::~URLFetcherDelegate() = default; + +void UrlFetcherDownloader::URLFetcherDelegate::OnURLFetchComplete( + const net::URLFetcher* source) { + downloader_->OnURLFetchComplete(source); +} + +void UrlFetcherDownloader::URLFetcherDelegate::OnURLFetchDownloadProgress( + const net::URLFetcher* source, + int64_t current, + int64_t total, + int64_t current_network_bytes) { + downloader_->OnURLFetchDownloadProgress(source, current, total, + current_network_bytes); +} + UrlFetcherDownloader::UrlFetcherDownloader( std::unique_ptr<CrxDownloader> successor, net::URLRequestContextGetter* context_getter) - : CrxDownloader(std::move(successor)), context_getter_(context_getter) {} + : CrxDownloader(std::move(successor)), + delegate_(std::make_unique<URLFetcherDelegate>(this)), + context_getter_(context_getter) {} UrlFetcherDownloader::~UrlFetcherDownloader() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -113,8 +135,8 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) { const base::FilePath response = download_dir_.AppendASCII(url.ExtractFileName()); - url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, this, - traffic_annotation); + url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, + delegate_.get(), traffic_annotation); url_fetcher_->SetRequestContext(context_getter_); url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES | diff --git a/chromium/components/update_client/url_fetcher_downloader.h b/chromium/components/update_client/url_fetcher_downloader.h index 2007aa76c8a..2241adc8fe1 100644 --- a/chromium/components/update_client/url_fetcher_downloader.h +++ b/chromium/components/update_client/url_fetcher_downloader.h @@ -25,29 +25,45 @@ class URLRequestContextGetter; namespace update_client { // Implements a CRX downloader in top of the URLFetcher. -class UrlFetcherDownloader : public CrxDownloader, - public net::URLFetcherDelegate { +class UrlFetcherDownloader : public CrxDownloader { public: UrlFetcherDownloader(std::unique_ptr<CrxDownloader> successor, net::URLRequestContextGetter* context_getter); ~UrlFetcherDownloader() override; private: + class URLFetcherDelegate : public net::URLFetcherDelegate { + public: + explicit URLFetcherDelegate(UrlFetcherDownloader* downloader); + ~URLFetcherDelegate() override; + + private: + // Overrides for URLFetcherDelegate. + void OnURLFetchComplete(const net::URLFetcher* source) override; + void OnURLFetchDownloadProgress(const net::URLFetcher* source, + int64_t current, + int64_t total, + int64_t current_network_bytes) override; + // Not owned by this class. + UrlFetcherDownloader* downloader_ = nullptr; + DISALLOW_COPY_AND_ASSIGN(URLFetcherDelegate); + }; + // Overrides for CrxDownloader. void DoStartDownload(const GURL& url) override; - // Overrides for URLFetcherDelegate. - void OnURLFetchComplete(const net::URLFetcher* source) override; + void CreateDownloadDir(); + void StartURLFetch(const GURL& url); + void OnURLFetchComplete(const net::URLFetcher* source); void OnURLFetchDownloadProgress(const net::URLFetcher* source, int64_t current, int64_t total, - int64_t current_network_bytes) override; - - void CreateDownloadDir(); - void StartURLFetch(const GURL& url); + int64_t current_network_bytes); THREAD_CHECKER(thread_checker_); + std::unique_ptr<URLFetcherDelegate> delegate_; + std::unique_ptr<net::URLFetcher> url_fetcher_; net::URLRequestContextGetter* context_getter_ = nullptr; diff --git a/chromium/components/update_client/url_request_post_interceptor.cc b/chromium/components/update_client/url_request_post_interceptor.cc index 50bf0370039..ac2cdb5431f 100644 --- a/chromium/components/update_client/url_request_post_interceptor.cc +++ b/chromium/components/update_client/url_request_post_interceptor.cc @@ -185,7 +185,7 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor { // Only intercepts POST. if (!request->has_upload()) - return NULL; + return nullptr; GURL url = request->url(); if (url.has_query()) { @@ -196,7 +196,7 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor { InterceptorMap::const_iterator it(interceptors_.find(url)); if (it == interceptors_.end()) - return NULL; + return nullptr; // There is an interceptor hooked up for this url. Read the request body, // check the existing expectations, and handle the matching case by @@ -215,7 +215,7 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor { base::AutoLock auto_lock(interceptor->interceptor_lock_); interceptor->requests_.push_back(request_body); if (interceptor->expectations_.empty()) - return NULL; + return nullptr; const URLRequestPostInterceptor::Expectation& expectation( interceptor->expectations_.front()); if (expectation.first->Match(request_body)) { @@ -230,7 +230,7 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor { } } - return NULL; + return nullptr; } typedef std::map<GURL, URLRequestPostInterceptor*> InterceptorMap; @@ -254,14 +254,15 @@ URLRequestPostInterceptorFactory::URLRequestPostInterceptorFactory( hostname, io_task_runner)) { io_task_runner_->PostTask( - FROM_HERE, base::Bind(&URLRequestPostInterceptor::Delegate::Register, - base::Unretained(delegate_))); + FROM_HERE, base::BindOnce(&URLRequestPostInterceptor::Delegate::Register, + base::Unretained(delegate_))); } URLRequestPostInterceptorFactory::~URLRequestPostInterceptorFactory() { io_task_runner_->PostTask( - FROM_HERE, base::Bind(&URLRequestPostInterceptor::Delegate::Unregister, - base::Unretained(delegate_))); + FROM_HERE, + base::BindOnce(&URLRequestPostInterceptor::Delegate::Unregister, + base::Unretained(delegate_))); } URLRequestPostInterceptor* URLRequestPostInterceptorFactory::CreateInterceptor( @@ -273,11 +274,12 @@ URLRequestPostInterceptor* URLRequestPostInterceptorFactory::CreateInterceptor( new URLRequestPostInterceptor(absolute_url, io_task_runner_)); bool res = io_task_runner_->PostTask( FROM_HERE, - base::Bind(&URLRequestPostInterceptor::Delegate::OnCreateInterceptor, - base::Unretained(delegate_), base::Unretained(interceptor))); + base::BindOnce(&URLRequestPostInterceptor::Delegate::OnCreateInterceptor, + base::Unretained(delegate_), + base::Unretained(interceptor))); if (!res) { delete interceptor; - return NULL; + return nullptr; } return interceptor; diff --git a/chromium/components/update_client/utils.cc b/chromium/components/update_client/utils.cc index 8d4afcfbff7..c200deff0be 100644 --- a/chromium/components/update_client/utils.cc +++ b/chromium/components/update_client/utils.cc @@ -16,9 +16,11 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/memory_mapped_file.h" +#include "base/json/json_file_value_serializer.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" #include "base/strings/string_util.h" +#include "base/values.h" #include "components/crx_file/id_util.h" #include "components/data_use_measurement/core/data_use_user_data.h" #include "components/update_client/component.h" @@ -36,30 +38,6 @@ namespace update_client { -namespace { - -// Produces an extension-like friendly id. -std::string HexStringToID(const std::string& hexstr) { - std::string id; - for (size_t i = 0; i < hexstr.size(); ++i) { - int val = 0; - if (base::HexStringToInt( - base::StringPiece(hexstr.begin() + i, hexstr.begin() + i + 1), - &val)) { - id.append(1, val + 'a'); - } else { - id.append(1, 'a'); - } - } - - DCHECK(crx_file::id_util::IdIsValid(id)); - - return id; -} - -} // namespace - - std::unique_ptr<net::URLFetcher> SendProtocolRequest( const GURL& url, const std::string& protocol_request, @@ -163,10 +141,10 @@ bool DeleteFileAndEmptyParentDirectory(const base::FilePath& filepath) { } std::string GetCrxComponentID(const CrxComponent& component) { - const size_t kCrxIdSize = 16; - CHECK_GE(component.pk_hash.size(), kCrxIdSize); - return HexStringToID(base::ToLowerASCII( - base::HexEncode(&component.pk_hash[0], kCrxIdSize))); + const std::string result = crx_file::id_util::GenerateIdFromHash( + &component.pk_hash[0], component.pk_hash.size()); + DCHECK(crx_file::id_util::IdIsValid(result)); + return result; } bool VerifyFileHash256(const base::FilePath& filepath, @@ -246,9 +224,31 @@ void RemoveUnsecureUrls(std::vector<GURL>* urls) { urls->end()); } -CrxInstaller::Result InstallFunctionWrapper(base::Callback<bool()> callback) { - return CrxInstaller::Result(callback.Run() ? InstallError::NONE - : InstallError::GENERIC_ERROR); +CrxInstaller::Result InstallFunctionWrapper( + base::OnceCallback<bool()> callback) { + return CrxInstaller::Result(std::move(callback).Run() + ? InstallError::NONE + : InstallError::GENERIC_ERROR); +} + +// TODO(cpu): add a specific attribute check to a component json that the +// extension unpacker will reject, so that a component cannot be installed +// as an extension. +std::unique_ptr<base::DictionaryValue> ReadManifest( + const base::FilePath& unpack_path) { + base::FilePath manifest = + unpack_path.Append(FILE_PATH_LITERAL("manifest.json")); + if (!base::PathExists(manifest)) + return std::unique_ptr<base::DictionaryValue>(); + JSONFileValueDeserializer deserializer(manifest); + std::string error; + std::unique_ptr<base::Value> root = deserializer.Deserialize(nullptr, &error); + if (!root.get()) + return std::unique_ptr<base::DictionaryValue>(); + if (!root->is_dict()) + return std::unique_ptr<base::DictionaryValue>(); + return std::unique_ptr<base::DictionaryValue>( + static_cast<base::DictionaryValue*>(root.release())); } } // namespace update_client diff --git a/chromium/components/update_client/utils.h b/chromium/components/update_client/utils.h index 7655ffb44f9..30f6edd82d7 100644 --- a/chromium/components/update_client/utils.h +++ b/chromium/components/update_client/utils.h @@ -16,6 +16,7 @@ class GURL; namespace base { +class DictionaryValue; class FilePath; } @@ -86,7 +87,12 @@ void RemoveUnsecureUrls(std::vector<GURL>* urls); // Adapter function for the old definitions of CrxInstaller::Install until the // component installer code is migrated to use a Result instead of bool. -CrxInstaller::Result InstallFunctionWrapper(base::Callback<bool()> callback); +CrxInstaller::Result InstallFunctionWrapper( + base::OnceCallback<bool()> callback); + +// Deserializes the CRX manifest. The top level must be a dictionary. +std::unique_ptr<base::DictionaryValue> ReadManifest( + const base::FilePath& unpack_path); } // namespace update_client diff --git a/chromium/components/update_client/utils_unittest.cc b/chromium/components/update_client/utils_unittest.cc index d3d4f189d58..77ea7ae3b51 100644 --- a/chromium/components/update_client/utils_unittest.cc +++ b/chromium/components/update_client/utils_unittest.cc @@ -67,6 +67,18 @@ TEST(UpdateClientUtils, IsValidBrand) { EXPECT_FALSE(IsValidBrand(std::string("\xaa"))); // Has non-ASCII char. } +TEST(UpdateClientUtils, GetCrxComponentId) { + static const uint8_t kHash[16] = { + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, + }; + CrxComponent component; + component.pk_hash.assign(kHash, kHash + sizeof(kHash)); + + EXPECT_EQ(std::string("abcdefghijklmnopabcdefghijklmnop"), + GetCrxComponentID(component)); +} + // Tests that the name of an InstallerAttribute matches ^[-_=a-zA-Z0-9]{1,256}$ TEST(UpdateClientUtils, IsValidInstallerAttributeName) { // Test the length boundaries. |