Commit 12e96721 authored by Ahmed Fakhry's avatar Ahmed Fakhry Committed by Commit Bot

(Reland) top-chrome-slide: Disable the behavior when Chromevox is enabled.

Users of Chromevox may need to touch explore the webpage, and for
things to remain consistent, we should disable hiding top-chrome
with gesture scrolls as long as Chromevox is enabled.

R=sky@chromium.org
BUG=857331, 896514
TEST=Added a new browser test.

Reviewed-on: https://chromium-review.googlesource.com/c/1284289Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600408}
Change-Id: I388bc5056f956bea2edffc7127dcab533824e56b
Reviewed-on: https://chromium-review.googlesource.com/c/1287510
Cr-Commit-Position: refs/heads/master@{#600966}
parent e60b2e25
......@@ -37,6 +37,13 @@ bool IsTabletModeEnabled() {
TabletModeClient::Get()->tablet_mode_enabled();
}
bool IsSpokenFeedbackEnabled() {
chromeos::AccessibilityManager* accessibility_manager =
chromeos::AccessibilityManager::Get();
return accessibility_manager &&
accessibility_manager->IsSpokenFeedbackEnabled();
}
// Based on the current status of |contents|, returns the browser top controls
// shown state constraints, which specifies if the top controls are allowed to
// be only shown, or either shown or hidden.
......@@ -275,7 +282,16 @@ TopControlsSlideControllerChromeOS::TopControlsSlideControllerChromeOS(
browser_view_->browser()->tab_strip_model()->AddObserver(this);
OnEnabledStateChanged(IsTabletModeEnabled() && !browser_view->IsFullscreen());
chromeos::AccessibilityManager* accessibility_manager =
chromeos::AccessibilityManager::Get();
if (accessibility_manager) {
accessibility_status_subscription_ =
accessibility_manager->RegisterCallback(base::BindRepeating(
&TopControlsSlideControllerChromeOS::OnAccessibilityStatusChanged,
base::Unretained(this)));
}
OnEnabledStateChanged(CanEnable(base::nullopt));
}
TopControlsSlideControllerChromeOS::~TopControlsSlideControllerChromeOS() {
......@@ -342,14 +358,13 @@ void TopControlsSlideControllerChromeOS::SetShownRatio(
defer_disabling_ = false;
// Don't just set |is_enabled_| to false. Make sure it's a correct value.
OnEnabledStateChanged(IsTabletModeEnabled() &&
!browser_view_->IsFullscreen());
OnEnabledStateChanged(CanEnable(base::nullopt));
}
}
void TopControlsSlideControllerChromeOS::OnBrowserFullscreenStateWillChange(
bool new_fullscreen_state) {
OnEnabledStateChanged(IsTabletModeEnabled() && !new_fullscreen_state);
OnEnabledStateChanged(CanEnable(new_fullscreen_state));
}
bool TopControlsSlideControllerChromeOS::DoBrowserControlsShrinkRendererSize(
......@@ -391,7 +406,7 @@ bool TopControlsSlideControllerChromeOS::IsTopControlsGestureScrollInProgress()
void TopControlsSlideControllerChromeOS::OnTabletModeToggled(
bool tablet_mode_enabled) {
OnEnabledStateChanged(tablet_mode_enabled && !browser_view_->IsFullscreen());
OnEnabledStateChanged(CanEnable(base::nullopt));
}
void TopControlsSlideControllerChromeOS::TabInsertedAt(
......@@ -477,6 +492,24 @@ void TopControlsSlideControllerChromeOS::Observe(
UpdateBrowserControlsStateShown(active_contents, true /* animate */);
}
bool TopControlsSlideControllerChromeOS::CanEnable(
base::Optional<bool> fullscreen_state) const {
return IsTabletModeEnabled() &&
!(fullscreen_state ? *fullscreen_state
: browser_view_->IsFullscreen()) &&
!IsSpokenFeedbackEnabled();
}
void TopControlsSlideControllerChromeOS::OnAccessibilityStatusChanged(
const chromeos::AccessibilityStatusEventDetails& event_details) {
if (event_details.notification_type !=
chromeos::ACCESSIBILITY_TOGGLE_SPOKEN_FEEDBACK) {
return;
}
OnEnabledStateChanged(CanEnable(base::nullopt));
}
void TopControlsSlideControllerChromeOS::OnEnabledStateChanged(bool new_state) {
if (new_state == is_enabled_)
return;
......
......@@ -9,6 +9,8 @@
#include "base/containers/flat_map.h"
#include "base/macros.h"
#include "base/optional.h"
#include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
#include "chrome/browser/ui/ash/tablet_mode_client_observer.h"
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#include "chrome/browser/ui/views/frame/top_controls_slide_controller.h"
......@@ -82,6 +84,21 @@ class TopControlsSlideControllerChromeOS
const content::NotificationDetails& details) override;
private:
// Returns true if this feature can be turned on. If |fullscreen_state| is
// supplied, it will be used in calculating the result, otherwise the current
// fullscreen state will be queried from BrowserView. This is needed since
// BrowserView informs us with fullscreen state changes before they happen
// (See OnBrowserFullscreenStateWillChange()) so that we can disable the
// sliding behavior *before* immersive mode is entered.
bool CanEnable(base::Optional<bool> fullscreen_state) const;
// Called back from the AccessibilityManager so that we're updated by the
// status of Chromevox, which when enabled, sliding the top-controls should
// be disabled. This is important for users who want to touch explore and need
// this to be consistent.
void OnAccessibilityStatusChanged(
const chromeos::AccessibilityStatusEventDetails& event_details);
void OnEnabledStateChanged(bool new_state);
// Refreshes the status of the browser top controls.
......@@ -148,6 +165,9 @@ class TopControlsSlideControllerChromeOS
content::NotificationRegistrar registrar_;
std::unique_ptr<chromeos::AccessibilityStatusSubscription>
accessibility_status_subscription_;
DISALLOW_COPY_AND_ASSIGN(TopControlsSlideControllerChromeOS);
};
......
......@@ -18,6 +18,7 @@
#include "base/strings/safe_sprintf.h"
#include "base/test/scoped_feature_list.h"
#include "cc/base/math_util.h"
#include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
#include "chrome/browser/ui/ash/tablet_mode_client.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
......@@ -1008,4 +1009,78 @@ IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest,
CheckBrowserLayout(browser_view(), TopChromeShownState::kFullyShown);
}
// Waits for a compositor frame to be drawn and committed on the given
// web_contents.
class CompositorFrameWaiter : content::WebContentsObserver {
public:
explicit CompositorFrameWaiter(content::WebContents* contents)
: WebContentsObserver(contents) {}
~CompositorFrameWaiter() override = default;
void Wait() { run_loop_.Run(); }
// content::WebContentsObserver:
void DidCommitAndDrawCompositorFrame() override { run_loop_.Quit(); }
private:
base::RunLoop run_loop_;
DISALLOW_COPY_AND_ASSIGN(CompositorFrameWaiter);
};
IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest, TestToggleChromeVox) {
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
EXPECT_TRUE(top_controls_slide_controller()->IsEnabled());
EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 1.f);
OpenUrlAtIndex(embedded_test_server()->GetURL("/top_controls_scroll.html"),
0);
content::WebContents* active_contents =
browser_view()->GetActiveWebContents();
PageStateUpdateWaiter page_state_update_waiter(active_contents);
page_state_update_waiter.Wait();
EXPECT_TRUE(
browser_view()->DoBrowserControlsShrinkRendererSize(active_contents));
aura::Window* browser_window = browser()->window()->GetNativeWindow();
ui::test::EventGenerator event_generator(browser_window->GetRootWindow(),
browser_window);
const gfx::Point start_point = event_generator.current_location();
const gfx::Point end_point = start_point + gfx::Vector2d(0, -100);
GenerateGestureFlingScrollSequence(&event_generator, start_point, end_point);
TopControlsShownRatioWaiter waiter(top_controls_slide_controller());
waiter.WaitForRatio(0.f);
EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 0);
CheckBrowserLayout(browser_view(), TopChromeShownState::kFullyHidden);
// Enable Chromevox (spoken feedback) and expect that top-chrome will be fully
// shown, and sliding top-chrome is no longer enabled.
chromeos::AccessibilityManager::Get()->EnableSpokenFeedback(true);
EXPECT_TRUE(chromeos::AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
waiter.WaitForRatio(1.f);
EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 1.f);
EXPECT_FALSE(top_controls_slide_controller()->IsEnabled());
EXPECT_EQ(browser_view()->GetTopControlsHeight(), 0);
EXPECT_FALSE(
browser_view()->DoBrowserControlsShrinkRendererSize(active_contents));
// Now disable Chromevox, and expect it's now possible to hide top-chrome with
// gesture scrolling.
CompositorFrameWaiter compositor_frame_waiter(active_contents);
chromeos::AccessibilityManager::Get()->EnableSpokenFeedback(false);
compositor_frame_waiter.Wait();
EXPECT_FALSE(
chromeos::AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 1.f);
EXPECT_TRUE(top_controls_slide_controller()->IsEnabled());
CheckBrowserLayout(browser_view(), TopChromeShownState::kFullyShown);
base::RunLoop().RunUntilIdle();
// Scroll down, top-chrome should hide.
GenerateGestureFlingScrollSequence(&event_generator, start_point, end_point);
waiter.WaitForRatio(0.f);
EXPECT_FLOAT_EQ(top_controls_slide_controller()->GetShownRatio(), 0);
CheckBrowserLayout(browser_view(), TopChromeShownState::kFullyHidden);
}
} // namespace
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment