Updates swipe from shelf state in marketing opt in screen

Disables the swipe when showing accessibility page.

Disables the swipe when screen is not shown fullscreen (i.e. the swipe
is enabled only in OOBE first run).

BUG=1059354,1058896

Change-Id: I400fb1e1321d1123b46cebfcc316cb261ff6e52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2091938
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Matthew Mourgos <mmourgos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748497}

chrome/browser/chromeos/login/screens/marketing_opt_in_screen.cc

@@ -26,9 +26,11 @@ namespace chromeos {
 
 MarketingOptInScreen::MarketingOptInScreen(
     MarketingOptInScreenView* view,
+    bool is_fullscreen,
     const base::RepeatingClosure& exit_callback)
     : BaseScreen(MarketingOptInScreenView::kScreenId),
       view_(view),
+      is_fullscreen_(is_fullscreen),
       exit_callback_(exit_callback) {
   DCHECK(view_);
   view_->Bind(this);
@@ -41,7 +43,13 @@ MarketingOptInScreen::~MarketingOptInScreen() {
 }
 
 void MarketingOptInScreen::OnShelfConfigUpdated() {
-  bool allow_shelf_gestures = !ash::ShelfConfig::Get()->shelf_controls_shown();
+  UpdateShelfGestureHandlingState();
+}
+
+void MarketingOptInScreen::UpdateShelfGestureHandlingState() {
+  const bool allow_shelf_gestures =
+      is_fullscreen_ && !ash::ShelfConfig::Get()->shelf_controls_shown() &&
+      !accessibility_page_shown_;
   if (allow_shelf_gestures == handling_shelf_gestures_)
     return;
 
@@ -96,12 +104,13 @@ void MarketingOptInScreen::ShowImpl() {
   }
 
   active_ = true;
+  accessibility_page_shown_ = false;
 
   view_->Show();
   prefs->SetBoolean(prefs::kOobeMarketingOptInScreenFinished, true);
 
   shelf_config_observer_.Add(ash::ShelfConfig::Get());
-  OnShelfConfigUpdated();
+  UpdateShelfGestureHandlingState();
 
   // Make sure the screen next button visibility is properly initialized.
   view_->UpdateAllSetButtonVisibility(!handling_shelf_gestures_ /*visible*/);
@@ -131,14 +140,11 @@ void MarketingOptInScreen::HideImpl() {
     return;
 
   active_ = false;
-
   shelf_config_observer_.RemoveAll();
-
+  active_user_pref_change_registrar_.reset();
   view_->Hide();
 
   ClearLoginShelfGestureHandler();
-
-  active_user_pref_change_registrar_.reset();
 }
 
 void MarketingOptInScreen::OnAllSet(bool play_communications_opt_in,
@@ -147,6 +153,11 @@ void MarketingOptInScreen::OnAllSet(bool play_communications_opt_in,
   ExitScreen();
 }
 
+void MarketingOptInScreen::OnAccessibilityPageVisibilityChanged(bool shown) {
+  accessibility_page_shown_ = shown;
+  UpdateShelfGestureHandlingState();
+}
+
 void MarketingOptInScreen::HandleFlingFromShelf() {
   ExitScreen();
 }

chrome/browser/chromeos/login/screens/marketing_opt_in_screen.h

@@ -23,6 +23,7 @@ class MarketingOptInScreen : public BaseScreen,
                              public ash::ShelfConfig::Observer {
  public:
   MarketingOptInScreen(MarketingOptInScreenView* view,
+                       bool is_fullscreen,
                        const base::RepeatingClosure& exit_callback);
   ~MarketingOptInScreen() override;
 
@@ -30,6 +31,10 @@ class MarketingOptInScreen : public BaseScreen,
   void OnAllSet(bool play_communications_opt_in,
                 bool tips_communications_opt_in);
 
+  // Called when the visibility of the accessibility page within the screen
+  // changes.
+  void OnAccessibilityPageVisibilityChanged(bool shown);
+
   // ash::ShelfCondif::Observer:
   void OnShelfConfigUpdated() override;
 
@@ -44,6 +49,14 @@ class MarketingOptInScreen : public BaseScreen,
   void HideImpl() override;
 
  private:
+  // Enables or disables shelf gesture handling depending on the current state.
+  // Gesture handling should be disabled if shelf navigation buttons should be
+  // shown in this context, or if the screen is on the accessibility settings
+  // page.
+  // Gesture handling is only supported if the screen is shown in fullscreen
+  // OOBE.
+  void UpdateShelfGestureHandlingState();
+
   // Called when a fling from the shelf is detected - it exits the screen.
   // This is the fling callback passed to
   // LoginScreen::SetLoginShelfGestureHandler().
@@ -66,12 +79,21 @@ class MarketingOptInScreen : public BaseScreen,
 
   MarketingOptInScreenView* const view_;
 
+  // Whether the screen is shown as part the first run with OOBE display type,
+  // i.e. if the screen UI is shown fullscreen. Note that the shelf swipe
+  // gesture will only be enabled in fullscreen mode.
+  const bool is_fullscreen_;
+
   // Whether the screen is shown and exit callback has not been run.
   bool active_ = false;
 
   // Whether the screen has set a login shelf gesture handler.
   bool handling_shelf_gestures_ = false;
 
+  // Whether the accessibility page (that contains an option to make the shelf
+  // navigation buttons show in tablet mode) is currently shown.
+  bool accessibility_page_shown_ = false;
+
   base::RepeatingClosure exit_callback_;
 
   ScopedObserver<ash::ShelfConfig, ash::ShelfConfig::Observer>

chrome/browser/chromeos/login/screens/marketing_opt_in_screen_browsertest.cc

@@ -222,4 +222,125 @@ IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTest,
   EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
 }
 
+// Tests that the user can enable shelf navigation buttons in tablet mode from
+// the screen.
+IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTest, EnableShelfNavigationButtons) {
+  ShowMarketingOptInScreen();
+  OobeScreenWaiter(MarketingOptInScreenView::kScreenId).Wait();
+
+  EXPECT_TRUE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tap on accessibility settings link, and wait for the accessibility settings
+  // UI to show up.
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityLink"})
+      ->Wait();
+  test::OobeJS().TapLinkOnPath({"marketing-opt-in", "finalAccessibilityLink"});
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityPage"})
+      ->Wait();
+
+  // Swipe from shelf should be disabled on this page.
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tap the shelf navigation buttons in tablet mode toggle.
+  test::OobeJS()
+      .CreateVisibilityWaiter(true, {"marketing-opt-in", "a11yNavButtonToggle"})
+      ->Wait();
+  test::OobeJS().ClickOnPath(
+      {"marketing-opt-in", "a11yNavButtonToggle", "button"});
+
+  // Go back to the first screen, and verify the 'all set button' is shown now.
+  test::OobeJS().TapOnPath(
+      {"marketing-opt-in", "final-accessibility-back-button"});
+
+  test::OobeJS()
+      .CreateVisibilityWaiter(
+          true, {"marketing-opt-in", "marketingOptInOverviewDialog"})
+      ->Wait();
+
+  // Verify that swipe gesture is still disabled.
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tapping the next button exits the screen.
+  test::OobeJS().ExpectVisiblePath(
+      {"marketing-opt-in", "marketing-opt-in-next-button"});
+  test::OobeJS().TapOnPath(
+      {"marketing-opt-in", "marketing-opt-in-next-button"});
+  WaitForScreenExit();
+
+  // Verify the accessibility pref for shelf navigation buttons is set.
+  EXPECT_TRUE(ProfileManager::GetActiveUserProfile()->GetPrefs()->GetBoolean(
+      ash::prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled));
+}
+
+// Tests that the user can exit the screen from the accessibility page.
+IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTest, ExitScreenFromA11yPage) {
+  ShowMarketingOptInScreen();
+  OobeScreenWaiter(MarketingOptInScreenView::kScreenId).Wait();
+
+  EXPECT_TRUE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tap on accessibility settings link, and wait for the accessibility settings
+  // UI to show up.
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityLink"})
+      ->Wait();
+  test::OobeJS().TapLinkOnPath({"marketing-opt-in", "finalAccessibilityLink"});
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityPage"})
+      ->Wait();
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tapping the next button exits the screen.
+  test::OobeJS().TapOnPath(
+      {"marketing-opt-in", "final-accessibility-next-button"});
+  WaitForScreenExit();
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+}
+
+// Tests that the swipe from shelf gets re-enabled when coming back from
+// accessibility settings page (if the shelf navigation toggle was not toggled).
+IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTest,
+                       SwipeFromShelfAfterReturnFromA11yPage) {
+  ShowMarketingOptInScreen();
+  OobeScreenWaiter(MarketingOptInScreenView::kScreenId).Wait();
+
+  EXPECT_TRUE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tap on accessibility settings link, and wait for the accessibility settings
+  // UI to show up.
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityLink"})
+      ->Wait();
+  test::OobeJS().TapLinkOnPath({"marketing-opt-in", "finalAccessibilityLink"});
+  test::OobeJS()
+      .CreateVisibilityWaiter(true,
+                              {"marketing-opt-in", "finalAccessibilityPage"})
+      ->Wait();
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Tapping back button to go back to the initial page.
+  test::OobeJS().TapOnPath(
+      {"marketing-opt-in", "final-accessibility-back-button"});
+
+  test::OobeJS()
+      .CreateVisibilityWaiter(
+          true, {"marketing-opt-in", "marketingOptInOverviewDialog"})
+      ->Wait();
+
+  // Verify that swipe gesture is enabled.
+  EXPECT_TRUE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+
+  // Swipe from shelf to exit the screen.
+  SimulateFlingFromShelf();
+  WaitForScreenExit();
+  EXPECT_FALSE(ash::ShelfTestApi().HasLoginShelfGestureHandler());
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/wizard_controller.cc

@@ -583,6 +583,7 @@ std::vector<std::unique_ptr<BaseScreen>> WizardController::CreateScreens() {
                           weak_factory_.GetWeakPtr())));
   append(std::make_unique<MarketingOptInScreen>(
       oobe_ui->GetView<MarketingOptInScreenHandler>(),
+      oobe_ui->display_type() == OobeUI::kOobeDisplay /*is_fullscreen*/,
       base::BindRepeating(&WizardController::OnMarketingOptInScreenExit,
                           weak_factory_.GetWeakPtr())));
   append(std::make_unique<PackagedLicenseScreen>(

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

@@ -96,7 +96,7 @@
     <oobe-dialog id="finalAccessibilityPage" role="dialog" has-buttons
         hidden="[[!isAccessibilitySettingsShown_]]"
         title-key="finalA11yPageTitle"
-        aria-label$="[[i18nDynamic(locale, 'finalA11yPageTitle')]]">>
+        aria-label$="[[i18nDynamic(locale, 'finalA11yPageTitle')]]">
       <hd-iron-icon slot="oobe-icon"
           icon1x="marketing-opt-in-32:accessibility"
           icon2x="marketing-opt-in-64:accessibility">
@@ -118,7 +118,8 @@
       <div slot="bottom-buttons" class="layout horizontal justified">
         <oobe-back-button on-tap="onToggleAccessibilityPage_"
             id="final-accessibility-back-button"></oobe-back-button>
-        <oobe-text-button on-tap="onAllSet_" class="focus-on-show" inverse
+        <oobe-text-button on-tap="onAllSet_" class="focus-on-show"
+            id="final-accessibility-next-button" inverse
             text-key="finalA11yPageDoneButtonTitle">
         </oobe-text-button>
       </div>

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

@@ -58,6 +58,16 @@ Polymer({
         .setPlay(true);
   },
 
+  /** Called when dialog is shown */
+  onBeforeShow() {
+    this.isAccessibilitySettingsShown_ = false;
+
+    this.behaviors.forEach((behavior) => {
+      if (behavior.onBeforeShow)
+        behavior.onBeforeShow.call(this);
+    });
+  },
+
   /**
    * This is 'on-tap' event handler for 'AcceptAndContinue/Next' buttons.
    * @private
@@ -108,6 +118,11 @@ Polymer({
     this.$['marketingOptInOverviewDialog']
         .querySelector('.marketing-animation')
         .setPlay(!this.isAccessibilitySettingsShown_);
+
+    chrome.send(
+        'login.MarketingOptInScreen.accessibilityPageVisibilityChanged', [
+          this.isAccessibilitySettingsShown_
+        ]);
   },
 
   /**

chrome/browser/ui/webui/chromeos/login/marketing_opt_in_screen_handler.cc

@@ -87,6 +87,9 @@ void MarketingOptInScreenHandler::RegisterMessages() {
   AddCallback(
       "login.MarketingOptInScreen.setA11yNavigationButtonsEnabled",
       &MarketingOptInScreenHandler::HandleSetA11yNavigationButtonsEnabled);
+  AddCallback(
+      "login.MarketingOptInScreen.accessibilityPageVisibilityChanged",
+      &MarketingOptInScreenHandler::HandleAccessibilityPageVisibilityChanged);
 }
 
 void MarketingOptInScreenHandler::GetAdditionalParameters(
@@ -103,6 +106,11 @@ void MarketingOptInScreenHandler::HandleAllSet(
   screen_->OnAllSet(play_communications_opt_in, tips_communications_opt_in);
 }
 
+void MarketingOptInScreenHandler::HandleAccessibilityPageVisibilityChanged(
+    bool shown) {
+  screen_->OnAccessibilityPageVisibilityChanged(shown);
+}
+
 void MarketingOptInScreenHandler::HandleSetA11yNavigationButtonsEnabled(
     bool enabled) {
   ProfileManager::GetActiveUserProfile()->GetPrefs()->SetBoolean(

chrome/browser/ui/webui/chromeos/login/marketing_opt_in_screen_handler.h

@@ -67,9 +67,10 @@ class MarketingOptInScreenHandler : public BaseScreenHandler,
   void RegisterMessages() override;
   void GetAdditionalParameters(base::DictionaryValue* parameters) override;
 
-  // WebUI event handler.
+  // WebUI event handlers.
   void HandleAllSet(bool play_communications_opt_in,
                     bool tips_communications_opt_in);
+  void HandleAccessibilityPageVisibilityChanged(bool shown);
   void HandleSetA11yNavigationButtonsEnabled(bool enabled);
 
   MarketingOptInScreen* screen_ = nullptr;