Commit 9a430681 authored by Maksim Ivanov's avatar Maksim Ivanov Committed by Commit Bot

Fix login spinner visibility race after SAML login

This fix part 1/2 for the bug with the bad spinner being displayed after
the end of the SAML login.

This fixes a race between multiple pieces of code changing
screen-gaia-signin's |isLoadingUiShown_|. These races led to the "please
wait" spinner not being shown in some cases.

In particular, one race condition was between (a) setting this flag to
true after the successful authentication (done by onAuthCompleted_())
and (b) setting it to false when the authenticator completely loads a
third-party page (done by onAuthReady_()). Before this. CL, the flag
could stay "false" after the SAML authentication, due to "(b)" happening
after "(a)" (after fully loading the Gaia's /programmatic_auth_chromeos
page).

The fix is to get rid of direct overwrites of the |isLoadingUiShown_|
property from different places, and instead make it computed from a few
variables that track independent states.

Bug: 1024253
Test: perform SAML login on a slow device, verify that after less than a second the "Please wait" spinner is shown
Change-Id: I8a9d3dbb859ca4c4bf87e0f9b595ae9410d7937b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2000627Reviewed-by: default avatarRoman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732840}
parent 748e5673
...@@ -103,13 +103,23 @@ Polymer({ ...@@ -103,13 +103,23 @@ Polymer({
value: DialogMode.GAIA, value: DialogMode.GAIA,
}, },
/**
* Whether the screen contents are currently being loaded.
* @private
*/
loadingFrameContents_: {
type: Boolean,
value: false,
},
/** /**
* Whether the loading UI is shown. * Whether the loading UI is shown.
* @private * @private
*/ */
isLoadingUiShown_: { isLoadingUiShown_: {
type: Boolean, type: Boolean,
value: false, computed: 'computeIsLoadingUiShown_(loadingFrameContents_, ' +
'isWhitelistErrorShown_, authCompleted_)',
}, },
/** /**
...@@ -487,7 +497,7 @@ Polymer({ ...@@ -487,7 +497,7 @@ Polymer({
* @private * @private
*/ */
loadAuthenticator_(doSamlRedirect) { loadAuthenticator_(doSamlRedirect) {
this.isLoadingUiShown_ = true; this.loadingFrameContents_ = true;
this.startLoadingTimer_(); this.startLoadingTimer_();
this.authenticatorParams_.doSamlRedirect = doSamlRedirect; this.authenticatorParams_.doSamlRedirect = doSamlRedirect;
...@@ -703,7 +713,7 @@ Polymer({ ...@@ -703,7 +713,7 @@ Polymer({
}); });
this.screenMode_ = AuthMode.DEFAULT; this.screenMode_ = AuthMode.DEFAULT;
this.isLoadingUiShown_ = true; this.loadingFrameContents_ = true;
chrome.send('loginUIStateChanged', ['gaia-signin', true]); chrome.send('loginUIStateChanged', ['gaia-signin', true]);
Oobe.getInstance().setSigninUIState(SIGNIN_UI_STATE.GAIA_SIGNIN); Oobe.getInstance().setSigninUIState(SIGNIN_UI_STATE.GAIA_SIGNIN);
...@@ -829,7 +839,7 @@ Polymer({ ...@@ -829,7 +839,7 @@ Polymer({
case AuthMode.SAML_INTERSTITIAL: case AuthMode.SAML_INTERSTITIAL:
this.samlInterstitialDomain_ = data.enterpriseDisplayDomain; this.samlInterstitialDomain_ = data.enterpriseDisplayDomain;
this.isLoadingUiShown_ = false; this.loadingFrameContents_ = false;
break; break;
} }
this.updateGuestButtonVisibility_(); this.updateGuestButtonVisibility_();
...@@ -935,7 +945,7 @@ Polymer({ ...@@ -935,7 +945,7 @@ Polymer({
this.showViewProcessed_ = false; this.showViewProcessed_ = false;
this.startLoadAnimationGuardTimer_(); this.startLoadAnimationGuardTimer_();
this.clearLoadingTimer_(); this.clearLoadingTimer_();
this.isLoadingUiShown_ = false; this.loadingFrameContents_ = false;
if (!this.$['offline-gaia'].hidden) if (!this.$['offline-gaia'].hidden)
this.$['offline-gaia'].focus(); this.$['offline-gaia'].focus();
...@@ -1074,8 +1084,6 @@ Polymer({ ...@@ -1074,8 +1084,6 @@ Polymer({
* @private * @private
*/ */
onAuthConfirmPassword_(email, passwordCount) { onAuthConfirmPassword_(email, passwordCount) {
this.isLoadingUiShown_ = true;
if (this.samlPasswordConfirmAttempt_ == 0) if (this.samlPasswordConfirmAttempt_ == 0)
chrome.send('scrapedPasswordCount', [passwordCount]); chrome.send('scrapedPasswordCount', [passwordCount]);
...@@ -1212,8 +1220,6 @@ Polymer({ ...@@ -1212,8 +1220,6 @@ Polymer({
]); ]);
} }
this.isLoadingUiShown_ = true;
// Hide the back button and the border line as they are not useful when // Hide the back button and the border line as they are not useful when
// the loading screen is shown. // the loading screen is shown.
this.setBackNavigationVisibility_(false); this.setBackNavigationVisibility_(false);
...@@ -1284,7 +1290,7 @@ Polymer({ ...@@ -1284,7 +1290,7 @@ Polymer({
if (this.screenMode_ != AuthMode.DEFAULT) if (this.screenMode_ != AuthMode.DEFAULT)
return; return;
this.authenticator_.reload(); this.authenticator_.reload();
this.isLoadingUiShown_ = true; this.loadingFrameContents_ = true;
this.startLoadingTimer_(); this.startLoadingTimer_();
this.lastBackMessageValue_ = false; this.lastBackMessageValue_ = false;
this.authCompleted_ = false; this.authCompleted_ = false;
...@@ -1361,7 +1367,7 @@ Polymer({ ...@@ -1361,7 +1367,7 @@ Polymer({
* @private * @private
*/ */
loadOffline_(params) { loadOffline_(params) {
this.isLoadingUiShown_ = true; this.loadingFrameContents_ = true;
this.startLoadingTimer_(); this.startLoadingTimer_();
const offlineLogin = this.$['offline-gaia']; const offlineLogin = this.$['offline-gaia'];
if ('enterpriseDisplayDomain' in params) if ('enterpriseDisplayDomain' in params)
...@@ -1374,7 +1380,7 @@ Polymer({ ...@@ -1374,7 +1380,7 @@ Polymer({
/** @private */ /** @private */
loadAdAuth_(params) { loadAdAuth_(params) {
this.isLoadingUiShown_ = true; this.loadingFrameContents_ = true;
this.startLoadingTimer_(); this.startLoadingTimer_();
const adAuthUI = this.getActiveFrame_(); const adAuthUI = this.getActiveFrame_();
adAuthUI.realm = params['realm']; adAuthUI.realm = params['realm'];
...@@ -1406,7 +1412,6 @@ Polymer({ ...@@ -1406,7 +1412,6 @@ Polymer({
} }
this.isWhitelistErrorShown_ = show; this.isWhitelistErrorShown_ = show;
this.isLoadingUiShown_ = !show;
if (show) if (show)
this.$['gaia-whitelist-error'].submitButton.focus(); this.$['gaia-whitelist-error'].submitButton.focus();
...@@ -1442,7 +1447,7 @@ Polymer({ ...@@ -1442,7 +1447,7 @@ Polymer({
adAuthUI.userName = username; adAuthUI.userName = username;
adAuthUI.errorState = errorState; adAuthUI.errorState = errorState;
this.authCompleted_ = false; this.authCompleted_ = false;
this.isLoadingUiShown_ = false; this.loadingFrameContents_ = false;
}, },
/** /**
...@@ -1600,6 +1605,19 @@ Polymer({ ...@@ -1600,6 +1605,19 @@ Polymer({
return loadTimeData.getStringF('samlInterstitialMessage', domain); return loadTimeData.getStringF('samlInterstitialMessage', domain);
}, },
/**
* Computes the value of the isLoadingUiShown_ property.
* @param {boolean} loadingFrameContents
* @param {boolean} isWhitelistErrorShown
* @param {boolean} authCompleted
* @return {boolean}
* @private
*/
computeIsLoadingUiShown_: function(
loadingFrameContents, isWhitelistErrorShown, authCompleted) {
return (loadingFrameContents || authCompleted) && !isWhitelistErrorShown;
},
/** /**
* Checks if string is empty * Checks if string is empty
* @param {string} value * @param {string} value
......
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