Commit a8e2e61c authored by Muhammad Hasan Khan's avatar Muhammad Hasan Khan Committed by Chromium LUCI CQ

arc: code cleanup in arc_session_manager

Move out block of code that decides the error message.

BUG=b:166322619
TEST=CQ

Change-Id: I0d97a9972a34d2696e8027c6216c3249688ee824
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574531Reviewed-by: default avatarHidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Muhammad Hasan Khan <mhasank@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834727}
parent e3ab7e14
......@@ -222,31 +222,30 @@ void UpdateSecondaryAccountSilentAuthCodeUMA(OptInSilentAuthCode state) {
ProvisioningStatus GetProvisioningStatus(
const ArcProvisioningResult& provisioning_result) {
if (provisioning_result.is_stopped())
if (provisioning_result.stop_reason())
return ProvisioningStatus::ARC_STOPPED;
if (provisioning_result.is_timedout())
return ProvisioningStatus::CHROME_PROVISIONING_TIMEOUT;
const mojom::ArcSignInResult* result = provisioning_result.sign_in_result();
if (result->is_success())
if (provisioning_result.is_success())
return ProvisioningStatus::SUCCESS;
if (result->get_error()->is_cloud_provision_flow_error())
if (provisioning_result.cloud_provision_flow_error())
return ProvisioningStatus::CLOUD_PROVISION_FLOW_ERROR;
if (result->get_error()->is_check_in_error())
if (provisioning_result.gms_check_in_error())
return ProvisioningStatus::GMS_CHECK_IN_ERROR;
if (result->get_error()->is_sign_in_error())
if (provisioning_result.gms_sign_in_error())
return ProvisioningStatus::GMS_SIGN_IN_ERROR;
if (result->get_error()->is_general_error()) {
if (provisioning_result.general_error()) {
#define MAP_GENERAL_ERROR(name) \
case mojom::GeneralSignInError::name: \
return ProvisioningStatus::name
switch (result->get_error()->get_general_error()) {
switch (provisioning_result.general_error().value()) {
MAP_GENERAL_ERROR(UNKNOWN_ERROR);
MAP_GENERAL_ERROR(MOJO_VERSION_MISMATCH);
MAP_GENERAL_ERROR(GENERIC_PROVISIONING_TIMEOUT);
......
......@@ -19,40 +19,53 @@ ArcProvisioningResult::ArcProvisioningResult(ArcProvisioningResult&& other) =
default;
ArcProvisioningResult::~ArcProvisioningResult() = default;
bool ArcProvisioningResult::has_sign_in_result() const {
return absl::holds_alternative<mojom::ArcSignInResultPtr>(result_);
base::Optional<mojom::GMSSignInError> ArcProvisioningResult::gms_sign_in_error()
const {
if (!sign_in_error() || !sign_in_error()->is_sign_in_error())
return base::nullopt;
return sign_in_error()->get_sign_in_error();
}
const mojom::ArcSignInResult* ArcProvisioningResult::sign_in_result() const {
DCHECK(has_sign_in_result());
return absl::get<mojom::ArcSignInResultPtr>(result_).get();
base::Optional<mojom::GMSCheckInError>
ArcProvisioningResult::gms_check_in_error() const {
if (!sign_in_error() || !sign_in_error()->is_check_in_error())
return base::nullopt;
return sign_in_error()->get_check_in_error();
}
bool ArcProvisioningResult::has_sign_in_error() const {
return has_sign_in_result() && sign_in_result()->is_error();
base::Optional<mojom::CloudProvisionFlowError>
ArcProvisioningResult::cloud_provision_flow_error() const {
if (!sign_in_error() || !sign_in_error()->is_cloud_provision_flow_error())
return base::nullopt;
return sign_in_error()->get_cloud_provision_flow_error();
}
const mojom::ArcSignInError* ArcProvisioningResult::sign_in_error() const {
DCHECK(has_sign_in_error());
if (!sign_in_result() || !sign_in_result()->is_error())
return nullptr;
return sign_in_result()->get_error().get();
}
bool ArcProvisioningResult::is_success() const {
return has_sign_in_result() && sign_in_result()->is_success();
}
base::Optional<mojom::GeneralSignInError> ArcProvisioningResult::general_error()
const {
if (!sign_in_error() || !sign_in_error()->is_general_error())
return base::nullopt;
bool ArcProvisioningResult::has_general_error(
mojom::GeneralSignInError error) const {
return has_sign_in_error() && sign_in_error()->is_general_error() &&
sign_in_error()->get_general_error() == error;
return sign_in_error()->get_general_error();
}
bool ArcProvisioningResult::is_stopped() const {
return absl::holds_alternative<ArcStopReason>(result_);
bool ArcProvisioningResult::is_success() const {
return sign_in_result() && sign_in_result()->is_success();
}
ArcStopReason ArcProvisioningResult::stop_reason() const {
DCHECK(is_stopped());
base::Optional<ArcStopReason> ArcProvisioningResult::stop_reason() const {
if (!absl::holds_alternative<ArcStopReason>(result_))
return base::nullopt;
return absl::get<ArcStopReason>(result_);
}
......@@ -60,6 +73,13 @@ bool ArcProvisioningResult::is_timedout() const {
return absl::holds_alternative<ChromeProvisioningTimeout>(result_);
}
const mojom::ArcSignInResult* ArcProvisioningResult::sign_in_result() const {
if (!absl::holds_alternative<mojom::ArcSignInResultPtr>(result_))
return nullptr;
return absl::get<mojom::ArcSignInResultPtr>(result_).get();
}
std::ostream& operator<<(std::ostream& os,
const ArcProvisioningResult& result) {
return os << GetProvisioningStatus(result);
......
......@@ -7,6 +7,7 @@
#include <ostream>
#include "base/optional.h"
#include "components/arc/mojom/auth.mojom.h"
#include "components/arc/session/arc_stop_reason.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
......@@ -27,34 +28,35 @@ class ArcProvisioningResult {
ArcProvisioningResult(ArcProvisioningResult&& other);
~ArcProvisioningResult();
// Returns true if signin_result from ARC is present.
bool has_sign_in_result() const;
// Returns gms sign-in error if sign-in result has it.
base::Optional<mojom::GMSSignInError> gms_sign_in_error() const;
// Returns the result of provisioning from inside ARC.
const mojom::ArcSignInResult* sign_in_result() const;
// Returns gms check-in error if sign-in result has it.
base::Optional<mojom::GMSCheckInError> gms_check_in_error() const;
// Returns true if signin_result is present with an error.
bool has_sign_in_error() const;
// Returns cloud provision flow error if sign-in result has it.
base::Optional<mojom::CloudProvisionFlowError> cloud_provision_flow_error()
const;
// Returns the error of signin_result coming from ARC.
const mojom::ArcSignInError* sign_in_error() const;
// Returns true if result has given general sign-in error.
bool has_general_error(mojom::GeneralSignInError error) const;
// Returns general sign-in error if result has it.
base::Optional<mojom::GeneralSignInError> general_error() const;
// Returns true if provisioning was successful.
bool is_success() const;
// Returns true if ARC provisioning was stopped pre-maturely.
bool is_stopped() const;
// Returns the reason for ARC stopped event.
ArcStopReason stop_reason() const;
// Returns the reason for ARC stopped event if it exists.
base::Optional<ArcStopReason> stop_reason() const;
// Returns true if ARC provisioning timed out in Chrome.
bool is_timedout() const;
private:
// Returns the result of provisioning from inside ARC.
const mojom::ArcSignInResult* sign_in_result() const;
absl::variant<mojom::ArcSignInResultPtr,
ArcStopReason,
ChromeProvisioningTimeout>
......
......@@ -9,23 +9,55 @@
namespace arc {
TEST(ArcProvisioningResultTest, HasSignInResult) {
TEST(ArcProvisioningResultTest, SignInError) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_FALSE(result1.has_sign_in_result());
EXPECT_FALSE(result1.sign_in_error());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewSuccess(
arc::mojom::ArcSignInSuccess::SUCCESS));
EXPECT_TRUE(result2.has_sign_in_result());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewGeneralError(
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR)));
EXPECT_TRUE(result2.sign_in_error());
}
TEST(ArcProvisioningResultTest, HasSignInError) {
TEST(ArcProvisioningResultTest, GmsSignInError) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_FALSE(result1.has_sign_in_error());
EXPECT_FALSE(result1.gms_sign_in_error());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewGeneralError(
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR)));
EXPECT_TRUE(result2.has_sign_in_error());
EXPECT_FALSE(result2.gms_sign_in_error());
ArcProvisioningResult result3(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewSignInError(
arc::mojom::GMSSignInError::GMS_SIGN_IN_TIMEOUT)));
EXPECT_EQ(result3.gms_sign_in_error(),
arc::mojom::GMSSignInError::GMS_SIGN_IN_TIMEOUT);
}
TEST(ArcProvisioningResultTest, GmsCheckInError) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_FALSE(result1.gms_check_in_error());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewCheckInError(
arc::mojom::GMSCheckInError::GMS_CHECK_IN_INTERNAL_ERROR)));
EXPECT_EQ(result2.gms_check_in_error(),
arc::mojom::GMSCheckInError::GMS_CHECK_IN_INTERNAL_ERROR);
}
TEST(ArcProvisioningResultTest, CloudProvisionFlowError) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_FALSE(result1.cloud_provision_flow_error());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewCloudProvisionFlowError(
arc::mojom::CloudProvisionFlowError::ERROR_ACCOUNT_NOT_ALLOWLISTED)));
EXPECT_EQ(result2.cloud_provision_flow_error(),
arc::mojom::CloudProvisionFlowError::ERROR_ACCOUNT_NOT_ALLOWLISTED);
}
TEST(ArcProvisioningResultTest, Success) {
......@@ -41,22 +73,22 @@ TEST(ArcProvisioningResultTest, Success) {
EXPECT_TRUE(result3.is_success());
}
TEST(ArcProvisioningResultTest, Stopped) {
TEST(ArcProvisioningResultTest, StopReason) {
ArcProvisioningResult result1(arc::mojom::ArcSignInResult::NewSuccess(
arc::mojom::ArcSignInSuccess::SUCCESS));
EXPECT_FALSE(result1.is_stopped());
EXPECT_FALSE(result1.stop_reason());
ArcProvisioningResult result2(ArcStopReason::CRASH);
EXPECT_TRUE(result2.is_stopped());
EXPECT_EQ(ArcStopReason::CRASH, result2.stop_reason());
ArcProvisioningResult result3(ArcStopReason::SHUTDOWN);
EXPECT_TRUE(result3.is_stopped());
EXPECT_EQ(ArcStopReason::SHUTDOWN, result3.stop_reason());
ArcProvisioningResult result4(ArcStopReason::GENERIC_BOOT_FAILURE);
EXPECT_TRUE(result4.is_stopped());
EXPECT_EQ(ArcStopReason::GENERIC_BOOT_FAILURE, result4.stop_reason());
ArcProvisioningResult result5(ArcStopReason::LOW_DISK_SPACE);
EXPECT_TRUE(result5.is_stopped());
EXPECT_EQ(ArcStopReason::LOW_DISK_SPACE, result5.stop_reason());
}
TEST(ArcProvisioningResultTest, Timedout) {
......@@ -67,30 +99,16 @@ TEST(ArcProvisioningResultTest, Timedout) {
EXPECT_TRUE(result2.is_timedout());
}
TEST(ArcProvisioningResultTest, HasGeneralError) {
TEST(ArcProvisioningResultTest, GeneralError) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_FALSE(result1.has_general_error(
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR));
EXPECT_FALSE(result1.general_error());
ArcProvisioningResult result2(arc::mojom::ArcSignInResult::NewError(
arc::mojom::ArcSignInError::NewGeneralError(
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR)));
EXPECT_TRUE(result2.has_general_error(
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR));
}
TEST(ArcProvisioningResultTest, StopReason) {
ArcProvisioningResult result1(ArcStopReason::CRASH);
EXPECT_EQ(ArcStopReason::CRASH, result1.stop_reason());
ArcProvisioningResult result2(ArcStopReason::SHUTDOWN);
EXPECT_EQ(ArcStopReason::SHUTDOWN, result2.stop_reason());
ArcProvisioningResult result3(ArcStopReason::GENERIC_BOOT_FAILURE);
EXPECT_EQ(ArcStopReason::GENERIC_BOOT_FAILURE, result3.stop_reason());
ArcProvisioningResult result4(ArcStopReason::LOW_DISK_SPACE);
EXPECT_EQ(ArcStopReason::LOW_DISK_SPACE, result4.stop_reason());
EXPECT_EQ(result2.general_error(),
arc::mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR);
}
} // namespace arc
......@@ -371,6 +371,47 @@ ArcSupportHost::Error GetCloudProvisionFlowError(
}
}
ArcSupportHost::Error GetSupportHostError(const ArcProvisioningResult& result) {
if (result.gms_sign_in_error() ==
mojom::GMSSignInError::GMS_SIGN_IN_NETWORK_ERROR) {
return ArcSupportHost::Error::SIGN_IN_NETWORK_ERROR;
}
if (result.gms_sign_in_error() ==
mojom::GMSSignInError::GMS_SIGN_IN_BAD_AUTHENTICATION) {
return ArcSupportHost::Error::SIGN_IN_BAD_AUTHENTICATION_ERROR;
}
if (result.gms_sign_in_error())
return ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR;
if (result.gms_check_in_error())
return ArcSupportHost::Error::SIGN_IN_GMS_NOT_AVAILABLE_ERROR;
if (result.cloud_provision_flow_error()) {
return GetCloudProvisionFlowError(
result.cloud_provision_flow_error().value());
}
if (result.general_error() ==
mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR) {
return ArcSupportHost::Error::SERVER_COMMUNICATION_ERROR;
}
if (result.general_error() ==
mojom::GeneralSignInError::NO_NETWORK_CONNECTION) {
return ArcSupportHost::Error::NETWORK_UNAVAILABLE_ERROR;
}
if (result.general_error() == mojom::GeneralSignInError::ARC_DISABLED)
return ArcSupportHost::Error::ANDROID_MANAGEMENT_REQUIRED_ERROR;
if (result.stop_reason() == ArcStopReason::LOW_DISK_SPACE)
return ArcSupportHost::Error::LOW_DISK_SPACE_ERROR;
return ArcSupportHost::Error::SIGN_IN_UNKNOWN_ERROR;
}
ArcSessionManager::ExpansionResult ExpandPropertyFilesAndReadSaltInternal(
const base::FilePath& source_path,
const base::FilePath& dest_path,
......@@ -578,9 +619,6 @@ void ArcSessionManager::OnProvisioningFinished(
return;
}
const mojom::ArcSignInError* sign_in_error =
result.has_sign_in_error() ? result.sign_in_error() : nullptr;
// Due asynchronous nature of stopping the ARC instance,
// OnProvisioningFinished may arrive after setting the |State::STOPPED| state
// and |State::Active| is not guaranteed to be set here.
......@@ -601,8 +639,8 @@ void ArcSessionManager::OnProvisioningFinished(
if (scoped_opt_in_tracker_ && !provisioning_successful)
scoped_opt_in_tracker_->TrackError();
if (result.has_general_error(
mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR)) {
if (result.general_error() ==
mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR) {
// TODO(poromov): Consider ARC PublicSession offline mode.
// Currently ARC session will be exited below, while the main user session
// will be kept alive without Android apps.
......@@ -626,15 +664,13 @@ void ArcSessionManager::OnProvisioningFinished(
provisioning_successful, profile_);
UpdateProvisioningStatusUMA(GetProvisioningStatus(result), profile_);
if (sign_in_error) {
if (sign_in_error->is_cloud_provision_flow_error()) {
UpdateCloudProvisionFlowErrorUMA(
sign_in_error->get_cloud_provision_flow_error(), profile_);
} else if (sign_in_error->is_sign_in_error()) {
UpdateGMSSignInErrorUMA(sign_in_error->get_sign_in_error(), profile_);
} else if (sign_in_error->is_check_in_error()) {
UpdateGMSCheckInErrorUMA(sign_in_error->get_check_in_error(), profile_);
}
if (result.gms_sign_in_error()) {
UpdateGMSSignInErrorUMA(result.gms_sign_in_error().value(), profile_);
} else if (result.gms_check_in_error()) {
UpdateGMSCheckInErrorUMA(result.gms_check_in_error().value(), profile_);
} else if (result.cloud_provision_flow_error()) {
UpdateCloudProvisionFlowErrorUMA(
result.cloud_provision_flow_error().value(), profile_);
}
if (!provisioning_successful)
......@@ -675,69 +711,38 @@ void ArcSessionManager::OnProvisioningFinished(
return;
}
ArcSupportHost::Error support_error;
VLOG(1) << "ARC provisioning failed: " << result << ".";
if (sign_in_error && sign_in_error->is_sign_in_error() &&
sign_in_error->get_sign_in_error() ==
mojom::GMSSignInError::GMS_SIGN_IN_NETWORK_ERROR) {
support_error = ArcSupportHost::Error::SIGN_IN_NETWORK_ERROR;
} else if (sign_in_error && sign_in_error->is_sign_in_error() &&
sign_in_error->get_sign_in_error() ==
mojom::GMSSignInError::GMS_SIGN_IN_BAD_AUTHENTICATION) {
support_error = ArcSupportHost::Error::SIGN_IN_BAD_AUTHENTICATION_ERROR;
} else if (sign_in_error && sign_in_error->is_sign_in_error()) {
support_error = ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR;
} else if (sign_in_error && sign_in_error->is_check_in_error()) {
support_error = ArcSupportHost::Error::SIGN_IN_GMS_NOT_AVAILABLE_ERROR;
} else if (sign_in_error && sign_in_error->is_cloud_provision_flow_error()) {
support_error = GetCloudProvisionFlowError(
sign_in_error->get_cloud_provision_flow_error());
} else if (result.has_general_error(mojom::GeneralSignInError::
CHROME_SERVER_COMMUNICATION_ERROR)) {
support_error = ArcSupportHost::Error::SERVER_COMMUNICATION_ERROR;
} else if (result.has_general_error(
mojom::GeneralSignInError::NO_NETWORK_CONNECTION)) {
support_error = ArcSupportHost::Error::NETWORK_UNAVAILABLE_ERROR;
} else if (result.has_general_error(
mojom::GeneralSignInError::ARC_DISABLED)) {
support_error = ArcSupportHost::Error::ANDROID_MANAGEMENT_REQUIRED_ERROR;
} else if (result.is_stopped()) {
support_error = result.stop_reason() == ArcStopReason::LOW_DISK_SPACE
? ArcSupportHost::Error::LOW_DISK_SPACE_ERROR
: ArcSupportHost::Error::SIGN_IN_UNKNOWN_ERROR;
} else {
support_error = ArcSupportHost::Error::SIGN_IN_UNKNOWN_ERROR;
}
// When ARC provisioning fails due to Chrome failing to talk to server, we
// don't need to keep the ARC session running as the logs necessary to
// investigate are already present. ARC session will not provide any useful
// context.
if (result.is_stopped() ||
result.has_general_error(
mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR)) {
if (result.stop_reason() ||
result.general_error() ==
mojom::GeneralSignInError::CHROME_SERVER_COMMUNICATION_ERROR) {
if (profile_->GetPrefs()->HasPrefPath(prefs::kArcSignedIn))
profile_->GetPrefs()->SetBoolean(prefs::kArcSignedIn, false);
VLOG(1) << "Stopping ARC due to provisioning failure";
ShutdownSession();
}
if ((sign_in_error && sign_in_error->is_cloud_provision_flow_error()) ||
if (result.cloud_provision_flow_error() ||
// OVERALL_SIGN_IN_TIMEOUT might be an indication that ARC believes it is
// fully setup, but Chrome does not.
result.is_timedout() ||
// Just to be safe, remove data if we don't know the cause.
result.has_general_error(mojom::GeneralSignInError::UNKNOWN_ERROR)) {
result.general_error() == mojom::GeneralSignInError::UNKNOWN_ERROR) {
VLOG(1) << "ARC provisioning failed permanently. Removing user data";
RequestArcDataRemoval();
}
base::Optional<int> error_code;
ArcSupportHost::Error support_error = GetSupportHostError(result);
if (support_error == ArcSupportHost::Error::SIGN_IN_UNKNOWN_ERROR) {
error_code = static_cast<std::underlying_type_t<ProvisioningStatus>>(
GetProvisioningStatus(result));
} else if (sign_in_error) {
error_code = GetSignInErrorCode(sign_in_error);
} else if (result.sign_in_error()) {
error_code = GetSignInErrorCode(result.sign_in_error());
}
ShowArcSupportHostError({support_error, error_code} /* error_info */,
true /* should_show_send_feedback */);
......
......@@ -213,7 +213,7 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
// On provisioning completion (regardless of whether successfully done or
// not), this is called with its status. On success, is_success() of
// |result| returns true, otherwise ArcSignInResult can be retrieved from
// get() if sign-in result came from ARC or is_stopped()
// get() if sign-in result came from ARC or stop_reason()
// will indicate that ARC stopped prematurely and provisioning could
// not finish successfully. is_timedout() indicates that operation timed
// out.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment