Commit 8d9e0bcb authored by Aya ElAttar's avatar Aya ElAttar Committed by Commit Bot

[Locked Fullscreen] Fix the vox panel inconsistency with the fullscreen mode.


Upon entering the fullscreen/locked-fullscreen while the chromeVox panel is working, You can see an empty bar in the top of the screen.
1. The fullscreen rect y dimension starts after the height of the chromeVox panel, and as we don't want the chromeVox panel to be displayed, its height should be removed from the calculations.

UPDATE: This issue got fixed by another colleague.
So I added a unittest, and removed two workarounds were added to fix this issue in the locked-fullscreen mode.
(one was to disable chromevox panel upon starting the lockedscreen mode, and the other was to hide the vox panel specifically for the locked fullscreen mode)

When a window is set to fullscreen screen_util::GetFullscreenWindowBoundsInParent & AccessibilityPanelLayoutManager::UpdateWindowBounds() are called twice with this stacktraces order:

[23272:23272:0723/164119.757770:ERROR:screen_util.cc(47)] ayaaa #0 0x0000081162bc base::debug::CollectStackTrace()
#1 0x0000080a6c88 base::debug::StackTrace::StackTrace()
#2 0x000009a8b57c ash::screen_util::GetFullscreenWindowBoundsInParent()
#3 0x000009b93e62 ash::wm::DefaultState::UpdateBoundsFromState()
#4 0x000009b93da4 ash::wm::DefaultState::EnterToNextState()
#5 0x000009b93c2e ash::wm::DefaultState::HandleTransitionEvents()
#6 0x000009b92e3e ash::wm::WindowState::OnWindowPropertyChanged()
#7 0x000008d085f6 aura::Window::AfterPropertyChange()
#8 0x0000082c62fc ui::PropertyHandler::SetPropertyInternal()
#9 0x000008d0324c ui::PropertyHandler::SetProperty<>()
#10 0x000009268956 views::Widget::SetFullscreen()
#11 0x00000a088c28 BrowserView::ProcessFullscreen()
#12 0x000009f48638 FullscreenController::EnterFullscreenModeInternal()
#13 0x000009f47c98 FullscreenController::ToggleFullscreenModeInternal()
#14 0x000009f47bba FullscreenController::ToggleBrowserFullscreenMode()
#15 0x00000a0b45a8 (anonymous namespace)::BrowserWindowStateDelegate::ToggleFullscreen()
#16 0x000009b93fc0 ash::wm::ToggleFullScreen()
#17 0x000009b9177e ash::wm::WindowState::OnWMEvent()
#18 0x000009a153ca ash::accelerators::ToggleFullscreen()
#19 0x000009a13f72 ash::(anonymous namespace)::HandleToggleFullscreen()
#20 0x000009a12332 ash::AcceleratorControllerImpl::AcceleratorPressed()
#21 0x000009253988 ui::AcceleratorManager::Process()
#22 0x000009ac225a ash::PreTargetAcceleratorHandler::ProcessAccelerator()
....
#80 0x0000eae970a2 __libc_start_main

[23272:23272:0723/164121.271202:ERROR:accessibility_panel_layout_manager.cc(103)] ayaaa #0 0x0000081162bc base::debug::CollectStackTrace()
#1 0x0000080a6c88 base::debug::StackTrace::StackTrace()
#2 0x000009a190fa ash::AccessibilityPanelLayoutManager::UpdateWindowBounds()
#3 0x0000099ebd12 display::DisplayManager::NotifyMetricsChanged()
#4 0x0000099ebf5a display::DisplayManager::UpdateWorkAreaOfDisplay()
#5 0x000009a33610 ash::WindowTreeHostManager::UpdateWorkAreaOfDisplayNearestWindow()
#6 0x000009a94ec6 ash::ShelfLayoutManager::UpdateBoundsAndOpacity()
#7 0x000009a93680 ash::ShelfLayoutManager::SetState()
#8 0x000009a932be ash::ShelfLayoutManager::UpdateVisibilityState()
#9 0x000009b96428 ash::WorkspaceLayoutManager::SetChildBounds()
#10 0x000009b8f6e4 ash::wm::SetBoundsInScreen()
#11 0x000009276346 views::NativeWidgetAura::SetBounds()
#12 0x000009225b72 views::BubbleDialogDelegateView::SizeToContents()
#13 0x000009269980 views::Widget::OnNativeWidgetMove()
#14 0x000009276cd4 views::NativeWidgetAura::OnBoundsChanged()
#15 0x00000a0b4332 BrowserFrameAsh::OnBoundsChanged()
#16 0x000008d0978c aura::Window::OnLayerBoundsChanged()
#17 0x000008d19a6e ui::Layer::SetBoundsFromAnimation()
#18 0x000008d0730c aura::Window::SetBoundsInternal()
#19 0x000009b929a6 ash::wm::WindowState::SetBoundsDirect()
#20 0x000009b93f0e ash::wm::DefaultState::UpdateBoundsFromState()
#21 0x000009b93da4 ash::wm::DefaultState::EnterToNextState()
#22 0x000009b93c2e ash::wm::DefaultState::HandleTransitionEvents()
#23 0x000009b92e3e ash::wm::WindowState::OnWindowPropertyChanged()
#24 0x000008d085f6 aura::Window::AfterPropertyChange()
#25 0x0000082c62fc ui::PropertyHandler::SetPropertyInternal()
#26 0x000008d0324c ui::PropertyHandler::SetProperty<>()
#27 0x000009268956 views::Widget::SetFullscreen()
#28 0x00000a088c28 BrowserView::ProcessFullscreen()
#29 0x000009f48638 FullscreenController::EnterFullscreenModeInternal()
#30 0x000009f47c98 FullscreenController::ToggleFullscreenModeInternal()
#31 0x000009f47bba FullscreenController::ToggleBrowserFullscreenMode()
#32 0x00000a0b45a8 (anonymous namespace)::BrowserWindowStateDelegate::ToggleFullscreen()
#33 0x000009b93fc0 ash::wm::ToggleFullScreen()
#34 0x000009b9177e ash::wm::WindowState::OnWMEvent()
#35 0x000009a153ca ash::accelerators::ToggleFullscreen()
#36 0x000009a13f72 ash::(anonymous namespace)::HandleToggleFullscreen()
#37 0x000009a12332 ash::AcceleratorControllerImpl::AcceleratorPressed()
#38 0x000009253988 ui::AcceleratorManager::Process()
#39 0x000009ac225a ash::PreTargetAcceleratorHandler::ProcessAccelerator()
#40 0x000009ab4ebe wm::AcceleratorFilter::OnKeyEvent()
....
#97 0x0000eae970a2 __libc_start_main

[23272:23272:0723/164123.029099:ERROR:screen_util.cc(47)] ayaaa #0 0x0000081162bc base::debug::CollectStackTrace()
#1 0x0000080a6c88 base::debug::StackTrace::StackTrace()
#2 0x000009a8b57c ash::screen_util::GetFullscreenWindowBoundsInParent()
#3 0x000009b93764 ash::wm::DefaultState::SetMaximizedOrFullscreenBounds()
#4 0x000009b9367a ash::wm::DefaultState::HandleWorkspaceEvents()
#5 0x000009b5ac5e ash::wm::BaseState::OnWMEvent()
#6 0x000009b9177e ash::wm::WindowState::OnWMEvent()
#7 0x000009b968e6 ash::WorkspaceLayoutManager::AdjustAllWindowsBoundsForWorkAreaChange()
#8 0x000009b96af8 ash::WorkspaceLayoutManager::OnDisplayMetricsChanged()
#9 0x0000099ebd12 display::DisplayManager::NotifyMetricsChanged()
#10 0x0000099ebf5a display::DisplayManager::UpdateWorkAreaOfDisplay()
#11 0x000009a33610 ash::WindowTreeHostManager::UpdateWorkAreaOfDisplayNearestWindow()
#12 0x000009a94ec6 ash::ShelfLayoutManager::UpdateBoundsAndOpacity()
#13 0x000009a93680 ash::ShelfLayoutManager::SetState()
#14 0x000009a932be ash::ShelfLayoutManager::UpdateVisibilityState()
#15 0x000009b96428 ash::WorkspaceLayoutManager::SetChildBounds()
#16 0x000009b8f6e4 ash::wm::SetBoundsInScreen()
#17 0x000009276346 views::NativeWidgetAura::SetBounds()
#18 0x000009225b72 views::BubbleDialogDelegateView::SizeToContents()
#19 0x000009269980 views::Widget::OnNativeWidgetMove()
#20 0x000009276cd4 views::NativeWidgetAura::OnBoundsChanged()
#21 0x00000a0b4332 BrowserFrameAsh::OnBoundsChanged()
#22 0x000008d0978c aura::Window::OnLayerBoundsChanged()
#23 0x000008d19a6e ui::Layer::SetBoundsFromAnimation()
#24 0x000008d0730c aura::Window::SetBoundsInternal()
#25 0x000009b929a6 ash::wm::WindowState::SetBoundsDirect()
#26 0x000009b93f0e ash::wm::DefaultState::UpdateBoundsFromState()
#27 0x000009b93da4 ash::wm::DefaultState::EnterToNextState()
#28 0x000009b93c2e ash::wm::DefaultState::HandleTransitionEvents()
#29 0x000009b92e3e ash::wm::WindowState::OnWindowPropertyChanged()
#30 0x000008d085f6 aura::Window::AfterPropertyChange()
#31 0x0000082c62fc ui::PropertyHandler::SetPropertyInternal()
#32 0x000008d0324c ui::PropertyHandler::SetProperty<>()
#33 0x000009268956 views::Widget::SetFullscreen()
#34 0x00000a088c28 BrowserView::ProcessFullscreen()
#35 0x000009f48638 FullscreenController::EnterFullscreenModeInternal()
#36 0x000009f47c98 FullscreenController::ToggleFullscreenModeInternal()
#37 0x000009f47bba FullscreenController::ToggleBrowserFullscreenMode()
#38 0x00000a0b45a8 (anonymous namespace)::BrowserWindowStateDelegate::ToggleFullscreen()
#39 0x000009b93fc0 ash::wm::ToggleFullScreen()
#40 0x000009b9177e ash::wm::WindowState::OnWMEvent()
#41 0x000009a153ca ash::accelerators::ToggleFullscreen()
#42 0x000009a13f72 ash::(anonymous namespace)::HandleToggleFullscreen()
#43 0x000009a12332 ash::AcceleratorControllerImpl::AcceleratorPressed()
#44 0x000009253988 ui::AcceleratorManager::Process()
#45 0x000009ac225a ash::PreTargetAcceleratorHandler::ProcessAccelerator()
#46 0x000009ab4ebe wm::AcceleratorFilter::OnKeyEvent()
....
#103 0x0000eae970a2 __libc_start_main

[23272:23272:0723/164124.932334:ERROR:accessibility_panel_layout_manager.cc(103)] ayaaa #0 0x0000081162bc base::debug::CollectStackTrace()
#1 0x0000080a6c88 base::debug::StackTrace::StackTrace()
#2 0x000009a190fa ash::AccessibilityPanelLayoutManager::UpdateWindowBounds()
#3 0x000009ab0dea ash::Shell::NotifyFullscreenStateChanged()
#4 0x000009b96a4c ash::WorkspaceLayoutManager::OnPostWindowStateTypeChange()
#5 0x000009b92738 ash::wm::WindowState::NotifyPostStateTypeChange()
#6 0x000009b93ddc ash::wm::DefaultState::EnterToNextState()
#7 0x000009b93c2e ash::wm::DefaultState::HandleTransitionEvents()
#8 0x000009b92e3e ash::wm::WindowState::OnWindowPropertyChanged()
#9 0x000008d085f6 aura::Window::AfterPropertyChange()
#10 0x0000082c62fc ui::PropertyHandler::SetPropertyInternal()
#11 0x000008d0324c ui::PropertyHandler::SetProperty<>()
#12 0x000009268956 views::Widget::SetFullscreen()
#13 0x00000a088c28 BrowserView::ProcessFullscreen()
#14 0x000009f48638 FullscreenController::EnterFullscreenModeInternal()
#15 0x000009f47c98 FullscreenController::ToggleFullscreenModeInternal()
#16 0x000009f47bba FullscreenController::ToggleBrowserFullscreenMode()
#17 0x00000a0b45a8 (anonymous namespace)::BrowserWindowStateDelegate::ToggleFullscreen()
#18 0x000009b93fc0 ash::wm::ToggleFullScreen()
#19 0x000009b9177e ash::wm::WindowState::OnWMEvent()
#20 0x000009a153ca ash::accelerators::ToggleFullscreen()
#21 0x000009a13f72 ash::(anonymous namespace)::HandleToggleFullscreen()
#22 0x000009a12332 ash::AcceleratorControllerImpl::AcceleratorPressed()
#23 0x000009253988 ui::AcceleratorManager::Process()
#24 0x000009ac225a ash::PreTargetAcceleratorHandler::ProcessAccelerator()
#25 0x000009ab4ebe wm::AcceleratorFilter::OnKeyEvent()
...
#82 0x0000eae970a2 __libc_start_main

