[go: nahoru, domu]

Hide Apps button after a user session has started

Previously, during the post login steps the Apps button stays visible,
and starting an app crashes the recently started user session.

This is fixed by hiding the Apps button in all post login dialogs.
Hide Apps button after a user session has started

Bug: 1124319
Change-Id: Ie1c1f72c4dd473b928c3f93dc5fbeaf8f03cb30e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407312
Commit-Queue: Ossama Mahmoud <osamafathy@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813709}
diff --git a/ash/login/login_screen_controller.cc b/ash/login/login_screen_controller.cc
index ba3ed011e..d0242739 100644
--- a/ash/login/login_screen_controller.cc
+++ b/ash/login/login_screen_controller.cc
@@ -320,11 +320,11 @@
       ->SetShutdownButtonEnabled(enable);
 }
 
-void LoginScreenController::ShowGuestButtonInOobe(bool show) {
+void LoginScreenController::SetIsFirstSigninStep(bool is_first) {
   Shelf::ForWindow(Shell::Get()->GetPrimaryRootWindow())
       ->shelf_widget()
       ->login_shelf_view()
-      ->ShowGuestButtonInOobe(show);
+      ->SetIsFirstSigninStep(is_first);
 }
 
 void LoginScreenController::ShowParentAccessButton(bool show) {
diff --git a/ash/login/login_screen_controller.h b/ash/login/login_screen_controller.h
index a885dc6..c79a983 100644
--- a/ash/login/login_screen_controller.h
+++ b/ash/login/login_screen_controller.h
@@ -112,7 +112,7 @@
   bool IsReadyForPassword() override;
   void EnableAddUserButton(bool enable) override;
   void EnableShutdownButton(bool enable) override;
-  void ShowGuestButtonInOobe(bool show) override;
+  void SetIsFirstSigninStep(bool is_first) override;
   void ShowParentAccessButton(bool show) override;
   void SetAllowLoginAsGuest(bool allow_guest) override;
   std::unique_ptr<ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker()
diff --git a/ash/public/cpp/login_screen.h b/ash/public/cpp/login_screen.h
index 7ef3cb5..4dd111a 100644
--- a/ash/public/cpp/login_screen.h
+++ b/ash/public/cpp/login_screen.h
@@ -51,8 +51,9 @@
   // Sets whether shutdown button is enabled in the login screen.
   virtual void EnableShutdownButton(bool enable) = 0;
 
-  // Shows or hides the guest button on the login shelf during OOBE.
-  virtual void ShowGuestButtonInOobe(bool show) = 0;
+  // Used to show or hide apps the guest and buttons on the login shelf during
+  // OOBE.
+  virtual void SetIsFirstSigninStep(bool is_first) = 0;
 
   // Shows or hides the parent access button on the login shelf.
   virtual void ShowParentAccessButton(bool show) = 0;
diff --git a/ash/shelf/login_shelf_view.cc b/ash/shelf/login_shelf_view.cc
index a1d25ee..968c09e51 100644
--- a/ash/shelf/login_shelf_view.cc
+++ b/ash/shelf/login_shelf_view.cc
@@ -275,12 +275,6 @@
       EmptyAccountId() /*prefilled_account*/);
 }
 
-bool DialogStateGuestAllowed(OobeDialogState state) {
-  return state == OobeDialogState::GAIA_SIGNIN ||
-         state == OobeDialogState::ERROR || state == OobeDialogState::HIDDEN ||
-         state == OobeDialogState::USER_CREATION;
-}
-
 bool ShutdownButtonHidden(OobeDialogState state) {
   return state == OobeDialogState::MIGRATION ||
          state == OobeDialogState::ENROLLMENT ||
@@ -668,8 +662,8 @@
   UpdateUi();
 }
 
-void LoginShelfView::ShowGuestButtonInOobe(bool show) {
-  allow_guest_in_oobe_ = show;
+void LoginShelfView::SetIsFirstSigninStep(bool is_first) {
+  is_first_signin_step_ = is_first;
   UpdateUi();
 }
 
@@ -802,13 +796,9 @@
   // visible. The button should not appear if the device is not connected to a
   // network.
   GetViewByID(kAddUser)->SetVisible(!dialog_visible && is_login_primary);
-  // Show kiosk apps button if:
-  // 1. It's in login screen.
-  // 2. There are Kiosk apps available.
-  // 3. Device is not currently blocked.
   kiosk_apps_button_->SetVisible(kiosk_apps_button_->HasApps() &&
-                                 (is_login_primary || is_oobe) &&
-                                 (dialog_state_ != OobeDialogState::BLOCKING));
+                                 ShouldShowAppsButton());
+
   // If there is no visible (and thus focusable) buttons, we shouldn't focus
   // LoginShelfView. We update it here, so we don't need to check visibility
   // every time we move focus to system tray.
