ChromeOS: Remove Guest button when Reset screen is displayed.

This Cl adds ScopedGuestButtonBlocker to temporarily hide Guest login button,
and blocks this button while Reset screen is shown.


Bug: 962398
TBR=oshima@chromium.org

Change-Id: I799d5723b4aea2a999a383c8635c790a2f510047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672629
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680731}

ash/login/login_screen_controller.cc

@@ -364,6 +364,14 @@ void LoginScreenController::SetAllowLoginAsGuest(bool allow_guest) {
       ->SetAllowLoginAsGuest(allow_guest);
 }
 
+std::unique_ptr<ScopedGuestButtonBlocker>
+LoginScreenController::GetScopedGuestButtonBlocker() {
+  return Shelf::ForWindow(Shell::Get()->GetPrimaryRootWindow())
+      ->shelf_widget()
+      ->login_shelf_view()
+      ->GetScopedGuestButtonBlocker();
+}
+
 void LoginScreenController::ShowLockScreen() {
   OnShow();
   LockScreen::Show(LockScreen::ScreenType::kLock);

ash/login/login_screen_controller.h

@@ -120,6 +120,8 @@ class ASH_EXPORT LoginScreenController : public LoginScreen,
       ParentAccessRequestReason reason,
       bool extra_dimmer) override;
   void SetAllowLoginAsGuest(bool allow_guest) override;
