Gaia sign-in screen: Reimpl visibility via Polymer

Use Polymer data binding in order to compute elements' visibility on the
screen_gaia_signin screen.

This removes most of the imperative manipulation of "hidden" attributes
and other related logic from the JavaScript code.

The only remaining old-style visibility logic is the one for SAML video
recording (saml-notice-recording-indicator) - that one will be
refactored in a follow-up CL.

Bug: 964069
Test: manual
Change-Id: If96b65dd6fb3664b66125b162e09e5d05a7e8dba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627383
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664465}

chrome/browser/chromeos/login/saml/saml_browsertest.cc

@@ -401,7 +401,8 @@ class SamlTest : public OobeBaseTest {
         "      $('gaia-signin').authenticator_.removeEventListener("
         "          'authFlowChange', f);"
         "      window.domAutomationController.send("
-        "          $('gaia-signin').isSAML() ? 'SamlLoaded' : 'GaiaLoaded');"
+        "          $('gaia-signin').isSamlForTesting() ?"
+        "              'SamlLoaded' : 'GaiaLoaded');"
         "    });");
   }
 
@@ -1373,7 +1374,7 @@ IN_PROC_BROWSER_TEST_F(SAMLPolicyTest, SAMLInterstitialChangeAccount) {
   WaitForSigninScreen();
 
   ShowSAMLInterstitial();
-  test::OobeJS().ExpectHiddenPath({"gaia-signin", "signin-frame"});
+  test::OobeJS().ExpectHiddenPath({"gaia-signin", "signin-frame-dialog"});
   test::OobeJS().ExpectHiddenPath({"gaia-signin", "offline-gaia"});
   test::OobeJS().ExpectVisiblePath({"gaia-signin", "saml-interstitial"});
 
@@ -1381,8 +1382,9 @@ IN_PROC_BROWSER_TEST_F(SAMLPolicyTest, SAMLInterstitialChangeAccount) {
   ClickChangeAccountOnSAMLInterstitialPage();
 
   // Expects that only the gaia signin frame is visible and shown.
-  test::OobeJS().ExpectHasClass("show", {"gaia-signin", "signin-frame"});
-  test::OobeJS().ExpectVisiblePath({"gaia-signin", "signin-frame"});
+  test::OobeJS().ExpectVisiblePath({"gaia-signin", "signin-frame-dialog"});
+  test::OobeJS().ExpectHasClass("non-transparent",
+                                {"gaia-signin", "signin-frame-container"});
   test::OobeJS().ExpectHiddenPath({"gaia-signin", "offline-gaia"});
   test::OobeJS().ExpectHiddenPath({"gaia-signin", "saml-interstitial"});
 }

chrome/browser/chromeos/login/test/offline_gaia_test_mixin.cc

@@ -104,7 +104,7 @@ void OfflineGaiaTestMixin::SubmitGaiaAuthOfflineForm(
     const std::string& user_email,
     const std::string& password) {
   test::OobeJS().ExpectVisiblePath({"gaia-signin", "offline-gaia"});
-  test::OobeJS().ExpectHiddenPath({"gaia-signin", "signin-frame"});
+  test::OobeJS().ExpectHiddenPath({"gaia-signin", "signin-frame-dialog"});
   test::OobeJS()
       .CreateDisplayedWaiter(true,
                              {"gaia-signin", "offline-gaia", "email-section"})

chrome/browser/resources/chromeos/login/screen_gaia_signin.css

@@ -31,23 +31,26 @@
   width: 100%;
 }
 
+.transparent {
+  opacity: 0;
+  transition: opacity 500ms ease-in;
+}
+
+.non-transparent {
+  opacity: 1;
+  transition: opacity 500ms ease-out;
+}
+
 #signin-frame {
   border-top-left-radius: 4px;
   border-top-right-radius: 4px;
   display: block;
   height: 560px;
-  opacity: 0;
   overflow: hidden;
   position: static;
-  transition: opacity 500ms ease-in;
   width: 768px;
 }
 
-#signin-frame.show {
-  opacity: 1;
-  transition: opacity 500ms ease-out;
-}
-
 :host .step-contents {
   -webkit-box-pack: center;
   display: -webkit-box;
@@ -112,12 +115,6 @@
   visibility: visible;
 }
 
-:host(.loading) .step-contents,
-:host(.loading) .step-controls,
-:host(.loading) .step-extra-controls {
-  visibility: hidden;
-}
-
 :host(.v2) #gaia-step-contents {
   display: none;
 }

chrome/browser/resources/chromeos/login/screen_gaia_signin.html

@@ -10,42 +10,51 @@
   <template>
 
     <oobe-dialog id="signin-frame-dialog" class="gaia-dialog" role="dialog"
-        has-buttons no-header no-footer-padding no-buttons-padding hidden>
-      <div id="signin-frame-container-v2" slot="footer"
-          class="flex layout vertical">
-        <webview id="signin-frame" name="signin-frame" hidden></webview>
+        has-buttons no-header no-footer-padding no-buttons-padding
+        hidden="[[!isSigninFrameDialogVisible_(screenMode_)]]">
+      <div slot="footer" class="flex layout vertical">
+        <div id="signin-frame-container"
+            class$="[[getSigninFrameContainerClass_(isLoadingUiShown_)]]">
+          <webview id="signin-frame" name="signin-frame"></webview>
+        </div>
       </div>
       <div id="gaia-screen-navigation-buttons"
           slot="bottom-buttons" class="relative flex layout horizontal center">
         <div id="buttons-container" class="fit">
-          <oobe-back-button id="signin-back-button"></oobe-back-button>
+          <oobe-back-button id="signin-back-button"
+              disabled="[[!navigationEnabled_]]"></oobe-back-button>
           <div class="flex">
           </div>
         </div>
-        <div id="gaia-screen-buttons-overlay" class="fit" hidden></div>
+        <div id="gaia-screen-buttons-overlay" class="fit"
+            hidden="[[navigationEnabled_]]"></div>
       </div>
     </oobe-dialog>
-    <offline-gaia id="offline-gaia" class="gaia-dialog" hidden>
+    <offline-gaia id="offline-gaia" class="gaia-dialog"
+        hidden="[[!isOfflineGaiaVisible_(screenMode_, isLoadingUiShown_)]]">
     </offline-gaia>
-    <offline-ad-login id="offline-ad-auth" class="gaia-dialog" hidden
+    <offline-ad-login id="offline-ad-auth" class="gaia-dialog"
+        hidden="[[!isOfflineAdAuthVisible_(screenMode_, isLoadingUiShown_)]]"
         i18n-values="ad-welcome-message:loginWelcomeMessage">
     </offline-ad-login>
     <div id="gaia-step-contents" class="step-contents">
       <div id="gaia-signin-form-container">
         <saml-interstitial id="saml-interstitial" class="fit gaia-dialog"
-            hidden>
+            hidden="[[!isSamlInterstitialVisible_(screenMode_,
+                                                  isLoadingUiShown_)]]">
         </saml-interstitial>
       </div>
     </div>
-    <div id="gaia-loading" class="step-loading gaia-dialog" hidden>
+    <div id="gaia-loading" class="step-loading gaia-dialog"
+        hidden="[[!isLoadingUiShown_]]">
       <throbber-notice i18n-values="text:gaiaLoading"></throbber-notice>
     </div>
     <notification-card id="gaia-whitelist-error" type="fail" class="gaia-dialog"
         i18n-values="button-label:tryAgainButton;
                      link-label:learnMoreButton">
     </notification-card>
-    <div id="saml-notice-container" hidden>
-      <span id="saml-notice-recording-indicator" hidden>
+    <div id="saml-notice-container" hidden="[[!isSaml_]]">
+      <span id="saml-notice-recording-indicator" hidden="[[!videoEnabled_]]">
         <img src="chrome://theme/IDR_TAB_RECORDING_INDICATOR">
       </span>
       <span id="saml-notice-message"></span>