Bug: 945794,959786
Change-Id: If04f70f2c71563e9660c18ca942ef4b798ebcd3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702312
Commit-Queue: Aya Elsayed <ayaelattar@google.com>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Reviewed-by: default avatarAga Wronska <agawronska@chromium.org>
Reviewed-by: default avatarIvan Šandrk <isandrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683631}
parent 9b6a00d3
......@@ -112,13 +112,6 @@ void AccessibilityPanelLayoutManager::UpdateWindowBounds() {
} else if (panel_state_ == AccessibilityPanelState::FULL_WIDTH) {
bounds.set_x(0);
bounds.set_width(root_window->bounds().width());
// TODO(isandrk, crbug.com/959786): Temporary fix that prevents ChromeVox
// panel from showing up in locked fullscreen mode (the panel was enabling
// an escape from locked mode crbug.com/957950). Remove once a more proper
// fix exists.
if (Shell::Get()->screen_pinning_controller()->IsPinned())
bounds.set_height(0);
}
// If a fullscreen browser window is open, give the panel a height of 0
......
......@@ -6,6 +6,8 @@
#include <memory>
#include "ash/accessibility/accessibility_controller_impl.h"
#include "ash/accessibility/accessibility_panel_layout_manager.h"
#include "ash/magnifier/docked_magnifier_controller_impl.h"
#include "ash/root_window_controller.h"
#include "ash/shelf/shelf.h"
......@@ -26,6 +28,33 @@
namespace ash {
namespace {
AccessibilityPanelLayoutManager* GetLayoutManager() {
aura::Window* container =
Shell::GetContainer(Shell::GetPrimaryRootWindow(),
kShellWindowId_AccessibilityPanelContainer);
return static_cast<AccessibilityPanelLayoutManager*>(
container->layout_manager());
}
// Simulates Chrome creating the ChromeVoxPanel widget.
std::unique_ptr<views::Widget> CreateChromeVoxPanel() {
std::unique_ptr<views::Widget> widget = std::make_unique<views::Widget>();
views::Widget::InitParams params(
views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
aura::Window* root_window = Shell::GetPrimaryRootWindow();
params.parent = Shell::GetContainer(
root_window, kShellWindowId_AccessibilityPanelContainer);
params.activatable = views::Widget::InitParams::ACTIVATABLE_NO;
params.bounds = gfx::Rect(root_window->bounds().size());
widget->Init(std::move(params));
return widget;
}
} // namespace
using ScreenUtilTest = AshTestBase;
TEST_F(ScreenUtilTest, Bounds) {
......@@ -250,4 +279,28 @@ TEST_F(ScreenUtilTest, FullscreenWindowBoundsWithDockedMagnifier) {
EXPECT_EQ(expected_bounds, window->bounds());
}
// Tests that making a window fullscreen while the chromevox is enabled won't
// create an empty bar instead of the chromevox panel, but the fullscreened
// window will take the whole screen size.
TEST_F(ScreenUtilTest, FullscreenWindowBoundsWithChromeVox) {
UpdateDisplay("1366x768");
// Create ChromeVox Panel
AccessibilityPanelLayoutManager* layout_manager = GetLayoutManager();
std::unique_ptr<views::Widget> widget = CreateChromeVoxPanel();
widget->Show();
layout_manager->SetPanelBounds(
gfx::Rect(0, 0, 0, AccessibilityPanelLayoutManager::kDefaultPanelHeight),
AccessibilityPanelState::FULL_WIDTH);
std::unique_ptr<aura::Window> window = CreateToplevelTestWindow(
gfx::Rect(300, 300, 200, 150), desks_util::GetActiveDeskContainerId());
const WMEvent event(WM_EVENT_TOGGLE_FULLSCREEN);
WindowState::Get(window.get())->OnWMEvent(&event);
constexpr gfx::Rect kDisplayBounds{1366, 768};
EXPECT_EQ(window->bounds(), kDisplayBounds);
}
} // namespace ash
......@@ -27,14 +27,6 @@ namespace tabs_util {
void SetLockedFullscreenState(Browser* browser, bool locked) {
UMA_HISTOGRAM_BOOLEAN("Extensions.LockedFullscreenStateRequest", locked);
// Disable ChromeVox before entering locked fullscreen. Quickfix for
// crbug.com/957950.
auto* const accessibility_manager = chromeos::AccessibilityManager::Get();
if (locked && accessibility_manager &&
accessibility_manager->IsSpokenFeedbackEnabled()) {
accessibility_manager->EnableSpokenFeedback(false);
}
aura::Window* window = browser->window()->GetNativeWindow();
// TRUSTED_PINNED is used here because that one locks the window fullscreen
// without allowing the user to exit (as opposed to regular PINNED).
......
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