Commit 769799b8 authored by James Cook's avatar James Cook Committed by Commit Bot

cros: Make ChromeVox panel work under mash

For mash, the ash window manager runs outside the browser process.
Remove direct access to ash::Shell from ChromeVoxPanel.

* Init the panel widget using mus properties
* Introduce mojo API for setting whether the panel should be fullscreen
* Remove mash checks from AccessibilityManager
* Close ChromeVoxPanel immediately on shutdown and during reloads
by session state changes and multi-profile user switching. This helps
prevent having multiple panels open during shutdown, especially in
tests.

ChromeVox doesn't read things yet under mash, but at least the panel
opens now.

Bug: 628655
Test: existing ash_unittests, browser_tests
Change-Id: I14cbf946809787791115930ad33c1c1d3357bf88
Reviewed-on: https://chromium-review.googlesource.com/956235Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542513}
parent 7be4df95
...@@ -325,19 +325,6 @@ void AccessibilityController::NotifyAccessibilityStatusChanged( ...@@ -325,19 +325,6 @@ void AccessibilityController::NotifyAccessibilityStatusChanged(
observer.OnAccessibilityStatusChanged(notify); observer.OnAccessibilityStatusChanged(notify);
} }
void AccessibilityController::SetAccessibilityPanelFullscreen(bool fullscreen) {
// The accessibility panel is only shown on the primary display.
aura::Window* root = Shell::GetPrimaryRootWindow();
aura::Window* container =
Shell::GetContainer(root, kShellWindowId_AccessibilityPanelContainer);
// TODO(jamescook): Avoid this cast by moving ash::AccessibilityObserver
// ownership to this class and notifying it on ChromeVox fullscreen updates.
AccessibilityPanelLayoutManager* layout =
static_cast<AccessibilityPanelLayoutManager*>(
container->layout_manager());
layout->SetPanelFullscreen(fullscreen);
}
void AccessibilityController::SetClient( void AccessibilityController::SetClient(
mojom::AccessibilityControllerClientPtr client) { mojom::AccessibilityControllerClientPtr client) {
client_ = std::move(client); client_ = std::move(client);
...@@ -366,6 +353,19 @@ void AccessibilityController::SetFocusHighlightRect( ...@@ -366,6 +353,19 @@ void AccessibilityController::SetFocusHighlightRect(
accessibility_highlight_controller_->SetFocusHighlightRect(bounds_in_screen); accessibility_highlight_controller_->SetFocusHighlightRect(bounds_in_screen);
} }
void AccessibilityController::SetAccessibilityPanelFullscreen(bool fullscreen) {
// The accessibility panel is only shown on the primary display.
aura::Window* root = Shell::GetPrimaryRootWindow();
aura::Window* container =
Shell::GetContainer(root, kShellWindowId_AccessibilityPanelContainer);
// TODO(jamescook): Avoid this cast by moving ash::AccessibilityObserver
// ownership to this class and notifying it on ChromeVox fullscreen updates.
AccessibilityPanelLayoutManager* layout =
static_cast<AccessibilityPanelLayoutManager*>(
container->layout_manager());
layout->SetPanelFullscreen(fullscreen);
}
void AccessibilityController::OnSigninScreenPrefServiceInitialized( void AccessibilityController::OnSigninScreenPrefServiceInitialized(
PrefService* prefs) { PrefService* prefs) {
ObservePrefs(prefs); ObservePrefs(prefs);
......
...@@ -129,10 +129,6 @@ class ASH_EXPORT AccessibilityController ...@@ -129,10 +129,6 @@ class ASH_EXPORT AccessibilityController
// countdown. // countdown.
void PlaySpokenFeedbackToggleCountdown(int tick_count); void PlaySpokenFeedbackToggleCountdown(int tick_count);
// Sets whether the accessibility panel is filling the entire screen.
// TODO(jamescook): Convert to mojo interface.
void SetAccessibilityPanelFullscreen(bool fullscreen);
// Public because a11y features like screen magnifier and tap dragging are // Public because a11y features like screen magnifier and tap dragging are
// managed outside of this controller. // managed outside of this controller.
void NotifyAccessibilityStatusChanged( void NotifyAccessibilityStatusChanged(
...@@ -143,6 +139,7 @@ class ASH_EXPORT AccessibilityController ...@@ -143,6 +139,7 @@ class ASH_EXPORT AccessibilityController
void SetDarkenScreen(bool darken) override; void SetDarkenScreen(bool darken) override;
void BrailleDisplayStateChanged(bool connected) override; void BrailleDisplayStateChanged(bool connected) override;
void SetFocusHighlightRect(const gfx::Rect& bounds_in_screen) override; void SetFocusHighlightRect(const gfx::Rect& bounds_in_screen) override;
void SetAccessibilityPanelFullscreen(bool fullscreen) override;
// SessionObserver: // SessionObserver:
void OnSigninScreenPrefServiceInitialized(PrefService* prefs) override; void OnSigninScreenPrefServiceInitialized(PrefService* prefs) override;
......
...@@ -51,6 +51,10 @@ interface AccessibilityController { ...@@ -51,6 +51,10 @@ interface AccessibilityController {
// Sets the focus highlight rect using |bounds_in_screen|. // Sets the focus highlight rect using |bounds_in_screen|.
SetFocusHighlightRect(gfx.mojom.Rect bounds_in_screen); SetFocusHighlightRect(gfx.mojom.Rect bounds_in_screen);
// Sets whether the accessibility panel is filling the entire screen (e.g. to
// show the expanded UI for the ChromeVox spoken feedback extension).
SetAccessibilityPanelFullscreen(bool fullscreen);
}; };
// Interface for ash to request accessibility service from its client (e.g. // Interface for ash to request accessibility service from its client (e.g.
......
...@@ -2,6 +2,8 @@ include_rules = [ ...@@ -2,6 +2,8 @@ include_rules = [
# TODO(crbug.com/678705): Convert to using mojo interfaces in # TODO(crbug.com/678705): Convert to using mojo interfaces in
# //ash/public/interfaces and eliminate this. # //ash/public/interfaces and eliminate this.
"!ash", "!ash",
# Public interfaces are OK.
"+ash/public",
# TODO(ananta): Remove this when we move files which display UI in # TODO(ananta): Remove this when we move files which display UI in
# chrome/browser/chromeos to chrome/browser/ui/views/chromeos # chrome/browser/chromeos to chrome/browser/ui/views/chromeos
......
...@@ -328,7 +328,7 @@ AccessibilityManager::~AccessibilityManager() { ...@@ -328,7 +328,7 @@ AccessibilityManager::~AccessibilityManager() {
session_manager::SessionManager::Get()->RemoveObserver(this); session_manager::SessionManager::Get()->RemoveObserver(this);
if (chromevox_panel_) { if (chromevox_panel_) {
chromevox_panel_->Close(); chromevox_panel_->CloseNow();
chromevox_panel_ = nullptr; chromevox_panel_ = nullptr;
} }
} }
...@@ -1254,8 +1254,7 @@ void AccessibilityManager::PostLoadChromeVox() { ...@@ -1254,8 +1254,7 @@ void AccessibilityManager::PostLoadChromeVox() {
event_router->DispatchEventWithLazyListener( event_router->DispatchEventWithLazyListener(
extension_misc::kChromeVoxExtensionId, std::move(event)); extension_misc::kChromeVoxExtensionId, std::move(event));
// TODO(mash): Support ChromeVoxPanel. http://crbug.com/628655 if (!chromevox_panel_) {
if (!chromevox_panel_ && chromeos::GetAshConfig() != ash::Config::MASH) {
chromevox_panel_ = new ChromeVoxPanel( chromevox_panel_ = new ChromeVoxPanel(
profile_, profile_,
session_manager::SessionManager::Get()->IsUserSessionBlocked()); session_manager::SessionManager::Get()->IsUserSessionBlocked());
...@@ -1296,12 +1295,8 @@ void AccessibilityManager::PostSwitchChromeVoxProfile() { ...@@ -1296,12 +1295,8 @@ void AccessibilityManager::PostSwitchChromeVoxProfile() {
} }
void AccessibilityManager::ReloadChromeVoxPanel() { void AccessibilityManager::ReloadChromeVoxPanel() {
// TODO(mash): Support ChromeVoxPanel. http://crbug.com/628655
if (chromeos::GetAshConfig() == ash::Config::MASH)
return;
if (chromevox_panel_) { if (chromevox_panel_) {
chromevox_panel_->Close(); chromevox_panel_->CloseNow();
chromevox_panel_ = nullptr; chromevox_panel_ = nullptr;
} }
chromevox_panel_ = new ChromeVoxPanel( chromevox_panel_ = new ChromeVoxPanel(
......
...@@ -767,7 +767,7 @@ IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeLoginTest, ...@@ -767,7 +767,7 @@ IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeLoginTest,
EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelay()); EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelay());
EXPECT_TRUE(IsMonoAudioEnabled()); EXPECT_TRUE(IsMonoAudioEnabled());
session_manager::SessionManager::Get()->SessionStarted(); StartUserSession(GetParam());
// Confirms that the features keep enabled after session starts. // Confirms that the features keep enabled after session starts.
EXPECT_TRUE(IsLargeCursorEnabled()); EXPECT_TRUE(IsLargeCursorEnabled());
......
...@@ -4,17 +4,25 @@ ...@@ -4,17 +4,25 @@
#include "chrome/browser/chromeos/accessibility/chromevox_panel.h" #include "chrome/browser/chromeos/accessibility/chromevox_panel.h"
#include "ash/accessibility/accessibility_controller.h"
#include "ash/public/cpp/shell_window_ids.h" #include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h" #include "ash/public/interfaces/accessibility_controller.mojom.h"
#include "ash/public/interfaces/constants.mojom.h"
#include "ash/shell.h" // mash-ok
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/chromeos/accessibility/accessibility_manager.h" #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
#include "chrome/browser/chromeos/ash_config.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/data_use_measurement/data_use_web_contents_observer.h" #include "chrome/browser/data_use_measurement/data_use_web_contents_observer.h"
#include "chrome/browser/extensions/chrome_extension_web_contents_observer.h" #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_constants.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/view_type_utils.h" #include "extensions/browser/view_type_utils.h"
#include "mojo/public/cpp/bindings/type_converter.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/ui/public/cpp/property_type_converters.h"
#include "services/ui/public/interfaces/window_manager.mojom.h"
#include "ui/display/display.h"
#include "ui/display/screen.h" #include "ui/display/screen.h"
#include "ui/views/controls/webview/webview.h" #include "ui/views/controls/webview/webview.h"
#include "ui/views/layout/fill_layout.h" #include "ui/views/layout/fill_layout.h"
...@@ -96,17 +104,36 @@ ChromeVoxPanel::ChromeVoxPanel(content::BrowserContext* browser_context, ...@@ -96,17 +104,36 @@ ChromeVoxPanel::ChromeVoxPanel(content::BrowserContext* browser_context,
widget_ = new views::Widget(); widget_ = new views::Widget();
views::Widget::InitParams params( views::Widget::InitParams params(
views::Widget::InitParams::TYPE_WINDOW_FRAMELESS); views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
aura::Window* root_window = ash::Shell::GetPrimaryRootWindow(); // Placing the panel in the accessibility panel container allows ash to manage
params.parent = ash::Shell::GetContainer( // both the window bounds and display work area.
root_window, ash::kShellWindowId_AccessibilityPanelContainer); const int container_id = ash::kShellWindowId_AccessibilityPanelContainer;
const display::Display primary_display =
display::Screen::GetScreen()->GetPrimaryDisplay();
if (chromeos::GetAshConfig() == ash::Config::MASH) {
using ui::mojom::WindowManager;
params.mus_properties[WindowManager::kContainerId_InitProperty] =
mojo::ConvertTo<std::vector<uint8_t>>(container_id);
params.mus_properties[WindowManager::kDisplayId_InitProperty] =
mojo::ConvertTo<std::vector<uint8_t>>(primary_display.id());
} else {
params.parent = ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
container_id);
}
params.bounds = primary_display.bounds();
params.delegate = this; params.delegate = this;
params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; params.activatable = views::Widget::InitParams::ACTIVATABLE_NO;
params.bounds = gfx::Rect(0, 0, root_window->bounds().width(),
root_window->bounds().height());
params.name = "ChromeVoxPanel"; params.name = "ChromeVoxPanel";
widget_->Init(params); widget_->Init(params);
wm::SetShadowElevation(widget_->GetNativeWindow(), wm::SetShadowElevation(widget_->GetNativeWindow(),
wm::kShadowElevationInactiveWindow); wm::kShadowElevationInactiveWindow);
// WebContentsObserver::DidFirstVisuallyNonEmptyPaint is not called under
// mash. Work around this by showing the window immediately.
// TODO(jamescook|fsamuel): Fix this. It causes a white flash when opening the
// window. The underlying problem is FrameToken plumbing, see
// ui::ws::ServerWindow::OnFrameTokenChanged. https://crbug.com/771331
if (chromeos::GetAshConfig() == ash::Config::MASH)
widget_->Show();
} }
ChromeVoxPanel::~ChromeVoxPanel() = default; ChromeVoxPanel::~ChromeVoxPanel() = default;
...@@ -115,10 +142,13 @@ aura::Window* ChromeVoxPanel::GetRootWindow() { ...@@ -115,10 +142,13 @@ aura::Window* ChromeVoxPanel::GetRootWindow() {
return GetWidget()->GetNativeWindow()->GetRootWindow(); return GetWidget()->GetNativeWindow()->GetRootWindow();
} }
void ChromeVoxPanel::CloseNow() {
widget_->CloseNow();
}
void ChromeVoxPanel::Close() { void ChromeVoxPanel::Close() {
// NOTE: CloseNow() does not work here due to a use-after-free where // NOTE: Close the widget asynchronously because it's not legal to delete
// WebContentsImpl accesses a deleted RenderFrameHost. It's not clear if it's // a WebView/WebContents during a DidFinishNavigation callback.
// legal to close a WebContents during DidFinishNavigation.
widget_->Close(); widget_->Close();
} }
...@@ -144,19 +174,13 @@ void ChromeVoxPanel::DidFirstVisuallyNonEmptyPaint() { ...@@ -144,19 +174,13 @@ void ChromeVoxPanel::DidFirstVisuallyNonEmptyPaint() {
void ChromeVoxPanel::EnterFullscreen() { void ChromeVoxPanel::EnterFullscreen() {
Focus(); Focus();
// TODO(jamescook): Convert to mojo. SetAccessibilityPanelFullscreen(true);
ash::Shell::Get()
->accessibility_controller()
->SetAccessibilityPanelFullscreen(true);
} }
void ChromeVoxPanel::ExitFullscreen() { void ChromeVoxPanel::ExitFullscreen() {
widget_->Deactivate(); widget_->Deactivate();
widget_->widget_delegate()->set_can_activate(false); widget_->widget_delegate()->set_can_activate(false);
// TODO(jamescook): Convert to mojo. SetAccessibilityPanelFullscreen(false);
ash::Shell::Get()
->accessibility_controller()
->SetAccessibilityPanelFullscreen(false);
} }
void ChromeVoxPanel::DisableSpokenFeedback() { void ChromeVoxPanel::DisableSpokenFeedback() {
...@@ -170,3 +194,11 @@ void ChromeVoxPanel::Focus() { ...@@ -170,3 +194,11 @@ void ChromeVoxPanel::Focus() {
web_view_->RequestFocus(); web_view_->RequestFocus();
} }
void ChromeVoxPanel::SetAccessibilityPanelFullscreen(bool fullscreen) {
// Connect to the accessibility mojo interface in ash.
ash::mojom::AccessibilityControllerPtr accessibility_controller;
content::ServiceManagerConnection::GetForProcess()
->GetConnector()
->BindInterface(ash::mojom::kServiceName, &accessibility_controller);
accessibility_controller->SetAccessibilityPanelFullscreen(fullscreen);
}
...@@ -28,6 +28,10 @@ class ChromeVoxPanel : public views::WidgetDelegate { ...@@ -28,6 +28,10 @@ class ChromeVoxPanel : public views::WidgetDelegate {
aura::Window* GetRootWindow(); aura::Window* GetRootWindow();
// Closes the panel immediately, deleting the WebView/WebContents.
void CloseNow();
// Closes the panel asynchronously.
void Close(); void Close();
// WidgetDelegate overrides. // WidgetDelegate overrides.
...@@ -48,6 +52,9 @@ class ChromeVoxPanel : public views::WidgetDelegate { ...@@ -48,6 +52,9 @@ class ChromeVoxPanel : public views::WidgetDelegate {
void DisableSpokenFeedback(); void DisableSpokenFeedback();
void Focus(); void Focus();
// Sends a request to the ash window manager.
void SetAccessibilityPanelFullscreen(bool fullscreen);
views::Widget* widget_; views::Widget* widget_;
std::unique_ptr<ChromeVoxPanelWebContentsObserver> web_contents_observer_; std::unique_ptr<ChromeVoxPanelWebContentsObserver> web_contents_observer_;
views::View* web_view_; views::View* web_view_;
......
...@@ -37,6 +37,7 @@ class TestAccessibilityController : ash::mojom::AccessibilityController { ...@@ -37,6 +37,7 @@ class TestAccessibilityController : ash::mojom::AccessibilityController {
void SetDarkenScreen(bool darken) override {} void SetDarkenScreen(bool darken) override {}
void BrailleDisplayStateChanged(bool connected) override {} void BrailleDisplayStateChanged(bool connected) override {}
void SetFocusHighlightRect(const gfx::Rect& bounds_in_screen) override {} void SetFocusHighlightRect(const gfx::Rect& bounds_in_screen) override {}
void SetAccessibilityPanelFullscreen(bool fullscreen) override {}
bool was_client_set() const { return was_client_set_; } bool was_client_set() const { return was_client_set_; }
......
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