Skip to content

Commit

Permalink
[M110] Revert "Refactor GaiaScreen to exit by code"
Browse files Browse the repository at this point in the history
This reverts commit a0bf305.

Reason for revert: Causes flaky login issues in multiple Tast
tests as the Oobe.loginForTesting() call sometimes fails to
complete. Example tests are rollback.EnterpriseRollbackInPlace
and login.SignInWithLotsOfUsers.20_users.

This CL merges https://crrev.com/c/4128041 into M110. The
revert in M111 included merge conflicts because of CLs
introduced in M111. Instead of cherry picking the original CL,
this CL is a clean revert as those conflicts do no exist in
this branch.

Original change's description:
> Refactor GaiaScreen to exit by code
>
> Move some logic to GaiaScreen from handler, save UserContext to
> WizardContext and exit screen by using exit code.
>
> Bug: b:257073746
> Change-Id: I184ebef8a48dc88e79f053bbc73a54f5f25fb813
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4063554
> Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
> Commit-Queue: Yunke Zhou <yunkez@google.com>
> Reviewed-by: Danila Kuzmin <dkuzmin@google.com>
> Cr-Commit-Position: refs/heads/main@{#1081914}

Bug: b:257073746, b:263947623
Change-Id: I39685009e44f73edad1e52086cd73b8b2fa97091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4131727
Reviewed-by: Renato Silva <rrsilva@google.com>
Auto-Submit: Cristina Guerrero <crisguerrero@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#176}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Cristina Guerrero authored and Chromium LUCI CQ committed Jan 9, 2023
1 parent 96e2265 commit 2e4655e
Show file tree
Hide file tree
Showing 10 changed files with 411 additions and 496 deletions.
382 changes: 3 additions & 379 deletions chrome/browser/ash/login/screens/gaia_screen.cc

Large diffs are not rendered by default.

63 changes: 0 additions & 63 deletions chrome/browser/ash/login/screens/gaia_screen.h
Expand Up @@ -13,12 +13,8 @@
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "base/timer/elapsed_timer.h"
#include "base/values.h"
#include "chrome/browser/ash/login/login_client_cert_usage_observer.h"
#include "chrome/browser/ash/login/screens/base_screen.h"
#include "chrome/browser/ui/webui/ash/login/gaia_screen_handler.h"
#include "chrome/browser/ui/webui/ash/login/online_login_helper.h"
#include "components/account_id/account_id.h"

namespace ash {
Expand All @@ -37,7 +33,6 @@ class GaiaScreen : public BaseScreen, public ScreenBacklightObserver {
CANCEL,
ENTERPRISE_ENROLL,
START_CONSUMER_KIOSK,
LOGIN_SUCCESS,
};

static std::string GetResultString(Result result);
Expand Down Expand Up @@ -74,70 +69,12 @@ class GaiaScreen : public BaseScreen, public ScreenBacklightObserver {
void OnUserAction(const base::Value::List& args) override;
bool HandleAccelerator(LoginAcceleratorAction action) override;

// Handle user actions.
void HandleCompleteAuthentication(
const std::string& gaia_id,
const std::string& email,
const std::string& password,
const base::Value::List& scraped_saml_passwords_value,
bool using_saml,
const base::Value::List& services_list,
bool services_provided,
const base::Value::Dict& password_attributes,
const base::Value::Dict& sync_trusted_vault_keys);
// TODO(yunkez): This is only used in `Oobe.loginForTesting` in tast tests. We
// could remove this or use HandleCompleteAuthentication instead.
void HandleCompleteLoginForTesting(const std::string& gaia_id,
const std::string& typed_email,
const std::string& password,
bool using_saml);
void HandleIdentifierEntered(const std::string& account_identifier);
void HandlePasswordEntered();
void HandleGaiaLoaded();

// Handles SAML/GAIA login flow metrics
// is_third_party_idp == false means GAIA-based authentication
void HandleUsingSAMLAPI(bool is_third_party_idp);

void LoadGaiaAsync(const AccountId& account_id);

// Updates the member variable and UMA histogram indicating whether the
// Chrome Credentials Passing API was used during SAML login.
void OnSamlPrincipalsAPIUsed(bool is_third_party_idp, bool is_api_used);

void RecordScrapedPasswordCount(int password_count);
bool IsSamlUserPasswordless();

void OnCookieWaitTimeout();

void OnCompleteLogin(std::unique_ptr<UserContext> user_context);
void SAMLConfirmPassword(::login::StringList scraped_saml_passwords,
std::unique_ptr<UserContext> user_context);

// This flag is set when user authenticated using the Chrome Credentials
// Passing API (the login could happen via SAML or, with the current
// server-side implementation, via Gaia).
bool using_saml_api_ = false;

std::unique_ptr<LoginClientCertUsageObserver>
extension_provided_client_cert_usage_observer_;

std::unique_ptr<OnlineLoginHelper> online_login_helper_;

GaiaView::GaiaLoginVariant login_request_variant_ =
GaiaView::GaiaLoginVariant::kUnknown;

// Used to record amount of time user needed for successful online login.
std::unique_ptr<base::ElapsedTimer> elapsed_timer_;

base::WeakPtr<TView> view_;

ScreenExitCallback exit_callback_;

base::ScopedObservation<BacklightsForcedOffSetter, ScreenBacklightObserver>
backlights_forced_off_observation_{this};

base::WeakPtrFactory<GaiaScreen> weak_factory_{this};
};

} // namespace ash
Expand Down
17 changes: 5 additions & 12 deletions chrome/browser/ash/login/wizard_controller.cc
Expand Up @@ -1195,10 +1195,6 @@ void WizardController::OnGaiaScreenExit(GaiaScreen::Result result) {
case GaiaScreen::Result::START_CONSUMER_KIOSK:
LoginDisplayHost::default_host()->AttemptShowEnableConsumerKioskScreen();
break;
case GaiaScreen::Result::LOGIN_SUCCESS:
LoginDisplayHost::default_host()->CompleteLogin(
*wizard_context_->extra_factors_auth_session);
break;
}
}

Expand Down Expand Up @@ -1419,15 +1415,12 @@ void WizardController::SkipToLoginForTesting() {
return;
wizard_context_->skip_to_login_for_tests = true;

if (LoginDisplayHost::default_host()->HasUserPods()) {
AdvanceToSigninScreen();
} else {
if (!features::IsOobeConsolidatedConsentEnabled())
StartupUtils::MarkEulaAccepted();

PerformPostNetworkScreenActions();
OnDeviceDisabledChecked(false /* device_disabled */);
if (!features::IsOobeConsolidatedConsentEnabled()) {
StartupUtils::MarkEulaAccepted();
}

PerformPostNetworkScreenActions();
OnDeviceDisabledChecked(false /* device_disabled */);
}

void WizardController::OnScreenExit(OobeScreenId screen,
Expand Down
Expand Up @@ -326,7 +326,7 @@ class GaiaDialog extends GaiaDialogBase {
chrome.send(
'metricsHandler:recordBooleanHistogram',
[CHROMEOS_GAIA_PASSWORD_METRIC, false]);
chrome.send('login.GaiaSigninScreen.userActed', ['passwordEntered']);
chrome.send('passwordEntered');
},
'authCompleted': (e) => {
// Only record the metric for Gaia flow without 3rd-party SAML IdP.
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/resources/chromeos/login/cr_ui.js
Expand Up @@ -143,9 +143,7 @@ import {OobeTypes} from './components/oobe_types.js';
chrome.send('OobeTestApi.skipToLoginForTesting');

if (!enterpriseEnroll) {
chrome.send(
'login.GaiaSigninScreen.userActed',
['completeLoginForTesting', gaia_id, username, password, false]);
chrome.send('completeLogin', [gaia_id, username, password, false]);
} else {
waitForOobeScreen('gaia-signin', function() {
// TODO(crbug.com/1100910): migrate logic to dedicated test api.
Expand Down
Expand Up @@ -611,7 +611,7 @@ class GaiaSigninElement extends GaiaSigninElementBase {
this.authenticatorParams_ = params;

this.loadAuthenticator_(params.doSamlRedirect);
this.userActed('gaiaLoaded');
chrome.send('authExtensionLoaded');
}

/**
Expand Down Expand Up @@ -765,7 +765,7 @@ class GaiaSigninElement extends GaiaSigninElementBase {
* @private
*/
samlApiUsed_(isThirdPartyIdP) {
this.userActed(['usingSAMLAPI', isThirdPartyIdP]);
chrome.send('usingSAMLAPI', [isThirdPartyIdP]);
}

/**
Expand All @@ -787,8 +787,7 @@ class GaiaSigninElement extends GaiaSigninElementBase {
this.email_ = credentials.email;
chrome.send('launchSAMLPublicSession', [credentials.email]);
} else {
this.userActed([
'completeAuthentication',
chrome.send('completeAuthentication', [
credentials.gaiaId,
credentials.email,
credentials.password,
Expand Down Expand Up @@ -913,7 +912,7 @@ class GaiaSigninElement extends GaiaSigninElementBase {
* @private
*/
onIdentifierEntered_(data) {
this.userActed(['identifierEntered', data.accountIdentifier]);
chrome.send('identifierEntered', [data.accountIdentifier]);
}

/**
Expand Down

0 comments on commit 2e4655e

Please sign in to comment.