CrOS Settings: Hide shelf navigation setting on clamshell-only devices.

Currently the setting for showing shelf navigation buttons is shown in
settings for all devices as long as the
IsHideShelfControlsInTabletModeEnabled feature is enabled. This setting
should be hidden on devices that are not tablet capable and are
clamshell-only.

This cl hides the setting from showing up in settings when the device is
not tablet capable.

Bug: 1062213
Change-Id: I99574f7b340d41250165618f327815617300a128
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152988
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#764211}

ash/public/cpp/BUILD.gn

@@ -345,6 +345,8 @@ source_set("test_support") {
     "test/test_new_window_delegate.h",
     "test/test_system_tray_client.cc",
     "test/test_system_tray_client.h",
+    "test/test_tablet_mode.cc",
+    "test/test_tablet_mode.h",
   ]
 
   deps = [

ash/public/cpp/test/test_tablet_mode.cc

+// Copyright 2020 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.
+
+#include "ash/public/cpp/test/test_tablet_mode.h"
+
+namespace ash {
+
+TestTabletMode::TestTabletMode() = default;
+TestTabletMode::~TestTabletMode() = default;
+
+void TestTabletMode::AddObserver(TabletModeObserver* observer) {}
+void TestTabletMode::RemoveObserver(TabletModeObserver* observer) {}
+
+bool TestTabletMode::InTabletMode() const {
+  return in_tablet_mode_;
+}
+
+void TestTabletMode::ForceUiTabletModeState(base::Optional<bool> enabled) {}
+
+void TestTabletMode::SetEnabledForTest(bool enabled) {
+  in_tablet_mode_ = enabled;
+}
+
+}  // namespace ash
\ No newline at end of file

ash/public/cpp/test/test_tablet_mode.h

+// Copyright 2020 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_TEST_TEST_TABLET_MODE_H_
+#define ASH_PUBLIC_CPP_TEST_TEST_TABLET_MODE_H_
+
+#include "ash/public/cpp/tablet_mode.h"
+
+namespace ash {
+
+// TabletMode implementation for test support.
+class TestTabletMode : public TabletMode {
+ public:
+  TestTabletMode();
+  ~TestTabletMode() override;
+
+  // TabletMode:
+  void AddObserver(ash::TabletModeObserver* observer) override;
+  void RemoveObserver(ash::TabletModeObserver* observer) override;
+  bool InTabletMode() const override;
+  void ForceUiTabletModeState(base::Optional<bool> enabled) override;
+  void SetEnabledForTest(bool enabled) override;
+
+ private:
+  bool in_tablet_mode_ = false;
+};
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_TEST_TEST_TABLET_MODE_H_
\ No newline at end of file

chrome/browser/chromeos/BUILD.gn

@@ -3251,6 +3251,7 @@ source_set("unit_tests") {
     "../ui/webui/chromeos/login/l10n_util_unittest.cc",
     "../ui/webui/chromeos/login/oobe_display_chooser_unittest.cc",
     "../ui/webui/chromeos/login/signin_userlist_unittest.cc",
+    "../ui/webui/settings/chromeos/accessibility_handler_unittest.cc",
     "../ui/webui/settings/chromeos/calculator/size_calculator_test_api.h",
     "../ui/webui/settings/chromeos/device_keyboard_handler_unittest.cc",
     "../ui/webui/settings/chromeos/device_storage_handler_unittest.cc",

chrome/browser/resources/settings/chromeos/os_a11y_page/manage_a11y_page.js

@@ -113,10 +113,7 @@ Polymer({
      */
     showShelfNavigationButtonsSettings_: {
       type: Boolean,
-      value() {
-        return loadTimeData.getBoolean(
-            'showTabletModeShelfNavigationButtonsSettings');
-      },
+      value: false,
     },
 
     /** @private */
@@ -192,9 +189,8 @@ Polymer({
   /** @override */
   ready() {
     this.addWebUIListener(
-        'startup-sound-enabled-updated',
-        this.updateStartupSoundEnabled_.bind(this));
-    chrome.send('getStartupSoundEnabled');
+        'initial-data-ready', this.onManageAllyPageReady_.bind(this));
+    chrome.send('manageA11yPageReady');
 
     const r = settings.routes;
     this.addFocusConfig_(r.MANAGE_TTS_SETTINGS, '#ttsSubpageButton');
@@ -242,14 +238,6 @@ Polymer({
     chrome.send('setStartupSoundEnabled', [e.detail]);
   },
 
-  /**
-   * @param {boolean} enabled
-   * @private
-   */
-  updateStartupSoundEnabled_(enabled) {
-    this.$.startupSoundEnabled.checked = enabled;
-  },
-
   /** @private */
   onManageTtsSettingsTap_() {
     settings.Router.getInstance().navigateTo(
@@ -371,4 +359,17 @@ Polymer({
         settings.routes.POINTERS,
         /* dynamicParams */ null, /* removeSearch */ true);
   },
+
+  /**
+   * Handles updating the visibility of the shelf navigation buttons setting
+   * and updating whether startupSoundEnabled is checked.
+   * @param {boolean} startup_sound_enabled Whether startup sound is enabled.
+   * @param {boolean} tablet_mode_supported Whether tablet mode is supported.
+   * @private
+   */
+  onManageAllyPageReady_(startup_sound_enabled, tablet_mode_supported) {
+    this.$.startupSoundEnabled.checked = startup_sound_enabled;
+    this.showShelfNavigationButtonsSettings_ = tablet_mode_supported &&
+        loadTimeData.getBoolean('showTabletModeShelfNavigationButtonsSettings');
+  },
 });

chrome/browser/ui/webui/settings/chromeos/accessibility_handler.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/webui/settings/chromeos/accessibility_handler.h"
 
+#include "ash/public/cpp/tablet_mode.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/histogram_functions.h"
@@ -30,9 +31,8 @@ void RecordShowShelfNavigationButtonsValueChange(bool enabled) {
 
 }  // namespace
 
-AccessibilityHandler::AccessibilityHandler(content::WebUI* webui)
-    : profile_(Profile::FromWebUI(webui)) {
-}
+AccessibilityHandler::AccessibilityHandler(Profile* profile)
+    : profile_(profile) {}
 
 AccessibilityHandler::~AccessibilityHandler() {
   if (a11y_nav_buttons_toggle_metrics_reporter_timer_.IsRunning())
@@ -49,10 +49,6 @@ void AccessibilityHandler::RegisterMessages() {
       base::BindRepeating(
           &AccessibilityHandler::HandleShowSelectToSpeakSettings,
           base::Unretained(this)));
-  web_ui()->RegisterMessageCallback(
-      "getStartupSoundEnabled",
-      base::BindRepeating(&AccessibilityHandler::HandleGetStartupSoundEnabled,
-                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "setStartupSoundEnabled",
       base::BindRepeating(&AccessibilityHandler::HandleSetStartupSoundEnabled,
@@ -64,6 +60,11 @@ void AccessibilityHandler::RegisterMessages() {
           &AccessibilityHandler::
               HandleRecordSelectedShowShelfNavigationButtonsValue,
           base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
+      "manageA11yPageReady",
+      base::BindRepeating(&AccessibilityHandler::HandleManageA11yPageReady,
+                          base::Unretained(this)));
 }
 
 void AccessibilityHandler::HandleShowChromeVoxSettings(
@@ -76,14 +77,6 @@ void AccessibilityHandler::HandleShowSelectToSpeakSettings(
   OpenExtensionOptionsPage(extension_misc::kSelectToSpeakExtensionId);
 }
 
-void AccessibilityHandler::HandleGetStartupSoundEnabled(
-    const base::ListValue* args) {
-  AllowJavascript();
-  FireWebUIListener(
-      "startup-sound-enabled-updated",
-      base::Value(AccessibilityManager::Get()->GetStartupSoundEnabled()));
-}
-
 void AccessibilityHandler::HandleSetStartupSoundEnabled(
     const base::ListValue* args) {
   DCHECK_EQ(1U, args->GetSize());
@@ -103,6 +96,37 @@ void AccessibilityHandler::HandleRecordSelectedShowShelfNavigationButtonsValue(
       base::BindOnce(&RecordShowShelfNavigationButtonsValueChange, enabled));
 }
 
+void AccessibilityHandler::HandleManageA11yPageReady(
+    const base::ListValue* args) {
+  AllowJavascript();
+
+  // When tablet mode is active we can return early since tablet mode
+  // is supported.
+  if (ash::TabletMode::Get()->InTabletMode()) {
+    FireWebUIListener(
+        "initial-data-ready",
+        base::Value(AccessibilityManager::Get()->GetStartupSoundEnabled()),
+        base::Value(true /* tablet_mode_supported */));
+    return;
+  }
+
+  PowerManagerClient::Get()->GetSwitchStates(
+      base::BindOnce(&AccessibilityHandler::OnReceivedSwitchStates,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void AccessibilityHandler::OnReceivedSwitchStates(
+    base::Optional<PowerManagerClient::SwitchStates> switch_states) {
+  bool tablet_mode_supported =
+      switch_states.has_value() &&
+      switch_states->tablet_mode != PowerManagerClient::TabletMode::UNSUPPORTED;
+
+  FireWebUIListener(
+      "initial-data-ready",
+      base::Value(AccessibilityManager::Get()->GetStartupSoundEnabled()),
+      base::Value(tablet_mode_supported));
+}
+
 void AccessibilityHandler::OpenExtensionOptionsPage(const char extension_id[]) {
   const extensions::Extension* extension =
       extensions::ExtensionRegistry::Get(profile_)->GetExtensionById(

chrome/browser/ui/webui/settings/chromeos/accessibility_handler.h

@@ -8,15 +8,12 @@
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
+#include "chromeos/dbus/power/power_manager_client.h"
 
 namespace base {
 class ListValue;
 }
 
-namespace content {
-class WebUI;
-}
-
 class Profile;
 
 namespace chromeos {
@@ -24,7 +21,7 @@ namespace settings {
 
 class AccessibilityHandler : public ::settings::SettingsPageUIHandler {
  public:
-  explicit AccessibilityHandler(content::WebUI* webui);
+  explicit AccessibilityHandler(Profile* profile);
   ~AccessibilityHandler() override;
 
   // SettingsPageUIHandler implementation.
@@ -32,16 +29,24 @@ class AccessibilityHandler : public ::settings::SettingsPageUIHandler {
   void OnJavascriptAllowed() override {}
   void OnJavascriptDisallowed() override {}
 
+  // Callback which updates if startup sound is enabled and if tablet
+  // mode is supported. Visible for testing.
+  void HandleManageA11yPageReady(const base::ListValue* args);
+
  private:
   // Callback for the messages to show settings for ChromeVox or
   // Select To Speak.
   void HandleShowChromeVoxSettings(const base::ListValue* args);
   void HandleShowSelectToSpeakSettings(const base::ListValue* args);
-  void HandleGetStartupSoundEnabled(const base::ListValue* args);
   void HandleSetStartupSoundEnabled(const base::ListValue* args);
   void HandleRecordSelectedShowShelfNavigationButtonsValue(
       const base::ListValue* args);
 
+  // Callback which updates visibility for the shelf navigation buttons
+  // accessibility setting, depending on whether tablet mode is supported.
+  void OnReceivedSwitchStates(
+      base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states);
+
   void OpenExtensionOptionsPage(const char extension_id[]);
 
   Profile* profile_;  // Weak pointer.
@@ -52,6 +57,8 @@ class AccessibilityHandler : public ::settings::SettingsPageUIHandler {
   // setting value in the screen UI.
   base::OneShotTimer a11y_nav_buttons_toggle_metrics_reporter_timer_;
 
+  base::WeakPtrFactory<AccessibilityHandler> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(AccessibilityHandler);
 };
 

chrome/browser/ui/webui/settings/chromeos/accessibility_handler_unittest.cc

+// Copyright 2020 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.
+
+#include "chrome/browser/ui/webui/settings/chromeos/accessibility_handler.h"
+
+#include <memory>
+
+#include "ash/public/cpp/test/test_tablet_mode.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/dbus/power/fake_power_manager_client.h"
+#include "content/public/test/test_web_ui.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace settings {
+
+class TestingAccessibilityHandler : public AccessibilityHandler {
+ public:
+  explicit TestingAccessibilityHandler(content::WebUI* web_ui)
+      : AccessibilityHandler(/* profile */ nullptr) {
+    set_web_ui(web_ui);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestingAccessibilityHandler);
+};
+
+class AccessibilityHandlerTest : public ChromeRenderViewHostTestHarness {
+ public:
+  AccessibilityHandlerTest() = default;
+  AccessibilityHandlerTest(const AccessibilityHandlerTest&) = delete;
+  AccessibilityHandlerTest& operator=(const AccessibilityHandlerTest&) = delete;
+
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    PowerManagerClient::InitializeFake();
+    test_tablet_mode_ = std::make_unique<ash::TestTabletMode>();
+    handler_ = std::make_unique<TestingAccessibilityHandler>(&web_ui_);
+  }
+
+  void TearDown() override {
+    handler_.reset();
+    test_tablet_mode_.reset();
+    PowerManagerClient::Shutdown();
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
+  content::TestWebUI web_ui_;
+  std::unique_ptr<ash::TestTabletMode> test_tablet_mode_;
+  std::unique_ptr<TestingAccessibilityHandler> handler_;
+};
+
+// Test that when tablet mode is supported, the correct data is returned by
+// HandleManageA11yPageReady().
+TEST_F(AccessibilityHandlerTest, ManageA11yPageReadyTabletModeSupported) {
+  // Set tablet mode as supported.
+  chromeos::FakePowerManagerClient::Get()->SetTabletMode(
+      chromeos::PowerManagerClient::TabletMode::OFF, base::TimeTicks());
+
+  handler_->HandleManageA11yPageReady(/* args */ nullptr);
+
+  // Wait for the AccessibilityHandler to receive data from PowerManagerClient.
+  base::RunLoop().RunUntilIdle();
+
+  const content::TestWebUI::CallData& call_data = *web_ui_.call_data().back();
+
+  // Ensure tablet mode is returned as supported.
+  EXPECT_TRUE(call_data.arg3()->GetBool());
+}
+
+// Test that when tablet mode is unsupported, the correct data is returned by
+// HandleManageA11yPageReady().
+TEST_F(AccessibilityHandlerTest, ManageA11yPageReadyTabletModeUnsupported) {
+  // Set tablet mode as unsupported.
+  chromeos::FakePowerManagerClient::Get()->SetTabletMode(
+      chromeos::PowerManagerClient::TabletMode::UNSUPPORTED, base::TimeTicks());
+  handler_->HandleManageA11yPageReady(/* args */ nullptr);
+
+  // Wait for the AccessibilityHandler to receive data from PowerManagerClient.
+  base::RunLoop().RunUntilIdle();
+
+  const content::TestWebUI::CallData& call_data = *web_ui_.call_data().back();
+
+  // Ensure tablet mode is returned as unsupported.
+  EXPECT_FALSE(call_data.arg3()->GetBool());
+}
+
+// Test that when tablet mode is enabled, the correct data is returned by
+// HandleManageA11yPageReady().
+TEST_F(AccessibilityHandlerTest, ManageA11yPageReadyTabletModeEnabled) {
+  test_tablet_mode_->SetEnabledForTest(true);
+  handler_->HandleManageA11yPageReady(/* args */ nullptr);
+
+  const content::TestWebUI::CallData& call_data = *web_ui_.call_data().back();
+
+  // Ensure tablet mode is returned as supported.
+  EXPECT_TRUE(call_data.arg3()->GetBool());
+}
+
+}  // namespace settings
+}  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.cc

@@ -268,7 +268,7 @@ void OSSettingsUI::InitOSWebUIHandlers(content::WebUIDataSource* html_source) {
       std::make_unique<chromeos::settings::ChangePictureHandler>());
 
   web_ui()->AddMessageHandler(
-      std::make_unique<chromeos::settings::AccessibilityHandler>(web_ui()));
+      std::make_unique<chromeos::settings::AccessibilityHandler>(profile));
   web_ui()->AddMessageHandler(
       std::make_unique<chromeos::settings::AndroidAppsHandler>(profile));
   if (crostini::CrostiniFeatures::Get()->IsUIAllowed(profile,