+  std::unique_ptr<ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker()
+      override;
 
   // KioskAppMenu:
   void SetKioskApps(

ash/public/cpp/BUILD.gn

@@ -155,6 +155,7 @@ component("cpp") {
     "rounded_corner_decorator.h",
     "scale_utility.cc",
     "scale_utility.h",
+    "scoped_guest_button_blocker.h",
     "select_to_speak_event_handler_delegate.h",
     "session/session_activation_observer.h",
     "session/session_controller.cc",

ash/public/cpp/login_screen.h

@@ -16,6 +16,7 @@ namespace ash {
 
 class LoginScreenClient;
 class LoginScreenModel;
+class ScopedGuestButtonBlocker;
 
 enum class ParentAccessRequestReason;
 
@@ -79,6 +80,10 @@ class ASH_PUBLIC_EXPORT LoginScreen {
   // true the button may still not be visible.
   virtual void SetAllowLoginAsGuest(bool allow_guest) = 0;
 
+  // Returns scoped object to temporarily disable Browse as Guest button.
+  virtual std::unique_ptr<ScopedGuestButtonBlocker>
+  GetScopedGuestButtonBlocker() = 0;
+
  protected:
   LoginScreen();
   virtual ~LoginScreen();

ash/public/cpp/scoped_guest_button_blocker.h

@@ -0,0 +1,26 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_PUBLIC_CPP_SCOPED_GUEST_BUTTON_BLOCKER_H_
+#define ASH_PUBLIC_CPP_SCOPED_GUEST_BUTTON_BLOCKER_H_
+
+#include "base/macros.h"
+
+namespace ash {
+
+// Class that temporarily disables the Browse as Guest login button on shelf.
+class ScopedGuestButtonBlocker {
+ public:
+  virtual ~ScopedGuestButtonBlocker() = default;
+
+ protected:
+  ScopedGuestButtonBlocker() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScopedGuestButtonBlocker);
+};
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_SCOPED_GUEST_BUTTON_BLOCKER_H_

ash/shelf/login_shelf_view.cc

@@ -33,6 +33,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/metrics/user_metrics.h"
+#include "base/sequence_checker.h"
 #include "chromeos/strings/grit/chromeos_strings.h"
 #include "skia/ext/image_operations.h"
 #include "ui/accessibility/ax_node_data.h"
@@ -399,6 +400,36 @@ class KioskAppsButton : public views::MenuButton,
   DISALLOW_COPY_AND_ASSIGN(KioskAppsButton);
 };
 
+// Class that temporarily disables Guest login buttin on shelf.
+class LoginShelfView::ScopedGuestButtonBlockerImpl
+    : public ScopedGuestButtonBlocker {
+ public:
+  ScopedGuestButtonBlockerImpl(base::WeakPtr<LoginShelfView> shelf_view)
+      : shelf_view_(shelf_view) {
+    ++(shelf_view_->scoped_guest_button_blockers_);
+    if (shelf_view_->scoped_guest_button_blockers_ == 1)
+      shelf_view_->UpdateUi();
+  }
+
+  ~ScopedGuestButtonBlockerImpl() override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    if (!shelf_view_)
+      return;
+
+    DCHECK_GT(shelf_view_->scoped_guest_button_blockers_, 0);
+    --(shelf_view_->scoped_guest_button_blockers_);
+    if (!shelf_view_->scoped_guest_button_blockers_)
+      shelf_view_->UpdateUi();
+  }
+
+ private:
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  // ScopedGuestButtonBlockerImpl is not owned by the LoginShelfView,
+  // so they could be independently destroyed.
+  base::WeakPtr<LoginShelfView> shelf_view_;
+};
+
 LoginShelfView::TestUiUpdateDelegate::~TestUiUpdateDelegate() = default;
 
 LoginShelfView::LoginShelfView(
@@ -590,6 +621,12 @@ void LoginShelfView::SetShutdownButtonEnabled(bool enable_shutdown_button) {
   GetViewByID(kShutdown)->SetEnabled(enable_shutdown_button);
 }
 
+std::unique_ptr<ScopedGuestButtonBlocker>
+LoginShelfView::GetScopedGuestButtonBlocker() {
+  return std::make_unique<LoginShelfView::ScopedGuestButtonBlockerImpl>(
+      weak_ptr_factory_.GetWeakPtr());
+}
+
 void LoginShelfView::OnLockScreenNoteStateChanged(
     mojom::TrayActionState state) {
   UpdateUi();
@@ -743,10 +780,14 @@ void LoginShelfView::UpdateButtonUnionBounds() {
 // are no user views available. If there are no user pods (i.e. Gaia is the
 // only signin option), the guest button should be shown if allowed by policy
 // and OOBE.
+// 6. There are no scoped guest buttons blockers active.
 bool LoginShelfView::ShouldShowGuestButton() const {
   if (!allow_guest_)
     return false;
 
+  if (scoped_guest_button_blockers_ > 0)
+    return false;
+
   if (!DialogStateGuestAllowed(dialog_state_))
     return false;
 

ash/shelf/login_shelf_view.h

@@ -14,9 +14,11 @@
 #include "ash/login/ui/login_data_dispatcher.h"
 #include "ash/public/cpp/kiosk_app_menu.h"
 #include "ash/public/cpp/login_types.h"
+#include "ash/public/cpp/scoped_guest_button_blocker.h"
 #include "ash/shutdown_controller_impl.h"
 #include "ash/system/locale/locale_update_controller_impl.h"
 #include "ash/tray_action/tray_action_observer.h"
+#include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
@@ -66,6 +68,7 @@ class ASH_EXPORT LoginShelfView : public views::View,
     virtual void OnUiUpdate() = 0;
   };
 
+ public:
   explicit LoginShelfView(
       LockScreenActionBackgroundController* lock_screen_action_background);
   ~LoginShelfView() override;
@@ -126,6 +129,9 @@ class ASH_EXPORT LoginShelfView : public views::View,
     return test_ui_update_delegate_.get();
   }
 
+  // Returns scoped object to temporarily block Browse as Guest login button.
+  std::unique_ptr<ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker();
+
  protected:
   // TrayActionObserver:
   void OnLockScreenNoteStateChanged(mojom::TrayActionState state) override;
@@ -145,6 +151,8 @@ class ASH_EXPORT LoginShelfView : public views::View,
   void OnLocaleChanged() override;
 
  private:
+  class ScopedGuestButtonBlockerImpl;
+
   bool LockScreenActionBackgroundAnimating() const;
 
   // Updates the visibility of buttons based on state changes, e.g. shutdown
@@ -197,6 +205,11 @@ class ASH_EXPORT LoginShelfView : public views::View,
   // coordinates are local to the view.
   gfx::Rect button_union_bounds_;
 
+  // Number of active scoped Guest button blockers.
+  int scoped_guest_button_blockers_ = 0;
+
+  base::WeakPtrFactory<LoginShelfView> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(LoginShelfView);
 };
 

chrome/browser/chromeos/login/reset_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "ash/public/cpp/login_screen_test_api.h"
 #include "base/command_line.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/browser_process.h"
@@ -264,20 +265,25 @@ class ResetTestWithTpmFirmwareUpdate : public ResetTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ResetTest, ShowAndCancel) {
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
   InvokeResetScreen();
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   test::OobeJS().ExpectVisible("reset");
 
   CloseResetScreen();
   test::OobeJS().CreateVisibilityWaiter(false, {"reset"})->Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 }
 
 IN_PROC_BROWSER_TEST_F(ResetTest, RestartBeforePowerwash) {
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
   PrefService* prefs = g_browser_process->local_state();
 
   InvokeResetScreen();
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   EXPECT_EQ(0, FakePowerManagerClient::Get()->num_request_restart_calls());
   EXPECT_EQ(0, FakeSessionManagerClient::Get()->start_device_wipe_call_count());
@@ -286,14 +292,17 @@ IN_PROC_BROWSER_TEST_F(ResetTest, RestartBeforePowerwash) {
   ASSERT_EQ(0, FakeSessionManagerClient::Get()->start_device_wipe_call_count());
 
   EXPECT_TRUE(prefs->GetBoolean(prefs::kFactoryResetRequested));
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 }
 
 IN_PROC_BROWSER_TEST_F(ResetOobeTest, ResetOnWelcomeScreen) {
   OobeScreenWaiter(WelcomeView::kScreenId).Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   InvokeResetScreen();
 
   OobeScreenWaiter(ResetView::kScreenId).Wait();
   test::OobeJS().ExpectVisible("reset");
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   ClickResetButton();
   EXPECT_EQ(0, FakePowerManagerClient::Get()->num_request_restart_calls());
@@ -303,14 +312,17 @@ IN_PROC_BROWSER_TEST_F(ResetOobeTest, ResetOnWelcomeScreen) {
 
 IN_PROC_BROWSER_TEST_F(ResetOobeTest, RequestAndCancleResetOnWelcomeScreen) {
   OobeScreenWaiter(WelcomeView::kScreenId).Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   InvokeResetScreen();
 
   OobeScreenWaiter(ResetView::kScreenId).Wait();
   test::OobeJS().ExpectVisible("reset");
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   CloseResetScreen();
   OobeScreenWaiter(WelcomeView::kScreenId).Wait();
   test::OobeJS().ExpectHidden("reset");
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   EXPECT_EQ(0, FakePowerManagerClient::Get()->num_request_restart_calls());
   EXPECT_EQ(0, FakeSessionManagerClient::Get()->start_device_wipe_call_count());
@@ -330,23 +342,28 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, ViewsLogic) {
   update_engine_client_->set_can_rollback_check_result(false);
   InvokeResetScreen();
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   test::OobeJS().CreateVisibilityWaiter(true, {"reset"})->Wait();
   test::OobeJS().ExpectHidden("overlay-reset");
   CloseResetScreen();
   test::OobeJS().CreateVisibilityWaiter(false, {"reset"})->Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   // Go to confirmation phase, cancel from there in 2 steps.
   prefs->SetBoolean(prefs::kFactoryResetRequested, true);
   InvokeResetScreen();
   test::OobeJS().CreateVisibilityWaiter(false, {"overlay-reset"})->Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   ClickToConfirmButton();
   test::OobeJS().CreateVisibilityWaiter(true, {"overlay-reset"})->Wait();
   ClickDismissConfirmationButton();
   test::OobeJS().CreateVisibilityWaiter(false, {"overlay-reset"})->Wait();
   test::OobeJS().CreateVisibilityWaiter(true, {"reset"})->Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   CloseResetScreen();
   test::OobeJS().CreateVisibilityWaiter(false, {"reset"})->Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   // Rollback available. Show and cancel from confirmation screen.
   update_engine_client_->set_can_rollback_check_result(true);
@@ -371,10 +388,13 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, PRE_ShowAfterBootIfRequested) {
 IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, ShowAfterBootIfRequested) {
   OobeScreenWaiter(ResetView::kScreenId).Wait();
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   test::OobeJS().CreateVisibilityWaiter(true, {"reset"})->Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   CloseResetScreen();
   test::OobeJS().CreateVisibilityWaiter(false, {"reset"})->Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 }
 
 IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, PRE_RollbackUnavailable) {
@@ -385,6 +405,7 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, PRE_RollbackUnavailable) {
 IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, RollbackUnavailable) {
   InvokeResetScreen();
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   EXPECT_EQ(0, FakePowerManagerClient::Get()->num_request_restart_calls());
   EXPECT_EQ(0, FakeSessionManagerClient::Get()->start_device_wipe_call_count());
@@ -397,6 +418,7 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTest, RollbackUnavailable) {
   EXPECT_EQ(0, update_engine_client_->rollback_call_count());
   CloseResetScreen();
   OobeScreenExitWaiter(ResetView::kScreenId).Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   // Next invocation leads to rollback view.
   PrefService* prefs = g_browser_process->local_state();
@@ -419,8 +441,12 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTestWithRollback,
 IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTestWithRollback, RollbackAvailable) {
   PrefService* prefs = g_browser_process->local_state();
 
-  InvokeResetScreen();
+  // PRE test triggers start with Reset screen.
+  OobeScreenWaiter(ResetView::kScreenId).Wait();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
+
   EXPECT_TRUE(login_prompt_visible_observer_->signal_emitted());
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   EXPECT_EQ(0, FakePowerManagerClient::Get()->num_request_restart_calls());
   EXPECT_EQ(0, FakeSessionManagerClient::Get()->start_device_wipe_call_count());
@@ -432,14 +458,19 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTestWithRollback, RollbackAvailable) {
   EXPECT_EQ(0, update_engine_client_->rollback_call_count());
   CloseResetScreen();
   OobeScreenExitWaiter(ResetView::kScreenId).Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   // Next invocation leads to simple reset, not rollback view.
   prefs->SetBoolean(prefs::kFactoryResetRequested, true);
   InvokeResetScreen();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   InvokeRollbackOption();  // Shows rollback.
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   ClickDismissConfirmationButton();
+  EXPECT_FALSE(ash::LoginScreenTestApi::IsGuestButtonShown());
   CloseResetScreen();
   OobeScreenExitWaiter(ResetView::kScreenId).Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   InvokeResetScreen();
   ClickToConfirmButton();
@@ -449,6 +480,7 @@ IN_PROC_BROWSER_TEST_F(ResetFirstAfterBootTestWithRollback, RollbackAvailable) {
   EXPECT_EQ(0, update_engine_client_->rollback_call_count());
   CloseResetScreen();
   OobeScreenExitWaiter(ResetView::kScreenId).Wait();
+  EXPECT_TRUE(ash::LoginScreenTestApi::IsGuestButtonShown());
 
   prefs->SetBoolean(prefs::kFactoryResetRequested, true);
   InvokeResetScreen();