Skip to content

Commit

Permalink
Update components/update_client to new Value API
Browse files Browse the repository at this point in the history 
Bug: 646113
Change-Id: I0e311b677ed48fd4f02d3739be6109133f979530
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4240982
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104779}
  • Loading branch information
Avi Drissman authored and Chromium LUCI CQ committed Feb 14, 2023
1 parent d9aafd9 commit 0b357d9
Show file tree
Hide file tree
Showing 19 changed files with 204 additions and 188 deletions.
35 changes: 18 additions & 17 deletions components/component_updater/component_installer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,13 @@ Result ComponentInstaller::InstallHelper(const base::FilePath& unpack_path,
base::Value::Dict* manifest,
base::Version* version,
base::FilePath* install_path) {
base::Value local_manifest_value = update_client::ReadManifest(unpack_path);
if (!local_manifest_value.is_dict())
absl::optional<base::Value::Dict> local_manifest =
update_client::ReadManifest(unpack_path);
if (!local_manifest) {
return Result(InstallError::BAD_MANIFEST);
base::Value::Dict local_manifest(std::move(local_manifest_value).TakeDict());
}

const std::string* version_ascii = local_manifest.FindString("version");
const std::string* version_ascii = local_manifest->FindString("version");
if (!version_ascii || !base::IsStringASCII(*version_ascii))
return Result(InstallError::INVALID_VERSION);

Expand Down Expand Up @@ -176,16 +177,16 @@ Result ComponentInstaller::InstallHelper(const base::FilePath& unpack_path,
#endif

const Result result =
installer_policy_->OnCustomInstall(local_manifest, local_install_path);
installer_policy_->OnCustomInstall(*local_manifest, local_install_path);
if (result.error)
return result;

if (!installer_policy_->VerifyInstallation(local_manifest,
if (!installer_policy_->VerifyInstallation(*local_manifest,
local_install_path)) {
return Result(InstallError::INSTALL_VERIFICATION_FAILED);
}

*manifest = std::move(local_manifest);
*manifest = std::move(local_manifest.value());
*version = manifest_version;
*install_path = install_path_owner.Take();

Expand Down Expand Up @@ -245,19 +246,19 @@ bool ComponentInstaller::FindPreinstallation(
return false;
}

base::Value manifest_value = update_client::ReadManifest(path);
if (!manifest_value.is_dict()) {
absl::optional<base::Value::Dict> manifest =
update_client::ReadManifest(path);
if (!manifest) {
DVLOG(1) << "Manifest does not exist: " << path.MaybeAsASCII();
return false;
}
base::Value::Dict manifest(std::move(manifest_value).TakeDict());

if (!installer_policy_->VerifyInstallation(manifest, path)) {
if (!installer_policy_->VerifyInstallation(*manifest, path)) {
DVLOG(1) << "Installation verification failed: " << path.MaybeAsASCII();
return false;
}

std::string* version_lexical = manifest.FindString("version");
std::string* version_lexical = manifest->FindString("version");
if (!version_lexical || !base::IsStringASCII(*version_lexical)) {
DVLOG(1) << "Failed to get component version from the manifest.";
return false;
Expand All @@ -284,23 +285,23 @@ bool ComponentInstaller::FindPreinstallation(
// its manifest if it is.
absl::optional<base::Value::Dict>
ComponentInstaller::GetValidInstallationManifest(const base::FilePath& path) {
base::Value manifest_value = update_client::ReadManifest(path);
if (!manifest_value.is_dict()) {
absl::optional<base::Value::Dict> manifest =
update_client::ReadManifest(path);
if (!manifest) {
PLOG(ERROR) << "Failed to read manifest for "
<< installer_policy_->GetName() << " (" << path.MaybeAsASCII()
<< ").";
return absl::nullopt;
}
base::Value::Dict manifest(std::move(manifest_value).TakeDict());

if (!installer_policy_->VerifyInstallation(manifest, path)) {
if (!installer_policy_->VerifyInstallation(*manifest, path)) {
PLOG(ERROR) << "Failed to verify installation for "
<< installer_policy_->GetName() << " (" << path.MaybeAsASCII()
<< ").";
return absl::nullopt;
}

const base::Value::List* accept_archs = manifest.FindList("accept_arch");
const base::Value::List* accept_archs = manifest->FindList("accept_arch");
if (accept_archs != nullptr &&
base::ranges::none_of(*accept_archs, [](const base::Value& v) {
static const char* current_arch =
Expand Down
157 changes: 85 additions & 72 deletions components/update_client/component.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ bool Component::CanDoBackgroundDownload() const {
update_context_->config->EnabledBackgroundDownloader();
}

void Component::AppendEvent(base::Value event) {
void Component::AppendEvent(base::Value::Dict event) {
events_.push_back(std::move(event));
}

Expand Down Expand Up @@ -650,112 +650,125 @@ base::TimeDelta Component::GetUpdateDuration() const {
return std::min(update_cost, max_update_delay);
}

base::Value Component::MakeEventUpdateComplete() const {
base::Value event(base::Value::Type::DICT);
event.SetKey("eventtype", base::Value(update_context_->is_install ? 2 : 3));
event.SetKey(
"eventresult",
base::Value(static_cast<int>(state() == ComponentState::kUpdated)));
if (error_category() != ErrorCategory::kNone)
event.SetKey("errorcat", base::Value(static_cast<int>(error_category())));
if (error_code())
event.SetKey("errorcode", base::Value(error_code()));
if (extra_code1())
event.SetKey("extracode1", base::Value(extra_code1()));
base::Value::Dict Component::MakeEventUpdateComplete() const {
base::Value::Dict event;
event.Set("eventtype", update_context_->is_install ? 2 : 3);
event.Set("eventresult",
static_cast<int>(state() == ComponentState::kUpdated));
if (error_category() != ErrorCategory::kNone) {
event.Set("errorcat", static_cast<int>(error_category()));
}
if (error_code()) {
event.Set("errorcode", error_code());
}
if (extra_code1()) {
event.Set("extracode1", extra_code1());
}
if (HasDiffUpdate(*this)) {
const int diffresult = static_cast<int>(!diff_update_failed());
event.SetKey("diffresult", base::Value(diffresult));
event.Set("diffresult", diffresult);
}
if (diff_error_category() != ErrorCategory::kNone) {
const int differrorcat = static_cast<int>(diff_error_category());
event.SetKey("differrorcat", base::Value(differrorcat));
event.Set("differrorcat", differrorcat);
}
if (diff_error_code()) {
event.Set("differrorcode", diff_error_code());
}
if (diff_extra_code1()) {
event.Set("diffextracode1", diff_extra_code1());
}
if (!previous_fp().empty()) {
event.Set("previousfp", previous_fp());
}
if (!next_fp().empty()) {
event.Set("nextfp", next_fp());
}
if (diff_error_code())
event.SetKey("differrorcode", base::Value(diff_error_code()));
if (diff_extra_code1())
event.SetKey("diffextracode1", base::Value(diff_extra_code1()));
if (!previous_fp().empty())
event.SetKey("previousfp", base::Value(previous_fp()));
if (!next_fp().empty())
event.SetKey("nextfp", base::Value(next_fp()));
DCHECK(previous_version().IsValid());
event.SetKey("previousversion", base::Value(previous_version().GetString()));
if (next_version().IsValid())
event.SetKey("nextversion", base::Value(next_version().GetString()));
event.Set("previousversion", previous_version().GetString());
if (next_version().IsValid()) {
event.Set("nextversion", next_version().GetString());
}
return event;
}

base::Value Component::MakeEventDownloadMetrics(
base::Value::Dict Component::MakeEventDownloadMetrics(
const CrxDownloader::DownloadMetrics& dm) const {
base::Value event(base::Value::Type::DICT);
event.SetKey("eventtype", base::Value(14));
event.SetKey("eventresult", base::Value(static_cast<int>(dm.error == 0)));
event.SetKey("downloader", base::Value(DownloaderToString(dm.downloader)));
if (dm.error)
event.SetKey("errorcode", base::Value(dm.error));
event.SetKey("url", base::Value(dm.url.spec()));
base::Value::Dict event;
event.Set("eventtype", 14);
event.Set("eventresult", static_cast<int>(dm.error == 0));
event.Set("downloader", DownloaderToString(dm.downloader));
if (dm.error) {
event.Set("errorcode", dm.error);
}
event.Set("url", dm.url.spec());

// -1 means that the byte counts are not known.
if (dm.total_bytes != -1 && dm.total_bytes < kProtocolMaxInt)
event.SetKey("total", base::Value(static_cast<double>(dm.total_bytes)));
if (dm.total_bytes != -1 && dm.total_bytes < kProtocolMaxInt) {
event.Set("total", static_cast<double>(dm.total_bytes));
}
if (dm.downloaded_bytes != -1 && dm.total_bytes < kProtocolMaxInt) {
event.SetKey("downloaded",
base::Value(static_cast<double>(dm.downloaded_bytes)));
event.Set("downloaded", static_cast<double>(dm.downloaded_bytes));
}
if (dm.download_time_ms && dm.total_bytes < kProtocolMaxInt) {
event.SetKey("download_time_ms",
base::Value(static_cast<double>(dm.download_time_ms)));
event.Set("download_time_ms", static_cast<double>(dm.download_time_ms));
}
DCHECK(previous_version().IsValid());
event.SetKey("previousversion", base::Value(previous_version().GetString()));
if (next_version().IsValid())
event.SetKey("nextversion", base::Value(next_version().GetString()));
event.Set("previousversion", previous_version().GetString());
if (next_version().IsValid()) {
event.Set("nextversion", next_version().GetString());
}
return event;
}

base::Value Component::MakeEventUninstalled() const {
base::Value::Dict Component::MakeEventUninstalled() const {
DCHECK(state() == ComponentState::kUninstalled);
base::Value event(base::Value::Type::DICT);
event.SetKey("eventtype", base::Value(4));
event.SetKey("eventresult", base::Value(1));
if (extra_code1())
event.SetKey("extracode1", base::Value(extra_code1()));
base::Value::Dict event;
event.Set("eventtype", 4);
event.Set("eventresult", 1);
if (extra_code1()) {
event.Set("extracode1", extra_code1());
}
DCHECK(previous_version().IsValid());
event.SetKey("previousversion", base::Value(previous_version().GetString()));
event.Set("previousversion", previous_version().GetString());
DCHECK(next_version().IsValid());
event.SetKey("nextversion", base::Value(next_version().GetString()));
event.Set("nextversion", next_version().GetString());
return event;
}

base::Value Component::MakeEventRegistration() const {
base::Value::Dict Component::MakeEventRegistration() const {
DCHECK(state() == ComponentState::kRegistration);
base::Value event(base::Value::Type::DICT);
event.SetKey("eventtype", base::Value(2));
event.SetKey("eventresult", base::Value(1));
if (error_code())
event.SetKey("errorcode", base::Value(error_code()));
if (extra_code1())
event.SetKey("extracode1", base::Value(extra_code1()));
base::Value::Dict event;
event.Set("eventtype", 2);
event.Set("eventresult", 1);
if (error_code()) {
event.Set("errorcode", error_code());
}
if (extra_code1()) {
event.Set("extracode1", extra_code1());
}
DCHECK(next_version().IsValid());
event.SetKey("nextversion", base::Value(next_version().GetString()));
event.Set("nextversion", next_version().GetString());
return event;
}

base::Value Component::MakeEventActionRun(bool succeeded,
int error_code,
int extra_code1) const {
base::Value event(base::Value::Type::DICT);
event.SetKey("eventtype", base::Value(42));
event.SetKey("eventresult", base::Value(static_cast<int>(succeeded)));
if (error_code)
event.SetKey("errorcode", base::Value(error_code));
if (extra_code1)
event.SetKey("extracode1", base::Value(extra_code1));
base::Value::Dict Component::MakeEventActionRun(bool succeeded,
int error_code,
int extra_code1) const {
base::Value::Dict event;
event.Set("eventtype", 42);
event.Set("eventresult", static_cast<int>(succeeded));
if (error_code) {
event.Set("errorcode", error_code);
}
if (extra_code1) {
event.Set("extracode1", extra_code1);
}
return event;
}

std::vector<base::Value> Component::GetEvents() const {
std::vector<base::Value> events;
std::vector<base::Value::Dict> Component::GetEvents() const {
std::vector<base::Value::Dict> events;
for (const auto& event : events_)
events.push_back(event.Clone());
return events;
Expand Down
27 changes: 12 additions & 15 deletions components/update_client/component.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,14 @@
#include "base/memory/ref_counted.h"
#include "base/sequence_checker.h"
#include "base/time/time.h"
#include "base/values.h"
#include "base/version.h"
#include "components/update_client/crx_downloader.h"
#include "components/update_client/protocol_parser.h"
#include "components/update_client/update_client.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"

namespace base {
class Value;
} // namespace base

namespace update_client {

class ActionRunner;
Expand Down Expand Up @@ -126,10 +123,10 @@ class Component {

std::string session_id() const;

const std::vector<base::Value>& events() const { return events_; }
const std::vector<base::Value::Dict>& events() const { return events_; }

// Returns a clone of the component events.
std::vector<base::Value> GetEvents() const;
std::vector<base::Value::Dict> GetEvents() const;

private:
friend class MockPingManagerImpl;
Expand Down Expand Up @@ -384,7 +381,7 @@ class Component {
// by a downloader which can do bandwidth throttling on the client side.
bool CanDoBackgroundDownload() const;

void AppendEvent(base::Value event);
void AppendEvent(base::Value::Dict event);

// Changes the component state and notifies the caller of the |Handle|
// function that the handling of this component state is complete.
Expand All @@ -400,14 +397,14 @@ class Component {

// These functions return a specific event. Each data member of the event is
// represented as a key-value pair in a dictionary value.
base::Value MakeEventUpdateComplete() const;
base::Value MakeEventDownloadMetrics(
base::Value::Dict MakeEventUpdateComplete() const;
base::Value::Dict MakeEventDownloadMetrics(
const CrxDownloader::DownloadMetrics& download_metrics) const;
base::Value MakeEventUninstalled() const;
base::Value MakeEventRegistration() const;
base::Value MakeEventActionRun(bool succeeded,
int error_code,
int extra_code1) const;
base::Value::Dict MakeEventUninstalled() const;
base::Value::Dict MakeEventRegistration() const;
base::Value::Dict MakeEventActionRun(bool succeeded,
int error_code,
int extra_code1) const;

std::unique_ptr<CrxInstaller::InstallParams> install_params() const;

Expand Down Expand Up @@ -483,7 +480,7 @@ class Component {
absl::optional<CrxInstaller::InstallParams> install_params_;

// Contains the events which are therefore serialized in the requests.
std::vector<base::Value> events_;
std::vector<base::Value::Dict> events_;

CallbackHandleComplete callback_handle_complete_;
std::unique_ptr<State> state_;
Expand Down

0 comments on commit 0b357d9

Please sign in to comment.