Commit faa9754c authored by Yuwei Huang's avatar Yuwei Huang Committed by Commit Bot

[Fullscreen control] Add timeout for cursor triggered exit UI

Some users complain that the fullscreen exit UI (FAB X) doesn't go away
when they move their cursor to the top of the screen. This is a common
scenario where people play video or present slides and they just want
to keep their cursor on the top.

To fix this use case, this CL adds a 3s timeout to the exit UI. After
the UI times out, the user needs to move the cursor away from the
buffer area then move it back to the top to re-trigger the UI.

Bug: 835223
Change-Id: I43a652d83ffb539a89f9a0bcb81987c62d439edf
Reviewed-on: https://chromium-review.googlesource.com/1033483
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555241}
parent c42a9c98
......@@ -31,6 +31,13 @@ namespace {
// | | before closing the fullscreen exit
// | | control.
// +----------------------------+
//
// The same value is also used for timeout cooldown.
// This is a common scenario where people play video or present slides and they
// just want to keep their cursor on the top. In this case we timeout the exit
// control so that it doesn't show permanently. The user will then need to move
// the cursor out of the cooldown area and move it back to the top to re-trigger
// the exit UI.
constexpr float kExitHeightScaleFactor = 1.5f;
// +----------------------------+
......@@ -44,8 +51,8 @@ constexpr float kExitHeightScaleFactor = 1.5f;
// +----------------------------+
constexpr float kShowFullscreenExitControlHeight = 3.f;
// Time to wait to hide the popup when it is triggered by touch input.
constexpr base::TimeDelta kTouchPopupTimeout = base::TimeDelta::FromSeconds(5);
// Time to wait to hide the popup after it is triggered.
constexpr base::TimeDelta kPopupTimeout = base::TimeDelta::FromSeconds(3);
// Time to wait before showing the popup when the escape key is held.
constexpr base::TimeDelta kKeyPressPopupDelay = base::TimeDelta::FromSeconds(1);
......@@ -57,10 +64,10 @@ FullscreenControlHost::FullscreenControlHost(BrowserView* browser_view,
: browser_view_(browser_view),
fullscreen_control_popup_(
browser_view->GetBubbleParentView(),
base::Bind(&BrowserView::ExitFullscreen,
base::Unretained(browser_view)),
base::Bind(&FullscreenControlHost::OnVisibilityChanged,
base::Unretained(this))) {}
base::BindRepeating(&BrowserView::ExitFullscreen,
base::Unretained(browser_view)),
base::BindRepeating(&FullscreenControlHost::OnVisibilityChanged,
base::Unretained(this))) {}
FullscreenControlHost::~FullscreenControlHost() = default;
......@@ -114,14 +121,18 @@ void FullscreenControlHost::OnMouseEvent(ui::MouseEvent* event) {
if (IsExitUiNeeded()) {
if (IsVisible()) {
float control_bottom = static_cast<float>(
fullscreen_control_popup_.GetFinalBounds().bottom());
float y_limit = control_bottom * kExitHeightScaleFactor;
if (event->y() >= y_limit)
if (event->y() >= CalculateCursorBufferHeight()) {
Hide(true);
}
} else {
if (event->y() <= kShowFullscreenExitControlHeight)
DCHECK_EQ(InputEntryMethod::NOT_ACTIVE, input_entry_method_);
if (!in_mouse_cooldown_mode_ &&
event->y() <= kShowFullscreenExitControlHeight) {
ShowForInputEntryMethod(InputEntryMethod::MOUSE);
} else if (in_mouse_cooldown_mode_ &&
event->y() >= CalculateCursorBufferHeight()) {
in_mouse_cooldown_mode_ = false;
}
}
} else if (IsVisible()) {
Hide(true);
......@@ -140,10 +151,7 @@ void FullscreenControlHost::OnTouchEvent(ui::TouchEvent* event) {
!fullscreen_control_popup_.IsAnimating()) {
Hide(true);
} else if (event->type() == ui::ET_TOUCH_RELEASED) {
touch_timeout_timer_.Start(
FROM_HERE, kTouchPopupTimeout,
base::Bind(&FullscreenControlHost::OnTouchPopupTimeout,
base::Unretained(this)));
StartPopupTimeout(InputEntryMethod::TOUCH);
}
}
......@@ -169,22 +177,38 @@ void FullscreenControlHost::ShowForInputEntryMethod(
if (bubble)
bubble->HideImmediately();
fullscreen_control_popup_.Show(browser_view_->GetClientAreaBoundsInScreen());
// Exit cooldown mode in case the exit UI is triggered by a different method.
in_mouse_cooldown_mode_ = false;
}
void FullscreenControlHost::OnVisibilityChanged() {
if (!IsVisible()) {
input_entry_method_ = InputEntryMethod::NOT_ACTIVE;
key_press_delay_timer_.Stop();
} else if (input_entry_method_ == InputEntryMethod::MOUSE) {
StartPopupTimeout(InputEntryMethod::MOUSE);
}
if (on_popup_visibility_changed_)
std::move(on_popup_visibility_changed_).Run();
}
void FullscreenControlHost::OnTouchPopupTimeout() {
void FullscreenControlHost::StartPopupTimeout(
InputEntryMethod expected_input_method) {
popup_timeout_timer_.Start(
FROM_HERE, kPopupTimeout,
base::BindRepeating(&FullscreenControlHost::OnPopupTimeout,
base::Unretained(this), expected_input_method));
}
void FullscreenControlHost::OnPopupTimeout(
InputEntryMethod expected_input_method) {
if (IsVisible() && !fullscreen_control_popup_.IsAnimating() &&
input_entry_method_ == InputEntryMethod::TOUCH) {
Hide(true);
input_entry_method_ == expected_input_method) {
if (input_entry_method_ == InputEntryMethod::MOUSE)
in_mouse_cooldown_mode_ = true;
fullscreen_control_popup_.Hide(true);
}
}
......@@ -192,3 +216,10 @@ bool FullscreenControlHost::IsExitUiNeeded() {
return browser_view_->IsFullscreen() &&
browser_view_->ShouldHideUIForFullscreen();
}
float FullscreenControlHost::CalculateCursorBufferHeight() const {
float control_bottom = FullscreenControlPopup::GetButtonBottomOffset() +
browser_view_->GetClientAreaBoundsInScreen().y();
DCHECK_GT(control_bottom, 0);
return control_bottom * kExitHeightScaleFactor;
}
......@@ -61,15 +61,19 @@ class FullscreenControlHost : public ui::EventHandler {
void ShowForInputEntryMethod(InputEntryMethod input_entry_method);
void OnVisibilityChanged();
void OnTouchPopupTimeout();
void StartPopupTimeout(InputEntryMethod expected_input_method);
void OnPopupTimeout(InputEntryMethod expected_input_method);
bool IsExitUiNeeded();
float CalculateCursorBufferHeight() const;
InputEntryMethod input_entry_method_ = InputEntryMethod::NOT_ACTIVE;
bool in_mouse_cooldown_mode_ = false;
BrowserView* const browser_view_;
FullscreenControlPopup fullscreen_control_popup_;
base::OneShotTimer touch_timeout_timer_;
base::OneShotTimer popup_timeout_timer_;
base::OneShotTimer key_press_delay_timer_;
// Used to allow tests to wait for popup visibility changes.
......
......@@ -51,6 +51,11 @@ FullscreenControlPopup::FullscreenControlPopup(
FullscreenControlPopup::~FullscreenControlPopup() {}
// static
int FullscreenControlPopup::GetButtonBottomOffset() {
return kFinalOffset + FullscreenControlView::kCircleButtonDiameter;
}
void FullscreenControlPopup::Show(const gfx::Rect& parent_bounds_in_screen) {
if (IsVisible())
return;
......@@ -64,8 +69,6 @@ void FullscreenControlPopup::Show(const gfx::Rect& parent_bounds_in_screen) {
// to prevent potential flickering.
AnimationProgressed(animation_.get());
popup_->Show();
OnVisibilityChanged();
}
void FullscreenControlPopup::Hide(bool animated) {
......@@ -82,10 +85,6 @@ void FullscreenControlPopup::Hide(bool animated) {
AnimationEnded(animation_.get());
}
gfx::Rect FullscreenControlPopup::GetFinalBounds() const {
return CalculateBounds(kFinalOffset);
}
views::Widget* FullscreenControlPopup::GetPopupWidget() {
return popup_.get();
}
......@@ -118,10 +117,10 @@ void FullscreenControlPopup::AnimationEnded(const gfx::Animation* animation) {
// It's the end of the reversed animation. Just hide the popup in this case.
parent_bounds_in_screen_ = gfx::Rect();
popup_->Hide();
OnVisibilityChanged();
return;
} else {
AnimationProgressed(animation);
}
AnimationProgressed(animation);
OnVisibilityChanged();
}
gfx::Rect FullscreenControlPopup::CalculateBounds(int y_offset) const {
......
......@@ -29,6 +29,9 @@ class FullscreenControlPopup : public gfx::AnimationDelegate {
const base::RepeatingClosure& on_visibility_changed);
~FullscreenControlPopup() override;
// Returns the final bottom of the button as a y offset to its parent view.
static int GetButtonBottomOffset();
// Shows the indicator with an animation that drops it off the top of
// |parent_view|.
// |parent_bounds_in_screen| holds the bounds of |parent_view| in the screen
......@@ -40,10 +43,6 @@ class FullscreenControlPopup : public gfx::AnimationDelegate {
// |parent_widget|.
void Hide(bool animated);
// Returns the final bounds of the control view when it is fully shown.
// Returns empty bounds if the popup is not visible.
gfx::Rect GetFinalBounds() const;
views::Widget* GetPopupWidget();
gfx::SlideAnimation* GetAnimationForTesting();
......
......@@ -79,13 +79,13 @@ TEST_F(FullscreenControlPopupTest, ShowPopupAnimated) {
// started.
EXPECT_GT(GetParentBounds().y(), GetPopupBounds().y());
gfx::Rect final_bounds = popup_->GetFinalBounds();
EXPECT_LT(GetParentBounds().y(), final_bounds.y());
CompleteAnimation();
EXPECT_FALSE(popup_->IsAnimating());
EXPECT_TRUE(popup_->IsVisible());
EXPECT_EQ(final_bounds, GetPopupBounds());
int final_bottom =
FullscreenControlPopup::GetButtonBottomOffset() + GetParentBounds().y();
EXPECT_EQ(final_bottom, GetPopupBounds().bottom());
}
TEST_F(FullscreenControlPopupTest, HidePopupWhileStillShowing) {
......
......@@ -29,7 +29,6 @@ namespace {
const SkColor kBackgroundColor = SkColorSetARGB(0xcc, 0x28, 0x2c, 0x32);
constexpr int kCloseIconSize = 24;
constexpr int kCircleButtonDiameter = 48;
class CloseFullscreenButton : public views::Button {
public:
......@@ -51,7 +50,7 @@ class CloseFullscreenButton : public views::Button {
flags.setAntiAlias(true);
flags.setColor(kBackgroundColor);
flags.setStyle(cc::PaintFlags::kFill_Style);
float radius = kCircleButtonDiameter / 2.0f;
float radius = FullscreenControlView::kCircleButtonDiameter / 2.0f;
canvas->DrawCircle(gfx::PointF(radius, radius), radius, flags);
}
......
......@@ -21,6 +21,8 @@ class FullscreenControlView : public views::View,
const base::RepeatingClosure& on_button_pressed);
~FullscreenControlView() override;
static constexpr int kCircleButtonDiameter = 48;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
......
......@@ -106,6 +106,18 @@ class FullscreenControlViewTest : public InProcessBrowserTest {
std::move(callback);
}
base::OneShotTimer* GetPopupTimeoutTimer() {
return &GetFullscreenControlHost()->popup_timeout_timer_;
}
void RunLoopUntilVisibilityChanges() {
base::RunLoop run_loop;
SetPopupVisibilityChangedCallback(run_loop.QuitClosure());
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(), kPopupEventTimeout);
run_loop.Run();
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
......@@ -146,6 +158,68 @@ IN_PROC_BROWSER_TEST_F(FullscreenControlViewTest, MAYBE_MouseExitFullscreen) {
ASSERT_FALSE(browser_view->IsFullscreen());
}
#if defined(OS_MACOSX)
// Entering fullscreen is flaky on Mac: http://crbug.com/824517
#define MAYBE_MouseExitFullscreen_TimeoutAndRetrigger \
DISABLED_MouseExitFullscreen_TimeoutAndRetrigger
#else
#define MAYBE_MouseExitFullscreen_TimeoutAndRetrigger \
MouseExitFullscreen_TimeoutAndRetrigger
#endif
IN_PROC_BROWSER_TEST_F(FullscreenControlViewTest,
MAYBE_MouseExitFullscreen_TimeoutAndRetrigger) {
EnterActiveTabFullscreen();
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
ASSERT_TRUE(browser_view->IsFullscreen());
FullscreenControlHost* host = GetFullscreenControlHost();
host->Hide(false);
ASSERT_FALSE(host->IsVisible());
// Simulate moving the mouse to the top of the screen, which should show the
// fullscreen exit UI.
ui::MouseEvent mouse_move(ui::ET_MOUSE_MOVED, gfx::Point(1, 1), gfx::Point(),
base::TimeTicks(), 0, 0);
host->OnMouseEvent(&mouse_move);
ASSERT_TRUE(host->IsVisible());
// Wait until popup times out. This is one wait for show and one wait for
// hide.
RunLoopUntilVisibilityChanges();
RunLoopUntilVisibilityChanges();
ASSERT_FALSE(host->IsVisible());
// Simulate moving the mouse to the top again. This should not show the exit
// UI.
mouse_move = ui::MouseEvent(ui::ET_MOUSE_MOVED, gfx::Point(2, 1),
gfx::Point(), base::TimeTicks(), 0, 0);
host->OnMouseEvent(&mouse_move);
ASSERT_FALSE(host->IsVisible());
// Simulate moving the mouse out of the buffer area. This resets the state.
mouse_move = ui::MouseEvent(ui::ET_MOUSE_MOVED, gfx::Point(2, 1000),
gfx::Point(), base::TimeTicks(), 0, 0);
host->OnMouseEvent(&mouse_move);
ASSERT_FALSE(host->IsVisible());
// Simulate moving the mouse to the top again, which should show the exit UI.
mouse_move = ui::MouseEvent(ui::ET_MOUSE_MOVED, gfx::Point(1, 1),
gfx::Point(), base::TimeTicks(), 0, 0);
host->OnMouseEvent(&mouse_move);
RunLoopUntilVisibilityChanges();
ASSERT_TRUE(host->IsVisible());
// Simulate immediately moving the mouse out of the buffer area. This should
// hide the exit UI.
mouse_move = ui::MouseEvent(ui::ET_MOUSE_MOVED, gfx::Point(2, 1000),
gfx::Point(), base::TimeTicks(), 0, 0);
host->OnMouseEvent(&mouse_move);
RunLoopUntilVisibilityChanges();
ASSERT_FALSE(host->IsVisible());
ASSERT_TRUE(browser_view->IsFullscreen());
}
#if defined(OS_MACOSX)
// Entering fullscreen is flaky on Mac: http://crbug.com/824517
#define MAYBE_KeyboardPopupInteraction DISABLED_KeyboardPopupInteraction
......@@ -154,6 +228,10 @@ IN_PROC_BROWSER_TEST_F(FullscreenControlViewTest, MAYBE_MouseExitFullscreen) {
#endif
IN_PROC_BROWSER_TEST_F(FullscreenControlViewTest,
MAYBE_KeyboardPopupInteraction) {
// TODO(yuweih): Test will fail if the cursor is at the top of the screen.
// That's because random mouse move events may pass from the browser to the
// host and interfere with the InputEntryMethod tracking. Consider fixing
// this.
EnterActiveTabFullscreen();
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
ASSERT_TRUE(browser_view->IsFullscreen());
......
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