chrome/browser/resources/chromeos/login/screen_gaia_signin.js

@@ -65,21 +65,51 @@ Polymer({
     'invalidateAd',
   ],
 
-  properties: {},
+  properties: {
+    /**
+     * Current mode of this screen.
+     * @private
+     */
+    screenMode_: {
+      type: Number,
+      value: ScreenMode.DEFAULT,
+      observer: 'screenModeChanged_',
+    },
 
     /**
-   * Saved authenticator load params.
-   * @type {?string}
+     * Whether the loading UI is shown.
      * @private
      */
-  authenticatorParams_: null,
+    isLoadingUiShown_: {
+      type: Boolean,
+      value: false,
+    },
 
     /**
-   * Current mode of this screen.
-   * @type {ScreenMode}
+     * Whether the navigation controls are enabled.
+     * @private
+     */
+    navigationEnabled_: {
+      type: Boolean,
+      value: true,
+    },
+
+    /**
+     * Whether the authenticator is currently in the |SAML| AuthFlow.
+     * @private
+     */
+    isSaml_: {
+      type: Boolean,
+      value: false,
+    },
+  },
+
+  /**
+   * Saved authenticator load params.
+   * @type {?string}
    * @private
    */
-  screenMode_: ScreenMode.DEFAULT,
+  authenticatorParams_: null,
 
   /**
    * Email of the user, which is logging in using offline mode.
@@ -251,7 +281,7 @@ Polymer({
     // Register handlers for the saml interstitial page events.
     this.$['saml-interstitial'].addEventListener(
         'samlPageNextClicked', function() {
-          this.setScreenMode_(ScreenMode.DEFAULT);
+          this.screenMode_ = ScreenMode.DEFAULT;
           this.loadAuthenticator_(true /* doSamlRedirect */);
         }.bind(this));
     this.$['saml-interstitial'].addEventListener(
@@ -259,7 +289,7 @@ Polymer({
           // The user requests to change the account. We must clear the email
           // field of the auth params.
           this.authenticatorParams_.email = '';
-          this.setScreenMode_(ScreenMode.DEFAULT);
+          this.screenMode_ = ScreenMode.DEFAULT;
           this.loadAuthenticator_(false /* doSamlRedirect */);
         }.bind(this));
 
@@ -290,7 +320,7 @@ Polymer({
    * @private
    */
   isAtTheBeginning_: function() {
-    return !this.canGoBack_() && !this.isSAML() &&
+    return !this.canGoBack_() && !this.isSaml_ &&
         !this.classList.contains('whitelist-error') && !this.authCompleted_;
   },
 
@@ -314,7 +344,7 @@ Polymer({
   canGoBack_: function() {
     const isWhitelistError = this.classList.contains('whitelist-error');
     return this.lastBackMessageValue_ && !isWhitelistError &&
-        !this.authCompleted_ && !this.isLoadingUiShown_() && !this.isSAML();
+        !this.authCompleted_ && !this.isLoadingUiShown_ && !this.isSaml_;
   },
 
   /**
@@ -337,7 +367,7 @@ Polymer({
    * @private
    */
   loadAuthenticator_: function(doSamlRedirect) {
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
     this.startLoadingTimer_();
 
     this.authenticatorParams_.doSamlRedirect = doSamlRedirect;
@@ -355,47 +385,81 @@ Polymer({
   },
 
   /**
-   * Sets the current screen mode and updates the visible elements accordingly.
-   * @param {ScreenMode} value The screen mode.
+   * Observer that is called when the |screenMode_| property gets changed.
+   * @param {number} newValue
+   * @param {number} oldValue
    * @private
    */
-  setScreenMode_: function(value) {
-    this.screenMode_ = value;
-    switch (this.screenMode_) {
-      case ScreenMode.DEFAULT:
-        this.$['signin-frame-dialog'].hidden = false;
-        this.getSigninFrame_().hidden = false;
-        this.$['offline-gaia'].hidden = true;
-        this.$['saml-interstitial'].hidden = true;
-        this.$['offline-ad-auth'].hidden = true;
-        break;
-      case ScreenMode.OFFLINE:
-        this.$['signin-frame-dialog'].hidden = true;
-        this.getSigninFrame_().hidden = true;
-        this.$['offline-gaia'].hidden = false;
-        this.$['saml-interstitial'].hidden = true;
-        this.$['offline-ad-auth'].hidden = true;
-        break;
-      case ScreenMode.AD_AUTH:
-        this.$['signin-frame-dialog'].hidden = true;
-        this.getSigninFrame_().hidden = true;
-        this.$['offline-gaia'].hidden = true;
-        this.$['saml-interstitial'].hidden = true;
-        this.$['offline-ad-auth'].hidden = false;
-        break;
-      case ScreenMode.SAML_INTERSTITIAL:
-        this.$['signin-frame-dialog'].hidden = true;
-        this.getSigninFrame_().hidden = true;
-        this.$['offline-gaia'].hidden = true;
-        this.$['saml-interstitial'].hidden = false;
-        this.$['offline-ad-auth'].hidden = true;
-        break;
+  screenModeChanged_: function(newValue, oldValue) {
+    if (oldValue === undefined) {
+      // Ignore the first call, triggered by the assignment of the initial
+      // value.
+      return;
     }
     this.updateSigninFrameContainers_();
     chrome.send('updateOfflineLogin', [this.isOffline_()]);
     this.updateGuestButtonVisibility_();
   },
 
+  /**
+   * Whether the signin-frame-dialog element should be visible.
+   * @param {number} screenMode
+   * @return {boolean}
+   * @private
+   */
+  isSigninFrameDialogVisible_: function(screenMode) {
+    // See the comment in getSigninFrameContainerClass_() for the explanation on
+    // why our element shouldn't be hidden during loading.
+    return screenMode == ScreenMode.DEFAULT;
+  },
+
+  /**
+   * Calculates the dynamically updatable classes for the signin-frame-container
+   * element.
+   * @param {boolean} isLoadingUiShown
+   * @return {string}
+   * @private
+   */
+  getSigninFrameContainerClass_: function(isLoadingUiShown) {
+    // Use the CSS class in order to make the signin-frame webview invisible
+    // (completely transparent) during loading, since setting the "hidden"
+    // attribute would affect its loading events.
+    return isLoadingUiShown ? 'transparent' : 'non-transparent';
+  },
+
+  /**
+   * Whether the offline-gaia element should be visible.
+   * @param {number} screenMode
+   * @param {boolean} isLoadingUiShown
+   * @return {boolean}
+   * @private
+   */
+  isOfflineGaiaVisible_: function(screenMode, isLoadingUiShown) {
+    return screenMode == ScreenMode.OFFLINE && !isLoadingUiShown;
+  },
+
+  /**
+   * Whether the saml-interstitial element should be visible.
+   * @param {number} screenMode
+   * @param {boolean} isLoadingUiShown
+   * @return {boolean}
+   * @private
+   */
+  isSamlInterstitialVisible_: function(screenMode, isLoadingUiShown) {
+    return screenMode == ScreenMode.SAML_INTERSTITIAL && !isLoadingUiShown;
+  },
+
+  /**
+   * Whether the offline-ad-auth element should be visible.
+   * @param {number} screenMode
+   * @param {boolean} isLoadingUiShown
+   * @return {boolean}
+   * @private
+   */
+  isOfflineAdAuthVisible_: function(screenMode, isLoadingUiShown) {
+    return screenMode == ScreenMode.AD_AUTH && !isLoadingUiShown;
+  },
+
   /**
    * This enables or disables trying to go back to the online login page
    * after the user is idle for a few minutes, assuming that we're currently
@@ -468,35 +532,6 @@ Polymer({
     }
   },
 
-  /**
-   * Returns whether the loading UI is currently displayed.
-   * @return {boolean}
-   * @private
-   */
-  isLoadingUiShown_() {
-    return !this.$['gaia-loading'].hidden;
-  },
-
-  /**
-   * Shows/hides loading UI.
-   * @param {boolean} show True to show loading UI.
-   * @private
-   */
-  showLoadingUi_: function(show) {
-    if (show == this.isLoadingUiShown_())
-      return;
-    this.$['gaia-loading'].hidden = !show;
-
-    // Only set hidden for offline-gaia or saml-interstitial and not set it on
-    // the 'sign-frame' webview element. Setting it on webview not only hides
-    // but also affects its loading events.
-    if (this.screenMode_ != ScreenMode.DEFAULT)
-      this.getActiveFrame_().hidden = show;
-    this.getActiveFrame_().classList.toggle('show', !show);
-    this.classList.toggle('loading', show);
-    this.updateGuestButtonVisibility_();
-  },
-
   /**
    * Handler for Gaia loading timeout.
    * @private
@@ -569,8 +604,8 @@ Polymer({
         behavior.onBeforeShow.call(this);
     });
 
-    this.setScreenMode_(ScreenMode.DEFAULT);
-    this.showLoadingUi_(true);
+    this.screenMode_ = ScreenMode.DEFAULT;
+    this.isLoadingUiShown_ = true;
     chrome.send('loginUIStateChanged', ['gaia-signin', true]);
     Oobe.getInstance().setSigninUIState(SIGNIN_UI_STATE.GAIA_SIGNIN);
 
@@ -580,7 +615,7 @@ Polymer({
     });
 
     // Re-enable navigation in case it was disabled before refresh.
-    this.enableNavigation_(true);
+    this.navigationEnabled_ = true;
 
     this.lastBackMessageValue_ = false;
     this.updateGuestButtonVisibility_();
@@ -589,16 +624,6 @@ Polymer({
     this.$['signin-frame-dialog'].onBeforeShow();
   },
 
-  /**
-   * Sets whether the navigation controls are visible and enabled.
-   * @param {boolean} value
-   * @private
-   */
-  enableNavigation_: function(value) {
-    this.$['gaia-screen-buttons-overlay'].hidden = value;
-    this.$['signin-back-button'].disabled = !value;
-  },
-
   /**
    * @return {!Element}
    * @private
@@ -633,7 +658,7 @@ Polymer({
 
   /** Event handler that is invoked after the screen is shown. */
   onAfterShow: function() {
-    if (!this.isLoadingUiShown_())
+    if (!this.isLoadingUiShown_)
       this.focusActiveFrame_();
   },
 
@@ -707,13 +732,13 @@ Polymer({
     this.setSigninFramePartition_(data.webviewPartitionName);
 
     // This triggers updateSigninFrameContainers_()
-    this.setScreenMode_(data.screenMode);
+    this.screenMode_ = data.screenMode;
     this.email_ = '';
     this.authCompleted_ = false;
     this.lastBackMessageValue_ = false;
 
     // Reset SAML
-    this.$['saml-notice-container'].hidden = true;
+    this.isSaml_ = false;
     this.samlPasswordConfirmAttempt_ = 0;
 
     this.updateSigninFrameContainers_();
@@ -750,7 +775,7 @@ Polymer({
 
       case ScreenMode.SAML_INTERSTITIAL:
         this.$['saml-interstitial'].domain = data.enterpriseDisplayDomain;
-        this.showLoadingUi_(false);
+        this.isLoadingUiShown_ = false;
         // This event is for the browser tests.
         this.samlInterstitialPageReady = true;
         this.$['saml-interstitial'].fire('samlInterstitialPageReady');
@@ -784,8 +809,8 @@ Polymer({
    * Whether the current auth flow is SAML.
    * @return {boolean}
    */
-  isSAML: function() {
-    return this.authenticator_.authFlow == cr.login.Authenticator.AuthFlow.SAML;
+  isSamlForTesting: function() {
+    return this.isSaml_;
   },
 
   /**
@@ -844,10 +869,10 @@ Polymer({
    * @private
    */
   onAuthFlowChange_: function() {
-    const isSaml = this.isSAML();
+    this.isSaml_ =
+        this.authenticator_.authFlow == cr.login.Authenticator.AuthFlow.SAML;
 
-    this.$['saml-notice-container'].hidden = !isSaml;
-    this.classList.toggle('saml', isSaml);
+    this.classList.toggle('saml', this.isSaml_);
 
     if (Oobe.getInstance().currentScreen.id == 'gaia-signin') {
       Oobe.getInstance().updateScreenSize(this);
@@ -865,7 +890,7 @@ Polymer({
     this.showViewProcessed_ = false;
     this.startLoadAnimationGuardTimer_();
     this.clearLoadingTimer_();
-    this.showLoadingUi_(false);
+    this.isLoadingUiShown_ = false;
 
     if (!this.$['offline-gaia'].hidden)
       this.$['offline-gaia'].focus();
@@ -876,7 +901,7 @@ Polymer({
    * @private
    */
   onDialogShown_: function() {
-    this.enableNavigation_(false);
+    this.navigationEnabled_ = false;
   },
 
   /**
@@ -884,7 +909,7 @@ Polymer({
    * @private
    */
   onDialogHidden_: function() {
-    this.enableNavigation_(true);
+    this.navigationEnabled_ = true;
   },
 
   /**
@@ -937,7 +962,6 @@ Polymer({
 
     this.showViewProcessed_ = true;
     this.clearLoadAnimationGuardTimer_();
-    this.getActiveFrame_().classList.toggle('show', true);
     this.onLoginUIVisible_();
   },
 
@@ -965,7 +989,7 @@ Polymer({
    * @private
    */
   onAuthConfirmPassword_: function(email, passwordCount) {
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
 
     if (this.samlPasswordConfirmAttempt_ == 0)
       chrome.send('scrapedPasswordCount', [passwordCount]);
@@ -1088,7 +1112,7 @@ Polymer({
       ]);
     }
 
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
     // Hide the back button and the border line as they are not useful when
     // the loading screen is shown.
     this.$['signin-back-button'].hidden = true;
@@ -1160,7 +1184,7 @@ Polymer({
     if (this.screenMode_ != ScreenMode.DEFAULT)
       return;
     this.authenticator_.reload();
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
     this.startLoadingTimer_();
     this.lastBackMessageValue_ = false;
     this.authCompleted_ = false;
@@ -1177,7 +1201,7 @@ Polymer({
       // Reload offline version of the sign-in extension, which will show
       // error itself.
       chrome.send('offlineLogin', [this.email_]);
-    } else if (!this.isLoadingUiShown_()) {
+    } else if (!this.isLoadingUiShown_) {
       $('bubble').showContentForElement(
           this, cr.ui.Bubble.Attachment.BOTTOM, error,
           BUBBLE_HORIZONTAL_PADDING, BUBBLE_VERTICAL_PADDING);
@@ -1238,7 +1262,7 @@ Polymer({
    * @private
    */
   loadOffline_: function(params) {
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
     this.startLoadingTimer_();
     const offlineLogin = this.$['offline-gaia'];
     if ('enterpriseDisplayDomain' in params)
@@ -1251,7 +1275,7 @@ Polymer({
 
   /** @private */
   loadAdAuth_: function(params) {
-    this.showLoadingUi_(true);
+    this.isLoadingUiShown_ = true;
     this.startLoadingTimer_();
     const adAuthUI = this.getActiveFrame_();
     adAuthUI.realm = params['realm'];
@@ -1283,7 +1307,7 @@ Polymer({
     }
 
     this.classList.toggle('whitelist-error', show);
-    this.showLoadingUi_(!show);
+    this.isLoadingUiShown_ = !show;
 
     if (show)
       this.$['gaia-whitelist-error'].submitButton.focus();
@@ -1304,7 +1328,7 @@ Polymer({
     adAuthUI.userName = username;
     adAuthUI.errorState = errorState;
     this.authCompleted_ = false;
-    this.showLoadingUi_(false);
+    this.isLoadingUiShown_ = false;
   },
 
 });