@@ -866,6 +856,22 @@
   }
 }
 
+bool LoginShelfView::ShouldShowGuestAndAppsButtons() const {
+  bool dialog_state_allowed = false;
+  if (dialog_state_ == OobeDialogState::USER_CREATION ||
+      dialog_state_ == OobeDialogState::GAIA_SIGNIN) {
+    dialog_state_allowed = !login_screen_has_users_ && is_first_signin_step_;
+  } else if (dialog_state_ == OobeDialogState::ERROR ||
+             dialog_state_ == OobeDialogState::HIDDEN) {
+    dialog_state_allowed = true;
+  }
+
+  const bool user_session_started =
+      Shell::Get()->session_controller()->NumberOfLoggedInUsers() != 0;
+
+  return dialog_state_allowed && !user_session_started;
+}
+
 // Show guest button if:
 // 1. Guest login is allowed.
 // 2. OOBE UI dialog is currently showing the login UI or error.
@@ -885,27 +891,18 @@
   if (scoped_guest_button_blockers_ > 0)
     return false;
 
-  if (!DialogStateGuestAllowed(dialog_state_))
-    return false;
-
-  const bool user_session_started =
-      Shell::Get()->session_controller()->NumberOfLoggedInUsers() != 0;
-  if (user_session_started)
+  if (!ShouldShowGuestAndAppsButtons())
     return false;
 
   const SessionState session_state =
       Shell::Get()->session_controller()->GetSessionState();
 
   if (session_state == SessionState::OOBE)
-    return allow_guest_in_oobe_;
+    return is_first_signin_step_;
 
   if (session_state != SessionState::LOGIN_PRIMARY)
     return false;
 
-  if (dialog_state_ == OobeDialogState::USER_CREATION ||
-      dialog_state_ == OobeDialogState::GAIA_SIGNIN)
-    return !login_screen_has_users_ && allow_guest_in_oobe_;
-
   return true;
 }
 
@@ -916,4 +913,16 @@
          dialog_state_ == OobeDialogState::USER_CREATION;
 }
 
+bool LoginShelfView::ShouldShowAppsButton() const {
+  if (!ShouldShowGuestAndAppsButtons())
+    return false;
+
+  const SessionState session_state =
+      Shell::Get()->session_controller()->GetSessionState();
+  if (session_state != SessionState::LOGIN_PRIMARY)
+    return false;
+
+  return true;
+}
+
 }  // namespace ash
diff --git a/ash/shelf/login_shelf_view.h b/ash/shelf/login_shelf_view.h
index c315d69..bfaa6b1 100644
--- a/ash/shelf/login_shelf_view.h
+++ b/ash/shelf/login_shelf_view.h
@@ -93,9 +93,9 @@
   // Sets whether parent access button can be shown on the login shelf.
   void ShowParentAccessButton(bool show);
 
-  // Sets if the guest button on the login shelf can be shown during gaia
-  // signin screen.
-  void ShowGuestButtonInOobe(bool show);
+  // Sets if the guest button and apps button on the login shelf can be
+  // shown during gaia signin screen.
+  void SetIsFirstSigninStep(bool is_first);
 
   // Sets whether users can be added from the login screen.
   void SetAddUserButtonEnabled(bool enable_add_user);
@@ -172,9 +172,13 @@
 
   bool ShouldShowEnterpriseEnrollmentButton() const;
 
+  bool ShouldShowAppsButton() const;
+
+  bool ShouldShowGuestAndAppsButtons() const;
+
   OobeDialogState dialog_state_ = OobeDialogState::HIDDEN;
   bool allow_guest_ = true;
-  bool allow_guest_in_oobe_ = false;
+  bool is_first_signin_step_ = false;
   bool show_parent_access_ = false;
   // When the Gaia screen is active during Login, the guest-login button should
   // appear if there are no user views.
diff --git a/ash/shelf/login_shelf_view_unittest.cc b/ash/shelf/login_shelf_view_unittest.cc
index 03e67e8..36aa208 100644
--- a/ash/shelf/login_shelf_view_unittest.cc
+++ b/ash/shelf/login_shelf_view_unittest.cc
@@ -239,24 +239,39 @@
       AreButtonsInOrder(LoginShelfView::kRestart, LoginShelfView::kSignOut));
 }
 
