Skip to content

Commit

Permalink
Migrate reporting::StatusOr as an alias to base::expected<T, Status>
Browse files Browse the repository at this point in the history
Also renamed the status macro `RETURN_IF_ERROR` to
`RETURN_IF_ERROR_STATUS` to distinguish from the macro `RETURN_IF_ERROR`
on `StatusOr`.

   `unexpected` to some `Status` instances.

Low-Coverage-Reason: TRIVIAL_CHANGE Changes in lowly coveraged files add
Bug: b:300464285
Change-Id: Ieaf34ac208b4675f4c8932da084fc3c7de904943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4981520
Commit-Queue: Hong Xu <xuhong@google.com>
Reviewed-by: Leonid Baraz <lbaraz@chromium.org>
Reviewed-by: Hong Xu <xuhong@google.com>
Cr-Commit-Position: refs/heads/main@{#1217278}
  • Loading branch information
xuhdev authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent e29ac69 commit a7c9ad8
Show file tree
Hide file tree
Showing 42 changed files with 647 additions and 1,136 deletions.
18 changes: 11 additions & 7 deletions chrome/browser/policy/messaging_layer/public/report_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/threading/sequence_bound.h"
#include "base/types/expected.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/policy/messaging_layer/storage_selector/storage_selector.h"
Expand Down Expand Up @@ -106,10 +107,11 @@ void ReportingClient::ConfigureReportQueue(
// found
if (!dm_token_retriever) {
std::move(completion_cb)
.Run(Status(error::INTERNAL,
base::StrCat({"No DM token retriever found for event type=",
base::NumberToString(static_cast<int>(
configuration->event_type()))})));
.Run(base::unexpected(
Status(error::INTERNAL,
base::StrCat({"No DM token retriever found for event type=",
base::NumberToString(static_cast<int>(
configuration->event_type()))}))));
return;
}

Expand All @@ -121,7 +123,8 @@ void ReportingClient::ConfigureReportQueue(
// Trigger completion callback with error if there was an error
// retrieving DM token.
if (!dm_token_result.has_value()) {
std::move(completion_cb).Run(dm_token_result.error());
std::move(completion_cb)
.Run(base::unexpected(dm_token_result.error()));
return;
}

Expand All @@ -132,7 +135,7 @@ void ReportingClient::ConfigureReportQueue(

// Fail on error
if (!config_result.ok()) {
std::move(completion_cb).Run(config_result);
std::move(completion_cb).Run(base::unexpected(config_result));
return;
}

Expand Down Expand Up @@ -317,7 +320,8 @@ void ReportingClient::DeliverAsyncStartUploader(
if (!StorageSelector::is_uploader_required() ||
StorageSelector::is_use_missive()) {
std::move(start_uploader_cb)
.Run(Status(error::UNAVAILABLE, "Uploader not available"));
.Run(base::unexpected(
Status(error::UNAVAILABLE, "Uploader not available")));
return;
}
instance->upload_provider_ =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/types/expected.h"
#include "base/values.h"
#include "chrome/browser/enterprise/browser_management/management_service_factory.h"
#include "chrome/browser/policy/messaging_layer/public/report_client_test_util.h"
Expand Down Expand Up @@ -162,7 +163,7 @@ class ReportClientTest : public ::testing::TestWithParam<bool> {
// Let everything ongoing to finish.
task_environment_.RunUntilIdle();

return std::move(report_queue_result);
return report_queue_result;
}

std::unique_ptr<ReportQueue, base::OnTaskRunnerDeleter>
Expand Down Expand Up @@ -314,8 +315,8 @@ TEST_P(ReportClientTest, CreatesReportQueueGivenEventType) {
// Tests that a ReportQueue cannot be created when there is DM token retrieval
// failure
TEST_P(ReportClientTest, CreateReportQueueWhenDMTokenRetrievalFailure) {
MockDMTokenRetrieverWithResult(
Status(error::INTERNAL, "Simulated DM token retrieval failure"));
MockDMTokenRetrieverWithResult(base::unexpected(
Status(error::INTERNAL, "Simulated DM token retrieval failure")));
auto report_queue_result = CreateQueue();
ASSERT_FALSE(report_queue_result.has_value());
EXPECT_EQ(report_queue_result.error().error_code(), error::INTERNAL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/files/file_path.h"
#include "base/memory/scoped_refptr.h"
#include "base/path_service.h"
#include "base/types/expected.h"
#include "chrome/browser/policy/messaging_layer/upload/upload_client.h"
#include "components/reporting/compression/compression_module.h"
#include "components/reporting/encryption/encryption_module.h"
Expand Down Expand Up @@ -53,7 +54,7 @@ void StorageSelector::CreateLocalStorageModule(
StatusOr<scoped_refptr<StorageModuleInterface>>)> cb,
StatusOr<scoped_refptr<StorageModule>> result) {
if (!result.has_value()) {
std::move(cb).Run(result.error());
std::move(cb).Run(base::unexpected(result.error()));
return;
}
std::move(cb).Run(result.value());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/logging.h"
#include "base/memory/scoped_refptr.h"
#include "base/task/bind_post_task.h"
#include "base/types/expected.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chromeos/dbus/missive/missive_client.h"
Expand Down Expand Up @@ -40,9 +41,9 @@ void StorageSelector::CreateMissiveStorageModule(
cb) {
MissiveClient* const missive_client = MissiveClient::Get();
if (!missive_client) {
std::move(cb).Run(Status(
std::move(cb).Run(base::unexpected(Status(
error::FAILED_PRECONDITION,
"Missive Client unavailable, probably has not been initialized"));
"Missive Client unavailable, probably has not been initialized")));
return;
}
// Refer to the storage module.
Expand All @@ -58,8 +59,9 @@ void StorageSelector::CreateMissiveStorageModule(
auto missive_storage_module =
MissiveStorageModule::Create(std::move(missive_storage_module_delegate));
if (!missive_storage_module) {
std::move(cb).Run(Status(error::FAILED_PRECONDITION,
"Missive Storage Module failed to create"));
std::move(cb).Run(
base::unexpected(Status(error::FAILED_PRECONDITION,
"Missive Storage Module failed to create")));
return;
}
LOG(WARNING) << "Store reporting data by a Missive daemon";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/task/bind_post_task.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/task_runner.h"
#include "base/types/expected.h"
#include "chrome/browser/policy/messaging_layer/upload/record_handler_impl.h"
#include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
#include "components/reporting/proto/synced/record.pb.h"
Expand Down Expand Up @@ -54,20 +55,21 @@ DmServerUploader::~DmServerUploader() = default;
void DmServerUploader::OnStart() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (handler_ == nullptr) {
Finalize(Status(error::INVALID_ARGUMENT, "handler was null"));
Finalize(
base::unexpected(Status(error::INVALID_ARGUMENT, "handler was null")));
return;
}
// Early exit if we don't have any records and do not need encryption key.
if (encrypted_records_.empty() && !need_encryption_key_) {
Finalize(
Status(error::INVALID_ARGUMENT, "No records received for upload."));
Finalize(base::unexpected(
Status(error::INVALID_ARGUMENT, "No records received for upload.")));
return;
}

if (!encrypted_records_.empty()) {
const auto process_status = ProcessRecords();
if (!process_status.ok()) {
Finalize(process_status);
Finalize(base::unexpected(process_status));
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/task/task_runner.h"
#include "base/task/thread_pool.h"
#include "base/test/task_environment.h"
#include "base/types/expected.h"
#include "components/reporting/resources/resource_manager.h"
#include "components/reporting/util/status.h"
#include "components/reporting/util/test_support_callbacks.h"
Expand Down Expand Up @@ -221,8 +222,8 @@ TEST_P(DmServerUploaderTest, ReportsFailureToProcess) {

EXPECT_CALL(*handler_, HandleRecords_(_, _, _, _, _, _))
.WillOnce(WithArgs<4>(Invoke([](CompletionCallback callback) {
std::move(callback).Run(
Status(error::FAILED_PRECONDITION, "Fail for test"));
std::move(callback).Run(base::unexpected(
Status(error::FAILED_PRECONDITION, "Fail for test")));
})));

StrictMock<TestSuccessfulUpload> successful_upload;
Expand Down Expand Up @@ -308,7 +309,8 @@ TEST_P(DmServerFailureTest, ReportsFailureToUpload) {

EXPECT_CALL(*handler_, HandleRecords_(_, _, _, _, _, _))
.WillOnce(WithArgs<4>(Invoke([error_code](CompletionCallback callback) {
std::move(callback).Run(Status(error_code, "Failing for test"));
std::move(callback).Run(
base::unexpected(Status(error_code, "Failing for test")));
})));

StrictMock<TestSuccessfulUpload> successful_upload;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/time/time.h"
#include "base/types/expected.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/policy/chrome_browser_policy_connector.h"
Expand Down Expand Up @@ -53,8 +54,9 @@ void EncryptedReportingClient::UploadReport(
policy::DeviceManagementService* const device_management_service =
delegate_->device_management_service();
if (!device_management_service) {
std::move(callback).Run(Status(
error::NOT_FOUND, "Device management service required, but not found"));
std::move(callback).Run(base::unexpected(
Status(error::NOT_FOUND,
"Device management service required, but not found")));
return;
}

Expand Down Expand Up @@ -106,19 +108,20 @@ void EncryptedReportingClient::OnReportUploadCompleted(
request_jobs_.erase(job);
}
if (response_code == ::policy::DeviceManagementService::kTooManyRequests) {
std::move(callback).Run(
Status(error::OUT_OF_RANGE, "Too many upload requests"));
std::move(callback).Run(base::unexpected(
Status(error::OUT_OF_RANGE, "Too many upload requests")));
return;
}
if (response_code != ::policy::DeviceManagementService::kSuccess) {
std::move(callback).Run(Status(
error::DATA_LOSS, base::StrCat({"Response code: ",
base::NumberToString(response_code)})));
std::move(callback).Run(base::unexpected(
Status(error::DATA_LOSS,
base::StrCat(
{"Response code: ", base::NumberToString(response_code)}))));
return;
}
if (!response.has_value()) {
std::move(callback).Run(
Status(error::DATA_LOSS, "Success response is empty"));
std::move(callback).Run(base::unexpected(
Status(error::DATA_LOSS, "Success response is empty")));
return;
}
std::move(callback).Run(std::move(response.value()));
Expand Down

0 comments on commit a7c9ad8

Please sign in to comment.