AppListView should not create accessibility event if AppListViewState is

unchanged.

Bug: 1005539
Change-Id: Icb089f71cdd35f8b7c6005e83322a997270a4753
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225379Reviewed-by: Yulun Wu <yulunwu@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: Yulun Wu <yulunwu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775774}

ash/app_list/app_list_controller_impl.cc

@@ -1155,6 +1155,10 @@ void AppListControllerImpl::ViewShown(int64_t display_id) {
   keyboard_traversal_engaged_ = false;
 }
 
+bool AppListControllerImpl::AppListTargetVisibility() const {
+  return last_target_visible_;
+}
+
 void AppListControllerImpl::ViewClosing() {
   if (presenter_.GetView()->search_box_view()->is_search_box_active()) {
     // Close the virtual keyboard before the app list view is dismissed.

ash/app_list/app_list_controller_impl.h

@@ -175,6 +175,7 @@ class ASH_EXPORT AppListControllerImpl : public AppListController,
       const std::string& result_id,
       GetContextMenuModelCallback callback) override;
   void ViewShown(int64_t display_id) override;
+  bool AppListTargetVisibility() const override;
   void ViewClosing() override;
   void ViewClosed() override;
   const std::vector<SkColor>& GetWallpaperProminentColors() override;

ash/app_list/app_list_view_delegate.h

@@ -216,6 +216,10 @@ class ASH_PUBLIC_EXPORT AppListViewDelegate {
   // space. This prevents 1px gaps on displays with non-integer scale factors.
   virtual gfx::Rect SnapBoundsToDisplayEdge(const gfx::Rect& bounds) = 0;
 
+  // Returns whether the app list is visible on the display. This checks that
+  // the app list windows is open and not obstructed by another window.
+  virtual bool AppListTargetVisibility() const = 0;
+
   // Gets the current shelf height (or width for side-shelf) from the
   // ShelfConfig.
   virtual int GetShelfSize() = 0;

ash/app_list/test/app_list_test_view_delegate.cc

@@ -207,6 +207,10 @@ int AppListTestViewDelegate::GetShelfSize() {
   return 56;
 }
 
+bool AppListTestViewDelegate::AppListTargetVisibility() const {
+  return true;
+}
+
 bool AppListTestViewDelegate::IsInTabletMode() {
   return is_tablet_mode_;
 }

ash/app_list/test/app_list_test_view_delegate.h

@@ -119,6 +119,7 @@ class AppListTestViewDelegate : public AppListViewDelegate,
       AppLaunchedMetricParams* metric_params) override;
   gfx::Rect SnapBoundsToDisplayEdge(const gfx::Rect& bounds) override;
   int GetShelfSize() override;
+  bool AppListTargetVisibility() const override;
   bool IsInTabletMode() override;
   AppListNotifier* GetNotifier() override;
 

ash/app_list/views/app_list_view.cc

@@ -1209,8 +1209,15 @@ void AppListView::RecordStateTransitionForUma(AppListViewState new_state) {
 }
 
 void AppListView::MaybeCreateAccessibilityEvent(AppListViewState new_state) {
-  if (new_state != AppListViewState::kPeeking &&
-      new_state != AppListViewState::kFullscreenAllApps)
+  if (new_state == app_list_state_)
+    return;
+
+  if ((new_state != AppListViewState::kPeeking &&
+       new_state != AppListViewState::kFullscreenAllApps)) {
+    return;
+  }
+
+  if (!delegate_->AppListTargetVisibility())
     return;
 
   base::string16 state_announcement;

chrome/browser/chromeos/accessibility/spoken_feedback_app_list_browsertest.cc

@@ -7,10 +7,15 @@
 #include "ash/app_list/test/app_list_test_model.h"
 #include "ash/app_list/test/test_search_result.h"
 #include "ash/app_list/views/app_list_view.h"
+#include "ash/public/cpp/ash_switches.h"
+#include "ash/public/cpp/tablet_mode.h"
+#include "ash/public/cpp/test/shell_test_api.h"
 #include "ash/shell.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/accessibility/spoken_feedback_browsertest.h"
 #include "chrome/browser/ui/app_list/app_list_client_impl.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/constants/chromeos_switches.h"
@@ -18,6 +23,8 @@
 #include "content/public/test/browser_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/display/display.h"
+#include "ui/display/manager/display_manager.h"
 
 namespace chromeos {
 
@@ -96,6 +103,104 @@ INSTANTIATE_TEST_SUITE_P(TestAsNormalAndGuestUser,
                          ::testing::Values(kTestAsNormalUser,
                                            kTestAsGuestUser));
 
+class TabletModeSpokenFeedbackAppListTest : public SpokenFeedbackAppListTest {
+ protected:
+  TabletModeSpokenFeedbackAppListTest() = default;
+  ~TabletModeSpokenFeedbackAppListTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    SpokenFeedbackAppListTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(ash::switches::kAshEnableTabletMode);
+  }
+
+  void SetTabletMode(bool enabled) {
+    ash::ShellTestApi().SetTabletModeEnabledForTest(enabled);
+  }
+
+  bool IsTabletModeEnabled() const {
+    return ash::TabletMode::Get()->InTabletMode();
+  }
+};
+
+INSTANTIATE_TEST_SUITE_P(TestAsNormalAndGuestUser,
+                         TabletModeSpokenFeedbackAppListTest,
+                         ::testing::Values(kTestAsNormalUser,
+                                           kTestAsGuestUser));
+
+// Checks that entering and exiting tablet mode with a browser window open does
+// not generate an accessibility event.
+IN_PROC_BROWSER_TEST_P(
+    TabletModeSpokenFeedbackAppListTest,
+    HiddenAppListDoesNotCreateAccessibilityEventWhenTransitioningToTabletMode) {
+  EnableChromeVox();
+
+  sm_.Call([this]() { EXPECT_FALSE(IsTabletModeEnabled()); });
+  sm_.Call([this]() { SetTabletMode(true); });
+  sm_.Call([this]() { EXPECT_TRUE(IsTabletModeEnabled()); });
+  sm_.ExpectNextSpeechIsNot("Launcher, all apps");
+  sm_.Call([this]() { SetTabletMode(false); });
+  sm_.Call([this]() { EXPECT_FALSE(IsTabletModeEnabled()); });
+  sm_.ExpectNextSpeechIsNot("Launcher, all apps");
+  sm_.Replay();
+}
+
+// Checks that rotating the display in tablet mode does not generate an
+// accessibility event.
+IN_PROC_BROWSER_TEST_P(
+    TabletModeSpokenFeedbackAppListTest,
+    LauncherAppListScreenRotationDoesNotCreateAccessibilityEvent) {
+  display::DisplayManager* display_manager =
+      ash::Shell::Get()->display_manager();
+  const int display_id = display_manager->GetDisplayAt(0).id();
+  EnableChromeVox();
+
+  sm_.Call(
+      [this]() { EXPECT_TRUE(PerformAcceleratorAction(ash::FOCUS_SHELF)); });
+
+  sm_.Call(
+      [this]() { EXPECT_TRUE(PerformAcceleratorAction(ash::FOCUS_SHELF)); });
+  sm_.ExpectSpeech("Shelf");
+
+  // Press space on the launcher button in shelf, this opens peeking launcher.
+  sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_SPACE); });
+  sm_.ExpectSpeech("Launcher, partial view");
+
+  // Send a key press to enable keyboard traversal
+  sm_.Call([this]() { SendKeyPressWithSearchAndShift(ui::VKEY_TAB); });
+
+  // Move focus to expand all apps button.
+  sm_.Call([this]() { SendKeyPressWithSearchAndShift(ui::VKEY_TAB); });
+  sm_.ExpectSpeech("Expand to all apps");
+
+  // Press space on expand arrow to go to fullscreen launcher.
+  sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_SPACE); });
+  sm_.ExpectSpeech("Launcher, all apps");
+
+  sm_.Call([this]() { EXPECT_FALSE(IsTabletModeEnabled()); });
+  sm_.Call([this]() { SetTabletMode(true); });
+  sm_.Call([this]() { EXPECT_TRUE(IsTabletModeEnabled()); });
+
+  sm_.Call([this]() { browser()->window()->Minimize(); });
+  // Set screen rotation to 90 degrees. No ChromeVox event should be created.
+  sm_.Call([&, display_manager, display_id]() {
+    display_manager->SetDisplayRotation(display_id, display::Display::ROTATE_90,
+                                        display::Display::RotationSource::USER);
+  });
+  sm_.ExpectNextSpeechIsNot("Launcher, all apps");
+
+  // Set screen rotation to 0 degrees. No ChromeVox event should be created.
+  sm_.Call([&, display_manager, display_id]() {
+    display_manager->SetDisplayRotation(display_id, display::Display::ROTATE_0,
+                                        display::Display::RotationSource::USER);
+  });
+  sm_.ExpectNextSpeechIsNot("Launcher, all apps");
+
+  sm_.Call([this]() { EXPECT_TRUE(IsTabletModeEnabled()); });
+  sm_.Call([this]() { SetTabletMode(false); });
+  sm_.Call([this]() { EXPECT_FALSE(IsTabletModeEnabled()); });
+  sm_.Replay();
+}
+
 IN_PROC_BROWSER_TEST_P(SpokenFeedbackAppListTest, LauncherStateTransition) {
   EnableChromeVox();
 
@@ -241,8 +346,7 @@ IN_PROC_BROWSER_TEST_P(SpokenFeedbackAppListTest,
   // Move focus to app list window;
   sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
   sm_.ExpectSpeech(
-      "Search your device, apps, and web. Use the arrow keys to navigate "
-      "your "
+      "Search your device, apps, and web. Use the arrow keys to navigate your "
       "apps.");
   // Move focus to search box;
   sm_.ExpectSpeech("Edit text");