first_run: Skip app list tutorial when home button is hidden

The app list step introduces the user to the home button, which might
be confusing if the home button is not shown, which is the case in
tablet mode with kHideShelfControlsInTabletMode feature enabled (the
feature removes home/back/overview buttons from shelf in favor of
gestural navigation).

BUG=1012814

Change-Id: I8621b72a734c08d84774537e962346a74ec3c5d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015649Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734640}

ash/first_run/first_run_helper_impl.cc

@@ -46,6 +46,9 @@ gfx::Rect FirstRunHelperImpl::GetAppListButtonBounds() {
   Shelf* shelf = Shelf::ForWindow(Shell::GetPrimaryRootWindow());
   HomeButton* home_button =
       shelf->shelf_widget()->navigation_widget()->GetHomeButton();
+  if (!home_button)
+    return gfx::Rect();
+
   return home_button->GetBoundsInScreen();
 }
 

ash/public/cpp/first_run_helper.h

@@ -31,7 +31,7 @@ class ASH_EXPORT FirstRunHelper {
   static std::unique_ptr<FirstRunHelper> Start(base::OnceClosure on_cancelled);
 
   // Returns the bounds of the home button on the primary display in screen
-  // coordinates.
+  // coordinates. Returns empty bounds if the home button is not shown in shelf.
   virtual gfx::Rect GetAppListButtonBounds() = 0;
 
   // Opens the system tray bubble menu to show the default view. Does nothing if

chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc

@@ -2,12 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/system_tray_test_api.h"
+#include "ash/public/cpp/test/shell_test_api.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/first_run/first_run.h"
 #include "chrome/browser/chromeos/first_run/first_run_controller.h"
 #include "chrome/browser/chromeos/first_run/step_names.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "content/public/test/test_utils.h"
 #include "ui/aura/window.h"
 #include "ui/events/event_handler.h"
@@ -51,17 +55,34 @@ class CountingEventHandler : public ui::EventHandler {
 
 }  // namespace
 
-class FirstRunUIBrowserTest : public InProcessBrowserTest,
-                              public FirstRunActor::Delegate {
+// The param respectively indicate whether tablet mode and the
+// kHideShelfControlsInTabletMode feature are enabled.
+class FirstRunUIBrowserTest
+    : public InProcessBrowserTest,
+      public FirstRunActor::Delegate,
+      public ::testing::WithParamInterface<std::tuple<bool, bool>> {
  public:
   FirstRunUIBrowserTest()
       : initialized_(false),
         finalized_(false) {
+    if (IsHomeButtonHiddenInTabletMode()) {
+      // kHideShelfControlsInTabletMode is predicated on hotseat being enabled.
+      scoped_feature_list_.InitWithFeatures(
+          {ash::features::kHideShelfControlsInTabletMode,
+           chromeos::features::kShelfHotseat},
+          {});
+    } else {
+      scoped_feature_list_.InitWithFeatures(
+          {}, {ash::features::kHideShelfControlsInTabletMode});
+    }
   }
 
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
     tray_test_api_ = ash::SystemTrayTestApi::Create();
+
+    if (InTabletMode())
+      ash::ShellTestApi().SetTabletModeEnabledForTest(true);
   }
 
   // FirstRunActor::Delegate overrides.
@@ -98,6 +119,16 @@ class FirstRunUIBrowserTest : public InProcessBrowserTest,
 
   void OnActorDestroyed() override { controller()->OnActorDestroyed(); }
 
+  bool InTabletMode() const { return std::get<0>(GetParam()); }
+
+  bool IsHomeButtonHiddenInTabletMode() const {
+    return std::get<1>(GetParam());
+  }
+
+  bool IsHomeButtonShown() const {
+    return !InTabletMode() || !IsHomeButtonHiddenInTabletMode();
+  }
+
   void LaunchTutorial() {
     chromeos::first_run::LaunchTutorial();
     EXPECT_TRUE(controller() != NULL);
@@ -155,19 +186,30 @@ class FirstRunUIBrowserTest : public InProcessBrowserTest,
   std::string current_step_name_;
   bool initialized_;
   bool finalized_;
+  base::test::ScopedFeatureList scoped_feature_list_;
   base::OnceClosure on_initialized_callback_;
   base::OnceClosure on_step_shown_callback_;
   base::OnceClosure on_finalized_callback_;
   test::JSChecker js_;
 };
 
-IN_PROC_BROWSER_TEST_F(FirstRunUIBrowserTest, FirstRunFlow) {
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    FirstRunUIBrowserTest,
+    ::testing::Combine(
+        ::testing::Bool() /*tablet mode*/,
+        ::testing::Bool() /*home button hidden in tablet mode*/));
+
+IN_PROC_BROWSER_TEST_P(FirstRunUIBrowserTest, FirstRunFlow) {
   LaunchTutorial();
   WaitForInitialization();
-  WaitForStep(first_run::kAppListStep);
-  EXPECT_FALSE(IsTrayBubbleOpen());
 
-  AdvanceStep();
+  if (IsHomeButtonShown()) {
+    WaitForStep(first_run::kAppListStep);
+    EXPECT_FALSE(IsTrayBubbleOpen());
+    AdvanceStep();
+  }
+
   WaitForStep(first_run::kTrayStep);
   EXPECT_TRUE(IsTrayBubbleOpen());
 
@@ -182,11 +224,12 @@ IN_PROC_BROWSER_TEST_F(FirstRunUIBrowserTest, FirstRunFlow) {
 // window might be open if enterprise policy forces a browser tab to open
 // on first login and the web page opens a JavaScript alert.
 // See https://crrev.com/99673003
-IN_PROC_BROWSER_TEST_F(FirstRunUIBrowserTest, ModalWindowDoesNotBlock) {
+IN_PROC_BROWSER_TEST_P(FirstRunUIBrowserTest, ModalWindowDoesNotBlock) {
   // Start the tutorial.
   LaunchTutorial();
   WaitForInitialization();
-  WaitForStep(first_run::kAppListStep);
+  WaitForStep(IsHomeButtonShown() ? first_run::kAppListStep
+                                  : first_run::kTrayStep);
 
   // Simulate the browser opening a modal dialog.
   views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget(
@@ -208,12 +251,14 @@ IN_PROC_BROWSER_TEST_F(FirstRunUIBrowserTest, ModalWindowDoesNotBlock) {
 }
 
 // Tests that the escape key cancels the tutorial.
-IN_PROC_BROWSER_TEST_F(FirstRunUIBrowserTest, EscapeCancelsTutorial) {
+IN_PROC_BROWSER_TEST_P(FirstRunUIBrowserTest, EscapeCancelsTutorial) {
   // Run the tutorial for a couple steps, but don't finish it.
   LaunchTutorial();
   WaitForInitialization();
-  WaitForStep(first_run::kAppListStep);
-  AdvanceStep();
+  if (IsHomeButtonShown()) {
+    WaitForStep(first_run::kAppListStep);
+    AdvanceStep();
+  }
   WaitForStep(first_run::kTrayStep);
   EXPECT_TRUE(IsTrayBubbleOpen());
 

chrome/browser/chromeos/first_run/first_run_controller.cc

@@ -208,7 +208,8 @@ void FirstRunController::ShowNextStep() {
     RecordCompletion(first_run::TUTORIAL_COMPLETED_WITH_GOT_IT);
     return;
   }
-  GetCurrentStep()->Show();
+  if (!GetCurrentStep()->Show())
+    ShowNextStep();
 }
 
 void FirstRunController::AdvanceStep() {

chrome/browser/chromeos/first_run/step.cc

@@ -45,9 +45,12 @@ Step::Step(const std::string& name,
 
 Step::~Step() { RecordCompletion(); }
 
-void Step::Show() {
+bool Step::Show() {
+  if (!DoShow())
+    return false;
+
   show_time_ = base::Time::Now();
-  DoShow();
+  return true;
 }
 
 void Step::OnBeforeHide() {

chrome/browser/chromeos/first_run/step.h

@@ -24,8 +24,9 @@ class Step {
        FirstRunActor* actor);
   virtual ~Step();
 
-  // Step shows its content.
-  void Show();
+  // Step shows its content. Return whether the step was successfully shown.
+  // Returns false if the step should be skipped.
+  bool Show();
 
   // Called before hiding step.
   void OnBeforeHide();
@@ -39,8 +40,9 @@ class Step {
   FirstRunController* first_run_controller() { return first_run_controller_; }
   FirstRunActor* actor() const { return actor_; }
 
-  // Called from Show method.
-  virtual void DoShow() = 0;
+  // Called from Show method. Returns false if the step should be skipped, true
+  // otherwise.
+  virtual bool DoShow() = 0;
 
   // Called from OnBeforeHide. Step implementation could override this method to
   // react on corresponding event.

chrome/browser/chromeos/first_run/steps/app_list_step.cc

@@ -23,13 +23,17 @@ namespace first_run {
 AppListStep::AppListStep(FirstRunController* controller, FirstRunActor* actor)
     : Step(kAppListStep, controller, actor) {}
 
-void AppListStep::DoShow() {
+bool AppListStep::DoShow() {
   // FirstRunController owns this object, so use Unretained.
   gfx::Rect screen_bounds =
       first_run_controller()->first_run_helper()->GetAppListButtonBounds();
+  if (screen_bounds.IsEmpty())
+    return false;
+
   gfx::Point center = screen_bounds.CenterPoint();
   actor()->AddRoundHole(center.x(), center.y(), kCircleRadius);
   actor()->ShowStepPointingTo(name(), center.x(), center.y(), kCircleRadius);
+  return true;
 }
 
 }  // namespace first_run

chrome/browser/chromeos/first_run/steps/app_list_step.h

@@ -17,7 +17,7 @@ class AppListStep : public Step {
 
  private:
   // Step:
-  void DoShow() override;
+  bool DoShow() override;
 
   DISALLOW_COPY_AND_ASSIGN(AppListStep);
 };

chrome/browser/chromeos/first_run/steps/tray_step.cc

@@ -19,7 +19,7 @@ namespace first_run {
 TrayStep::TrayStep(FirstRunController* controller, FirstRunActor* actor)
     : Step(kTrayStep, controller, actor) {}
 
-void TrayStep::DoShow() {
+bool TrayStep::DoShow() {
   // FirstRunController owns this object, so use Unretained.
   gfx::Rect bounds =
       first_run_controller()->first_run_helper()->OpenTrayBubble();
@@ -37,6 +37,7 @@ void TrayStep::DoShow() {
     position.SetLeft(bounds.right());
   }
   actor()->ShowStepPositioned(name(), position);
+  return true;
 }
 
 }  // namespace first_run

chrome/browser/chromeos/first_run/steps/tray_step.h

@@ -17,7 +17,7 @@ class TrayStep : public Step {
 
  private:
   // Step:
-  void DoShow() override;
+  bool DoShow() override;
 
   DISALLOW_COPY_AND_ASSIGN(TrayStep);
 };