Hide App Icons in KioskNextShell.

Bug: 951212
Change-Id: Ie69a2cd77aaa9d169d102cd3582a6d1902306001
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610501Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yilkal Abe <yilkal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660091}

ash/kiosk_next/kiosk_next_shell_controller.cc

@@ -12,14 +12,45 @@
 #include "ash/kiosk_next/kiosk_next_shell_observer.h"
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/ash_pref_names.h"
+#include "ash/public/cpp/shelf_model.h"
 #include "ash/session/session_controller_impl.h"
+#include "ash/shelf/home_button_delegate.h"
 #include "ash/shell.h"
+#include "ash/strings/grit/ash_strings.h"
+#include "chromeos/strings/grit/chromeos_strings.h"
 #include "components/account_id/account_id.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace ash {
 
+namespace {
+
+std::unique_ptr<ShelfModel> CreateKioskNextShelfModel() {
+  auto shelf_model = std::make_unique<ShelfModel>();
+
+  shelf_model->SetShelfItemDelegate(ShelfID(kBackButtonId), nullptr);
+  shelf_model->SetShelfItemDelegate(ShelfID(kAppListId),
+                                    std::make_unique<HomeButtonDelegate>());
+
+  DCHECK_EQ(0, shelf_model->ItemIndexByID(ShelfID(kBackButtonId)));
+  DCHECK_EQ(1, shelf_model->ItemIndexByID(ShelfID(kAppListId)));
+
+  ShelfItem back_item = shelf_model->items()[0];
+  ShelfItem home_item = shelf_model->items()[1];
+
+  back_item.title = l10n_util::GetStringUTF16(IDS_ASH_SHELF_BACK_BUTTON_TITLE);
+  home_item.title =
+      l10n_util::GetStringUTF16(IDS_ASH_SHELF_APP_LIST_LAUNCHER_TITLE);
+
+  shelf_model->Set(0, back_item);
+  shelf_model->Set(1, home_item);
+  return shelf_model;
+}
+
+}  // namespace
+
 KioskNextShellController::KioskNextShellController() = default;
 
 KioskNextShellController::~KioskNextShellController() = default;
