Skip to content

Commit

Permalink
update_client: Move TLS fallback loop into the update checker.
Browse files Browse the repository at this point in the history 
Fixed: 645654
Change-Id: I850b86245667c38de545ccb1b74e50376a786025
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3638240
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002220}
  • Loading branch information
Joshua Pawlicki authored and Chromium LUCI CQ committed May 11, 2022
1 parent b44ece5 commit c4e4d69
Show file tree
Hide file tree
Showing 9 changed files with 295 additions and 336 deletions.
56 changes: 15 additions & 41 deletions components/component_updater/component_updater_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,52 +367,26 @@ bool CrxUpdateService::CheckForUpdates(
UMA_HISTOGRAM_ENUMERATION("ComponentUpdater.Calls", UPDATE_TYPE_AUTOMATIC,
UPDATE_TYPE_COUNT);

std::vector<std::string> secure_ids; // Requires HTTPS for update checks.
std::vector<std::string> unsecure_ids; // Can fallback to HTTP.
for (const auto& id : components_order_) {
DCHECK(components_.find(id) != components_.end());

const auto component = GetComponent(id);
if (!component || component->requires_network_encryption)
secure_ids.push_back(id);
else
unsecure_ids.push_back(id);
}

if (unsecure_ids.empty() && secure_ids.empty()) {
if (components_order_.empty()) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
std::move(on_finished));
return true;
}

Callback on_finished_callback = base::BindOnce(
[](UpdateScheduler::OnFinishedCallback on_finished,
update_client::Error error) { std::move(on_finished).Run(); },
std::move(on_finished));

if (!unsecure_ids.empty()) {
update_client_->Update(
unsecure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
{}, false,
base::BindOnce(
&CrxUpdateService::OnUpdateComplete, base::Unretained(this),
secure_ids.empty() ? std::move(on_finished_callback) : Callback(),
base::TimeTicks::Now()));
}

if (!secure_ids.empty()) {
update_client_->Update(
secure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
{}, false,
base::BindOnce(&CrxUpdateService::OnUpdateComplete,
base::Unretained(this), std::move(on_finished_callback),
base::TimeTicks::Now()));
}

update_client_->Update(
components_order_,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
{}, false,
base::BindOnce(&CrxUpdateService::OnUpdateComplete,
base::Unretained(this),
base::BindOnce(
[](UpdateScheduler::OnFinishedCallback on_finished,
update_client::Error /*error*/) {
std::move(on_finished).Run();
},
std::move(on_finished)),
base::TimeTicks::Now()));
return true;
}