+// Checks that the Apps button is hidden if a session has started
+TEST_F(LoginShelfViewTest, ShouldNotShowAppsButtonAfterSessionStarted) {
+  NotifySessionStateChanged(SessionState::LOGIN_PRIMARY);
+
+  std::vector<KioskAppMenuEntry> kiosk_apps(1);
+  login_shelf_view_->SetKioskApps(kiosk_apps, {}, {});
+  EXPECT_TRUE(
+      login_shelf_view_->GetViewByID(LoginShelfView::kApps)->GetVisible());
+
+  CreateUserSessions(1);
+  EXPECT_FALSE(
+      login_shelf_view_->GetViewByID(LoginShelfView::kApps)->GetVisible());
+}
+
 // Checks that the shutdown or restart buttons shown before the Apps button when
-// kiosk mode is enabled at GAIA_SIGNIN state
+// kiosk mode is enabled
 TEST_F(LoginShelfViewTest, ShouldShowShutdownOrRestartButtonsBeforeApps){
   NotifySessionStateChanged(SessionState::LOGIN_PRIMARY);
 
   std::vector<KioskAppMenuEntry> kiosk_apps(1);
   login_shelf_view_->SetKioskApps(kiosk_apps, {}, {});
-  login_shelf_view_->SetLoginDialogState(OobeDialogState::GAIA_SIGNIN);
 
   // |reboot_on_shutdown| is initially off
-  EXPECT_TRUE(
-      ShowsShelfButtons({LoginShelfView::kShutdown, LoginShelfView::kApps}));
+  EXPECT_TRUE(ShowsShelfButtons(
+      {LoginShelfView::kShutdown, LoginShelfView::kBrowseAsGuest,
+       LoginShelfView::kAddUser, LoginShelfView::kApps}));
   EXPECT_TRUE(
       AreButtonsInOrder(LoginShelfView::kShutdown, LoginShelfView::kApps));
 
   NotifyShutdownPolicyChanged(true /*reboot_on_shutdown*/);
-  EXPECT_TRUE(
-      ShowsShelfButtons({LoginShelfView::kRestart, LoginShelfView::kApps}));
+  EXPECT_TRUE(ShowsShelfButtons(
+      {LoginShelfView::kRestart, LoginShelfView::kBrowseAsGuest,
+       LoginShelfView::kAddUser, LoginShelfView::kApps}));
   EXPECT_TRUE(
       AreButtonsInOrder(LoginShelfView::kRestart, LoginShelfView::kApps));
 }
@@ -394,23 +409,18 @@
   login_shelf_view_->SetAllowLoginAsGuest(false /*allow_guest*/);
   EXPECT_TRUE(ShowsShelfButtons({LoginShelfView::kShutdown}));
 
-  // Kiosk app button is visible when dialog state == OobeDialogState::HIDDEN
-  // or GAIA_SIGNIN.
+  // By default apps button is hidden during gaia sign in
   login_shelf_view_->SetLoginDialogState(OobeDialogState::GAIA_SIGNIN);
   std::vector<KioskAppMenuEntry> kiosk_apps(1);
   login_shelf_view_->SetKioskApps(kiosk_apps, {}, {});
-  EXPECT_TRUE(
-      ShowsShelfButtons({LoginShelfView::kShutdown, LoginShelfView::kApps}));
-  EXPECT_TRUE(
-      AreButtonsInOrder(LoginShelfView::kShutdown, LoginShelfView::kApps));
+  EXPECT_TRUE(ShowsShelfButtons({LoginShelfView::kShutdown}));
 
+  // Apps button is hidden during SAML_PASSWORD_CONFIRM STATE
   login_shelf_view_->SetLoginDialogState(
       OobeDialogState::SAML_PASSWORD_CONFIRM);
-  EXPECT_TRUE(
-      ShowsShelfButtons({LoginShelfView::kShutdown, LoginShelfView::kApps}));
-  EXPECT_TRUE(
-      AreButtonsInOrder(LoginShelfView::kShutdown, LoginShelfView::kApps));
+  EXPECT_TRUE(ShowsShelfButtons({LoginShelfView::kShutdown}));
 
+  // Kiosk apps button is visible when dialog state == OobeDialogState::HIDDEN
   login_shelf_view_->SetLoginDialogState(OobeDialogState::HIDDEN);
   EXPECT_TRUE(
       ShowsShelfButtons({LoginShelfView::kShutdown, LoginShelfView::kAddUser,
@@ -450,7 +460,7 @@
 
 TEST_F(LoginShelfViewTest, ShouldShowGuestButtonWhenNoUserPods) {
   login_shelf_view_->SetAllowLoginAsGuest(/*allow_guest=*/true);
-  login_shelf_view_->ShowGuestButtonInOobe(/*show=*/true);
+  login_shelf_view_->SetIsFirstSigninStep(/*is_first=*/true);
   SetUserCount(0);
 
   NotifySessionStateChanged(SessionState::LOGIN_PRIMARY);