@@ -90,6 +121,8 @@ void KioskNextShellController::LaunchKioskNextShellIfEnabled() {
   kiosk_next_shell_client_->LaunchKioskNextShell(
       session_controller->GetPrimaryUserSession()->user_info.account_id);
 
+  shelf_model_ = CreateKioskNextShelfModel();
+
   // Notify observers that KioskNextShell has been enabled.
   for (KioskNextShellObserver& observer : observer_list_) {
     observer.OnKioskNextEnabled();

ash/kiosk_next/kiosk_next_shell_controller.h

@@ -5,6 +5,8 @@
 #ifndef ASH_KIOSK_NEXT_KIOSK_NEXT_SHELL_CONTROLLER_H_
 #define ASH_KIOSK_NEXT_KIOSK_NEXT_SHELL_CONTROLLER_H_
 
+#include <memory>
+
 #include "ash/ash_export.h"
 #include "ash/kiosk_next/kiosk_next_shell_observer.h"
 #include "ash/public/interfaces/kiosk_next_shell.mojom.h"
@@ -18,6 +20,7 @@ class PrefRegistrySimple;
 namespace ash {
 
 class KioskNextHomeController;
+class ShelfModel;
 
 // KioskNextShellController allows an ash consumer to manage a Kiosk Next
 // session. During this session most system functions are disabled and we launch
@@ -48,6 +51,8 @@ class ASH_EXPORT KioskNextShellController
   // SessionObserver:
   void OnActiveUserPrefServiceChanged(PrefService* pref_service) override;
 
+  ShelfModel* shelf_model() { return shelf_model_.get(); }
+
  private:
   // Launches Kiosk Next if the pref is enabled and the KioskNextShellClient is
   // available.
@@ -62,6 +67,11 @@ class ASH_EXPORT KioskNextShellController
   // Controls the KioskNext home screen when the Kiosk Next Shell is enabled.
   std::unique_ptr<KioskNextHomeController> kiosk_next_home_controller_;
 
+  // When KioskNextShell is enabled, only the home button and back button are
+  // made visible on the Shelf. KioskNextShellController therefore hosts its own
+  // ShelfModel to control the entries visible on the shelf.
+  std::unique_ptr<ShelfModel> shelf_model_;
+
   DISALLOW_COPY_AND_ASSIGN(KioskNextShellController);
 };
 

ash/shelf/back_button_unittest.cc

@@ -32,14 +32,13 @@ class BackButtonTest : public AshTestBase {
   BackButtonTest() = default;
   ~BackButtonTest() override = default;
 
-  BackButton* back_button() { return back_button_; }
+  BackButton* back_button() { return test_api_->shelf_view()->GetBackButton(); }
   ShelfViewTestAPI* test_api() { return test_api_.get(); }
 
   void SetUp() override {
     AshTestBase::SetUp();
     test_api_ = std::make_unique<ShelfViewTestAPI>(
         GetPrimaryShelf()->GetShelfViewForTesting());
-    back_button_ = test_api_->shelf_view()->GetBackButton();
 
     // Finish all setup tasks. In particular we want to finish the
     // GetSwitchStates post task in (Fake)PowerManagerClient which is triggered
@@ -49,7 +48,6 @@ class BackButtonTest : public AshTestBase {
   }
 
  private:
-  BackButton* back_button_ = nullptr;
   std::unique_ptr<ShelfViewTestAPI> test_api_;
 
   DISALLOW_COPY_AND_ASSIGN(BackButtonTest);

ash/shelf/shelf_view.cc

@@ -12,7 +12,6 @@
 #include "ash/focus_cycler.h"
 #include "ash/keyboard/keyboard_util.h"
 #include "ash/keyboard/ui/keyboard_controller.h"
-#include "ash/kiosk_next/kiosk_next_shell_controller.h"
 #include "ash/metrics/user_metrics_recorder.h"
 #include "ash/public/cpp/ash_constants.h"
 #include "ash/public/cpp/ash_features.h"
@@ -1234,17 +1233,6 @@ ShelfView::AppCenteringStrategy ShelfView::CalculateAppCenteringStrategy()
   if (is_overflow_mode())
     return strategy;
 
-  // We only show the back and home buttons in Kiosk Next, with no overflow
-  // button.
-  // TODO(https://crbug.com/951212): Prevent creation of the app item views
-  // instead of using this hack. This is difficult today due to the tight
-  // coupling of the view model and shelf model.
-  if (Shell::Get()->kiosk_next_shell_controller() &&
-      Shell::Get()->kiosk_next_shell_controller()->IsEnabled()) {
-    last_visible_index_ = 1;
-    return strategy;
-  }
-
   const int total_available_size = shelf_->PrimaryAxisValue(width(), height());
   StatusAreaWidget* status_widget = shelf_widget_->status_area_widget();
   const int status_widget_size =

ash/shelf/shelf_view_unittest.cc

@@ -13,6 +13,7 @@
 #include "ash/app_list/views/app_list_view.h"
 #include "ash/focus_cycler.h"
 #include "ash/ime/ime_controller.h"
+#include "ash/kiosk_next/kiosk_next_shell_controller.h"
 #include "ash/kiosk_next/kiosk_next_shell_test_util.h"
 #include "ash/kiosk_next/mock_kiosk_next_shell_client.h"
 #include "ash/public/cpp/ash_features.h"
@@ -3953,6 +3954,15 @@ class KioskNextShelfViewTest : public ShelfViewTest {
     client_ = BindMockKioskNextShellClient();
   }
 
+ protected:
+  void LogInKioskNextUserInternal() {
+    LogInKioskNextUser(GetSessionControllerClient());
+
+    // The shelf_view_ in ShelfWidget will be replaced. Therefore, we need
+    // to update |shelf_view_|.
+    shelf_view_ = GetPrimaryShelf()->GetShelfViewForTesting();
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<MockKioskNextShellClient> client_;
@@ -3961,16 +3971,22 @@ class KioskNextShelfViewTest : public ShelfViewTest {
 };
 
 TEST_F(KioskNextShelfViewTest, AppButtonHidden) {
-  LogInKioskNextUser(GetSessionControllerClient());
+  // When a KioskNextUser is not logged in, the shelf model is not hosted
+  // in KioskNextSellController.
+  EXPECT_FALSE(shelf_view_->model() ==
+               Shell::Get()->kiosk_next_shell_controller()->shelf_model());
+
+  LogInKioskNextUserInternal();
+
+  // When a KiosknextUser is logged in, the shelf model for the shelf view
+  // is hosted in KioskNextShellController.
+  EXPECT_TRUE(shelf_view_->model() ==
+              Shell::Get()->kiosk_next_shell_controller()->shelf_model());
 
   // The home and back buttons are always visible.
   EXPECT_TRUE(shelf_view_->GetAppListButton()->GetVisible());
   EXPECT_TRUE(shelf_view_->GetBackButton()->GetVisible());
 
-  // Adding app items doesn't add them to the visible shelf, and the overflow
-  // button remains hidden.
-  AddApp();
-  AddApp();
   ASSERT_FALSE(shelf_view_->GetOverflowButton()->GetVisible());
   EXPECT_EQ(1, shelf_view_->last_visible_index());
 }

ash/shelf/shelf_widget.cc

@@ -9,6 +9,7 @@
 #include "ash/animation/animation_change_type.h"
 #include "ash/focus_cycler.h"
 #include "ash/keyboard/ui/keyboard_controller.h"
+#include "ash/kiosk_next/kiosk_next_shell_controller.h"
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/window_properties.h"
@@ -363,6 +364,14 @@ ShelfWidget::ShelfWidget(aura::Window* shelf_container, Shelf* shelf)
 
   background_animator_.AddObserver(delegate_view_);
   shelf_->AddObserver(this);
+
+  // KioskNextShell controller may have already notified its observers that
+  // it has been enabled by the time this ShelfWidget is being created.
+  if (Shell::Get()->kiosk_next_shell_controller()->IsEnabled()) {
+    OnKioskNextEnabled();
+  } else {
+    Shell::Get()->kiosk_next_shell_controller()->AddObserver(this);
+  }
 }
 
 ShelfWidget::~ShelfWidget() {
@@ -391,6 +400,9 @@ void ShelfWidget::Shutdown() {
   background_animator_.RemoveObserver(delegate_view_);
   shelf_->RemoveObserver(this);
 
+  if (Shell::Get()->kiosk_next_shell_controller())
+    Shell::Get()->kiosk_next_shell_controller()->RemoveObserver(this);
+
   // Don't need to observe focus/activation during shutdown.
   Shell::Get()->focus_cycler()->RemoveWidget(this);
   SetFocusCycler(nullptr);
@@ -585,6 +597,18 @@ void ShelfWidget::OnUserSessionAdded(const AccountId& account_id) {
   login_shelf_view_->UpdateAfterSessionChange();
 }
 
+void ShelfWidget::OnKioskNextEnabled() {
+  // Hide the shelf view and delete/remove it.
+  shelf_view_->SetVisible(false);
+  delete shelf_view_;
+
+  shelf_view_ = new ShelfView(
+      Shell::Get()->kiosk_next_shell_controller()->shelf_model(), shelf_, this);
+  shelf_view_->Init();
+  GetContentsView()->AddChildView(shelf_view_);
+  shelf_view_->SetVisible(true);
+}
+
 SkColor ShelfWidget::GetShelfBackgroundColor() const {
   return delegate_view_->GetShelfBackgroundColor();
 }

ash/shelf/shelf_widget.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/kiosk_next/kiosk_next_shell_observer.h"
 #include "ash/public/cpp/shelf_types.h"
 #include "ash/session/session_observer.h"
 #include "ash/shelf/shelf_background_animator.h"
@@ -39,7 +40,8 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
                                public views::WidgetObserver,
                                public ShelfLayoutManagerObserver,
                                public ShelfObserver,
-                               public SessionObserver {
+                               public SessionObserver,
+                               public KioskNextShellObserver {
  public:
   ShelfWidget(aura::Window* shelf_container, Shelf* shelf);
   ~ShelfWidget() override;
@@ -118,6 +120,9 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
   void OnSessionStateChanged(session_manager::SessionState state) override;
   void OnUserSessionAdded(const AccountId& account_id) override;
 
+  // KioskNextShellObserver:
+  void OnKioskNextEnabled() override;
+
   SkColor GetShelfBackgroundColor() const;
   bool GetHitTestRects(aura::Window* target,
                        gfx::Rect* hit_test_rect_mouse,
@@ -162,7 +167,7 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
 
   // View containing the shelf items within an active user session. Owned by
   // the views hierarchy.
-  ShelfView* const shelf_view_;
+  ShelfView* shelf_view_;
 
   // View containing the shelf items for Login/Lock/OOBE/Add User screens.
   // Owned by the views hierarchy.