Expand Down
3 changes: 2 additions & 1 deletion components/update_client/configurator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> {
virtual int UpdateDelay() const = 0;

// The URLs for the update checks. The URLs are tried in order, the first one
// that succeeds wins.
// that succeeds wins. Since some components cannot be updated over HTTP,
// HTTPS URLs should appear first.
virtual std::vector<GURL> UpdateUrl() const = 0;

// The URLs for pings. Returns an empty vector if and only if pings are
Expand Down
10 changes: 7 additions & 3 deletions components/update_client/test_configurator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ int TestConfigurator::UpdateDelay() const {
}

std::vector<GURL> TestConfigurator::UpdateUrl() const {
if (!update_check_url_.is_empty())
return std::vector<GURL>(1, update_check_url_);
if (!update_check_urls_.empty())
return update_check_urls_;

return MakeDefaultUrls();
}
Expand Down Expand Up @@ -191,7 +191,11 @@ void TestConfigurator::SetDownloadPreference(
}

void TestConfigurator::SetUpdateCheckUrl(const GURL& url) {
update_check_url_ = url;
update_check_urls_ = {url};
}

void TestConfigurator::SetUpdateCheckUrls(const std::vector<GURL>& urls) {
update_check_urls_ = urls;
}

void TestConfigurator::SetPingUrl(const GURL& url) {
Expand Down
3 changes: 2 additions & 1 deletion components/update_client/test_configurator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class TestConfigurator : public Configurator {
void SetDownloadPreference(const std::string& download_preference);
void SetEnabledCupSigning(bool use_cup_signing);
void SetUpdateCheckUrl(const GURL& url);
void SetUpdateCheckUrls(const std::vector<GURL>& urls);
void SetPingUrl(const GURL& url);
void SetCrxDownloaderFactory(
scoped_refptr<CrxDownloaderFactory> crx_downloader_factory);
Expand All @@ -130,7 +131,7 @@ class TestConfigurator : public Configurator {
std::string download_preference_;
bool enabled_cup_signing_;
raw_ptr<PrefService> pref_service_; // Not owned by this class.
GURL update_check_url_;
std::vector<GURL> update_check_urls_;
GURL ping_url_;

scoped_refptr<update_client::UnzipChromiumFactory> unzip_factory_;
Expand Down
110 changes: 67 additions & 43 deletions components/update_client/update_checker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,15 @@
#include "components/update_client/request_sender.h"
#include "components/update_client/task_traits.h"
#include "components/update_client/update_client.h"
#include "components/update_client/update_engine.h"
#include "components/update_client/utils.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"

namespace update_client {

namespace {

// Returns true if at least one item requires network encryption.
bool IsEncryptionRequired(const IdToComponentPtrMap& components) {
for (const auto& item : components) {
const auto& component = item.second;
if (component->crx_component() &&
component->crx_component()->requires_network_encryption)
return true;
}
return false;
}

class UpdateCheckerImpl : public UpdateChecker {
public:
UpdateCheckerImpl(scoped_refptr<Configurator> config,
Expand All @@ -62,24 +53,25 @@ class UpdateCheckerImpl : public UpdateChecker {

// Overrides for UpdateChecker.
void CheckForUpdates(
const std::string& session_id,
const std::vector<std::string>& ids_checked,
const IdToComponentPtrMap& components,
scoped_refptr<UpdateContext> context,
const base::flat_map<std::string, std::string>& additional_attributes,
UpdateCheckCallback update_check_callback) override;

private:
UpdaterStateAttributes ReadUpdaterStateAttributes() const;
void CheckForUpdatesHelper(
const std::string& session_id,
const IdToComponentPtrMap& components,
scoped_refptr<UpdateContext> context,
const std::vector<GURL>& urls,
const base::flat_map<std::string, std::string>& additional_attributes,
const UpdaterStateAttributes& updater_state_attributes,
const std::set<std::string>& active_ids);
void OnRequestSenderComplete(int error,
void OnRequestSenderComplete(scoped_refptr<UpdateContext> context,
absl::optional<base::OnceClosure> fallback,
int error,
const std::string& response,
int retry_after_sec);
void UpdateCheckSucceeded(const ProtocolParser::Results& results,
void UpdateCheckSucceeded(scoped_refptr<UpdateContext> context,
const ProtocolParser::Results& results,
int retry_after_sec);
void UpdateCheckFailed(ErrorCategory error_category,
int error,
Expand All @@ -89,7 +81,6 @@ class UpdateCheckerImpl : public UpdateChecker {

const scoped_refptr<Configurator> config_;
raw_ptr<PersistedData> metadata_ = nullptr;
std::vector<std::string> ids_checked_;
UpdateCheckCallback update_check_callback_;
std::unique_ptr<RequestSender> request_sender_;
};
Expand All @@ -103,19 +94,16 @@ UpdateCheckerImpl::~UpdateCheckerImpl() {
}

void UpdateCheckerImpl::CheckForUpdates(
const std::string& session_id,
const std::vector<std::string>& ids_checked,
const IdToComponentPtrMap& components,
scoped_refptr<UpdateContext> context,
const base::flat_map<std::string, std::string>& additional_attributes,
UpdateCheckCallback update_check_callback) {
DCHECK(thread_checker_.CalledOnValidThread());

ids_checked_ = ids_checked;
update_check_callback_ = std::move(update_check_callback);

auto check_for_updates_invoker = base::BindOnce(
&UpdateCheckerImpl::CheckForUpdatesHelper, base::Unretained(this),
session_id, std::cref(components), additional_attributes);
context, config_->UpdateUrl(), additional_attributes);

base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, kTaskTraits,
Expand All @@ -132,7 +120,7 @@ void UpdateCheckerImpl::CheckForUpdates(
updater_state_attributes));
},
std::move(check_for_updates_invoker), base::Unretained(metadata_),
ids_checked));
context->components_to_check_for_updates));
}

// This function runs on the blocking pool task runner.
Expand All @@ -148,36 +136,49 @@ UpdaterStateAttributes UpdateCheckerImpl::ReadUpdaterStateAttributes() const {
}

void UpdateCheckerImpl::CheckForUpdatesHelper(
const std::string& session_id,
const IdToComponentPtrMap& components,
scoped_refptr<UpdateContext> context,
const std::vector<GURL>& urls,
const base::flat_map<std::string, std::string>& additional_attributes,
const UpdaterStateAttributes& updater_state_attributes,
const std::set<std::string>& active_ids) {
DCHECK(thread_checker_.CalledOnValidThread());

auto urls(config_->UpdateUrl());
if (IsEncryptionRequired(components))
RemoveUnsecureUrls(&urls);
if (urls.empty()) {
UpdateCheckFailed(ErrorCategory::kUpdateCheck,
static_cast<int>(ProtocolError::MISSING_URLS), 0);
return;
}
GURL url = urls.front();

// Components in this update check are either all foreground, or all
// background since this member is inherited from the component's update
// context. Pick the state of the first component to use in the update check.
DCHECK(!components.empty());
const bool is_foreground = components.at(ids_checked_[0])->is_foreground();
DCHECK(!context->components.empty());
const bool is_foreground =
context->components.cbegin()->second->is_foreground();
DCHECK(
std::all_of(components.cbegin(), components.cend(),
std::all_of(context->components.cbegin(), context->components.cend(),
[is_foreground](IdToComponentPtrMap::const_reference& elem) {
return is_foreground == elem.second->is_foreground();
}));

std::vector<std::string> sent_ids;

std::vector<protocol_request::App> apps;
for (const auto& app_id : ids_checked_) {
DCHECK_EQ(1u, components.count(app_id));
const auto& component = components.at(app_id);
for (const auto& app_id : context->components_to_check_for_updates) {
DCHECK_EQ(1u, context->components.count(app_id));
const auto& component = context->components.at(app_id);
DCHECK_EQ(component->id(), app_id);
const auto& crx_component = component->crx_component();
DCHECK(crx_component);

if (crx_component->requires_network_encryption &&
!url.SchemeIsCryptographic()) {
continue;
}

sent_ids.push_back(app_id);

std::string install_source;
if (!crx_component->install_source.empty())
install_source = crx_component->install_source;
Expand Down Expand Up @@ -207,35 +208,56 @@ void UpdateCheckerImpl::CheckForUpdatesHelper(
absl::nullopt));
}

if (sent_ids.empty()) {
// No apps could be checked over this URL.
UpdateCheckFailed(ErrorCategory::kUpdateCheck,
static_cast<int>(ProtocolError::MISSING_URLS), 0);
return;
}

const auto request = MakeProtocolRequest(
!config_->IsPerUserInstall(), session_id, config_->GetProdId(),
!config_->IsPerUserInstall(), context->session_id, config_->GetProdId(),
config_->GetBrowserVersion().GetString(), config_->GetChannel(),
config_->GetOSLongName(), config_->GetDownloadPreference(),
config_->IsMachineExternallyManaged(), additional_attributes,
updater_state_attributes, std::move(apps));

request_sender_ = std::make_unique<RequestSender>(config_);
request_sender_->Send(
urls,
{url},
BuildUpdateCheckExtraRequestHeaders(config_->GetProdId(),
config_->GetBrowserVersion(),
ids_checked_, is_foreground),
sent_ids, is_foreground),
config_->GetProtocolHandlerFactory()->CreateSerializer()->Serialize(
request),
config_->EnabledCupSigning(),
base::BindOnce(&UpdateCheckerImpl::OnRequestSenderComplete,
base::Unretained(this)));
base::Unretained(this), context,
urls.size() > 1
? absl::optional<base::OnceClosure>(base::BindOnce(
&UpdateCheckerImpl::CheckForUpdatesHelper,
base::Unretained(this), context,
std::vector<GURL>(urls.begin() + 1, urls.end()),
additional_attributes, updater_state_attributes,
active_ids))
: absl::nullopt));
}

void UpdateCheckerImpl::OnRequestSenderComplete(
scoped_refptr<UpdateContext> context,
absl::optional<base::OnceClosure> fallback,
int error,
const std::string& response,
int retry_after_sec) {
DCHECK(thread_checker_.CalledOnValidThread());

if (error) {
VLOG(1) << "RequestSender failed " << error;
UpdateCheckFailed(ErrorCategory::kUpdateCheck, error, retry_after_sec);
if (fallback && retry_after_sec <= 0) {
std::move(*fallback).Run();
} else {
UpdateCheckFailed(ErrorCategory::kUpdateCheck, error, retry_after_sec);
}
return;
}

Expand All @@ -249,10 +271,11 @@ void UpdateCheckerImpl::OnRequestSenderComplete(
}

DCHECK_EQ(0, error);
UpdateCheckSucceeded(parser->results(), retry_after_sec);
UpdateCheckSucceeded(context, parser->results(), retry_after_sec);
}

void UpdateCheckerImpl::UpdateCheckSucceeded(
scoped_refptr<UpdateContext> context,
const ProtocolParser::Results& results,
int retry_after_sec) {
DCHECK(thread_checker_.CalledOnValidThread());
Expand All @@ -276,7 +299,8 @@ void UpdateCheckerImpl::UpdateCheckSucceeded(
ErrorCategory::kNone, 0, retry_after_sec);

if (daynum != ProtocolParser::kNoDaystart) {
metadata_->SetDateLastData(ids_checked_, daynum, std::move(reply));
metadata_->SetDateLastData(context->components_to_check_for_updates, daynum,
std::move(reply));
return;
}

Expand Down
13 changes: 5 additions & 8 deletions components/update_client/update_checker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace update_client {

class Configurator;
class PersistedData;
struct UpdateContext;

class UpdateChecker {
public:
Expand All @@ -40,15 +41,11 @@ class UpdateChecker {
virtual ~UpdateChecker() = default;

// Initiates an update check for the components specified by their ids.
// |additional_attributes| provides a way to customize the <request> element.
// |is_foreground| controls the value of "X-Goog-Update-Interactivity"
// header which is sent with the update check.
// On completion, the state of |components| is mutated as required by the
// server response received.
// `update_context` contains the updateable apps. `additional_attributes`
// specifies any extra request data to send. On completion, the state of
// `components` is mutated as required by the server response received.
virtual void CheckForUpdates(
const std::string& session_id,
const std::vector<std::string>& ids_to_check,
const IdToComponentPtrMap& components,
scoped_refptr<UpdateContext> update_context,
const base::flat_map<std::string, std::string>& additional_attributes,
UpdateCheckCallback update_check_callback) = 0;

Expand Down

0 comments on commit c4e4d69

Please sign in to comment.