Fix Chrome OS virtual keyboard accessibility
The user-visible change here is that Select-to-speak on Chrome OS now
works on the virtual keyboard. It also makes it visible to ChromeVox
so we can make that work better too.
Three fixes were required:
* We need to always set the window property on the window for a
RenderWidgetHostViewAura, whether that WebContents already has
accessibility enabled or not. That's solved by moving the
trigger for the window property to RenderViewReady and
WebContentsImpl::NotifySwappedFromRenderManager.
* AXAuraObjCache needs to listen for newly-created windows,
changes to window bounds, and window property changes.
Otherwise the virtual keyboard window is created but no
accessibility events fire allowing automation clients to
find it.
* The bounds for a AXWindowObjWrapper should be global bounds and not
local. This wasn't caught before because widget bounds were correct,
but the virtual keyboard is directly in an aura Window, with no Widget
and no Views.
BUG=699617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/2803823002
Cr-Commit-Position: refs/heads/master@{#468182}
diff --git a/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc b/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc
index 1818c11..a687ecb2 100644
--- a/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc
+++ b/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc
@@ -108,6 +108,11 @@
SendEvent(nullptr, parent, ui::AX_EVENT_CHILDREN_CHANGED);
}
+void AutomationManagerAura::OnEvent(views::AXAuraObjWrapper* aura_obj,
+ ui::AXEvent event_type) {
+ SendEvent(nullptr, aura_obj, event_type);
+}
+
AutomationManagerAura::AutomationManagerAura()
: AXHostDelegate(extensions::api::automation::kDesktopTreeID),
enabled_(false),
diff --git a/chrome/browser/ui/aura/accessibility/automation_manager_aura.h b/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
index ecb3c60..c6ec8a1c 100644
--- a/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
+++ b/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
@@ -62,6 +62,8 @@
// views::AXAuraObjCache::Delegate implementation.
void OnChildWindowRemoved(views::AXAuraObjWrapper* parent) override;
+ void OnEvent(views::AXAuraObjWrapper* aura_obj,
+ ui::AXEvent event_type) override;
protected:
AutomationManagerAura();
@@ -70,6 +72,8 @@
private:
friend struct base::DefaultSingletonTraits<AutomationManagerAura>;
+ FRIEND_TEST_ALL_PREFIXES(AutomationManagerAuraBrowserTest, WebAppearsOnce);
+
// Reset state in this manager. If |reset_serializer| is true, reset the
// serializer to save memory.
void Reset(bool reset_serializer);
diff --git a/chrome/browser/ui/aura/accessibility/automation_manager_aura_browsertest.cc b/chrome/browser/ui/aura/accessibility/automation_manager_aura_browsertest.cc
new file mode 100644
index 0000000..ade6d07
--- /dev/null
+++ b/chrome/browser/ui/aura/accessibility/automation_manager_aura_browsertest.cc
@@ -0,0 +1,85 @@
+// Copyright 2017 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/aura/accessibility/automation_manager_aura.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/browser_accessibility_state.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "ui/accessibility/ax_node_data.h"
+
+namespace {
+
+// Given an AXTreeSourceAura and a node within that tree, recursively search
+// for all nodes who have a child tree id of |target_ax_tree_id|, meaning
+// that they're a parent of a particular web contents.
+void FindAllHostsOfWebContentsWithAXTreeID(
+ AXTreeSourceAura* tree,
+ views::AXAuraObjWrapper* node,
+ int target_ax_tree_id,
+ std::vector<views::AXAuraObjWrapper*>* web_hosts) {
+ ui::AXNodeData node_data;
+ tree->SerializeNode(node, &node_data);
+ if (node_data.GetIntAttribute(ui::AX_ATTR_CHILD_TREE_ID) ==
+ target_ax_tree_id) {
+ web_hosts->push_back(node);
+ }
+
+ std::vector<views::AXAuraObjWrapper*> children;
+ tree->GetChildren(node, &children);
+ for (auto* child : children) {
+ FindAllHostsOfWebContentsWithAXTreeID(tree, child, target_ax_tree_id,
+ web_hosts);
+ }
+}
+
+} // namespace
+
+typedef InProcessBrowserTest AutomationManagerAuraBrowserTest;
+
+// A WebContents can be "hooked up" to the Chrome OS Desktop accessibility
+// tree two different ways: via its aura::Window, and via a views::WebView.
+// This test makes sure that we don't hook up both simultaneously, leading
+// to the same web page appearing in the overall tree twice.
+IN_PROC_BROWSER_TEST_F(AutomationManagerAuraBrowserTest, WebAppearsOnce) {
+ content::BrowserAccessibilityState::GetInstance()->EnableAccessibility();
+
+ AutomationManagerAura* manager = AutomationManagerAura::GetInstance();
+ manager->Enable(browser()->profile());
+ auto* tree = manager->current_tree_.get();
+
+ ui_test_utils::NavigateToURL(
+ browser(),
+ GURL("data:text/html;charset=utf-8,<button autofocus>Click me</button>"));
+
+ auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+
+ WaitForAccessibilityTreeToContainNodeWithName(web_contents, "Click me");
+
+ auto* frame_host = web_contents->GetMainFrame();
+ int ax_tree_id = frame_host->GetAXTreeID();
+ ASSERT_GT(ax_tree_id, 0);
+
+ std::vector<views::AXAuraObjWrapper*> web_hosts;
+ FindAllHostsOfWebContentsWithAXTreeID(tree, tree->GetRoot(), ax_tree_id,
+ &web_hosts);
+
+ EXPECT_EQ(1U, web_hosts.size());
+ if (web_hosts.size() == 1) {
+ ui::AXNodeData node_data;
+ tree->SerializeNode(web_hosts[0], &node_data);
+ EXPECT_EQ(ui::AX_ROLE_WEB_VIEW, node_data.role);
+ } else {
+ for (size_t i = 0; i < web_hosts.size(); i++) {
+ ui::AXNodeData node_data;
+ tree->SerializeNode(web_hosts[i], &node_data);
+ LOG(ERROR) << i << ": " << node_data.ToString();
+ }
+ }
+}
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 5350de8..1c996cf 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -1945,6 +1945,9 @@
deps += [ ":test_support_applist_ash" ]
}
}
+ if (use_aura) {
+ sources += [ "../browser/ui/aura/accessibility/automation_manager_aura_browsertest.cc" ]
+ }
if (use_aura || toolkit_views) {
deps += [ "//ui/events:test_support" ]
}
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index d9be37f5..3e82e43 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -2167,10 +2167,6 @@
accessibility_reset_token_ = 0;
RenderWidgetHostViewBase* view = GetViewForAccessibility();
-
- if (frame_tree_node_->IsMainFrame() && view)
- view->SetMainFrameAXTreeID(GetAXTreeID());
-
AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode();
if (!accessibility_mode.is_mode_off() && view && is_active()) {
if (accessibility_mode.has_mode(AccessibilityMode::kNativeAPIs))
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 2888777..18572e8 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -4520,6 +4520,11 @@
return;
}
+ RenderWidgetHostViewBase* rwhv =
+ static_cast<RenderWidgetHostViewBase*>(GetRenderWidgetHostView());
+ if (rwhv)
+ rwhv->SetMainFrameAXTreeID(GetMainFrame()->GetAXTreeID());
+
notify_disconnection_ = true;
// TODO(avi): Remove. http://crbug.com/170921
NotificationService::current()->Notify(
@@ -5080,6 +5085,11 @@
view_->SetOverscrollControllerEnabled(CanOverscrollContent());
view_->RenderViewSwappedIn(new_host->GetRenderViewHost());
+
+ RenderWidgetHostViewBase* rwhv =
+ static_cast<RenderWidgetHostViewBase*>(GetRenderWidgetHostView());
+ if (rwhv)
+ rwhv->SetMainFrameAXTreeID(GetMainFrame()->GetAXTreeID());
}
NotifyFrameSwapped(old_host, new_host);
diff --git a/ui/views/accessibility/ax_aura_obj_cache.cc b/ui/views/accessibility/ax_aura_obj_cache.cc
index ad9df5c..cfb2cdf 100644
--- a/ui/views/accessibility/ax_aura_obj_cache.cc
+++ b/ui/views/accessibility/ax_aura_obj_cache.cc
@@ -34,6 +34,14 @@
AXAuraObjWrapper* AXAuraObjCache::GetOrCreate(aura::Window* window) {
if (!focus_client_) {
+ // Note: On Chrome OS, there's exactly one root window per screen,
+ // it's the same as ash::Shell::Get()->GetPrimaryRootWindow() when
+ // there's only one screen. Observing the root window allows us to
+ // detect any time a window is added or removed.
+ //
+ // TODO(dmazzoni): Explicitly observe each root window on Chrome OS
+ // and track as root windows are added and removed.
+ // http://crbug.com/713278
aura::Window* root_window = window->GetRootWindow();
if (root_window) {
focus_client_ = aura::client::GetFocusClient(root_window);
@@ -124,6 +132,12 @@
view->NotifyAccessibilityEvent(ui::AX_EVENT_FOCUS, true);
}
+void AXAuraObjCache::FireEvent(AXAuraObjWrapper* aura_obj,
+ ui::AXEvent event_type) {
+ if (delegate_)
+ delegate_->OnEvent(aura_obj, event_type);
+}
+
AXAuraObjCache::AXAuraObjCache()
: current_id_(1),
focus_client_(nullptr),
@@ -181,6 +195,15 @@
focus_client_ = nullptr;
}
+void AXAuraObjCache::OnWindowHierarchyChanged(
+ const HierarchyChangeParams& params) {
+ aura::Window* window = params.target;
+ if (window->parent()) {
+ delegate_->OnEvent(GetOrCreate(window->parent()),
+ ui::AX_EVENT_CHILDREN_CHANGED);
+ }
+}
+
template <typename AuraViewWrapper, typename AuraView>
AXAuraObjWrapper* AXAuraObjCache::CreateInternal(
AuraView* aura_view,
diff --git a/ui/views/accessibility/ax_aura_obj_cache.h b/ui/views/accessibility/ax_aura_obj_cache.h
index c73abe1a..6ab3a53 100644
--- a/ui/views/accessibility/ax_aura_obj_cache.h
+++ b/ui/views/accessibility/ax_aura_obj_cache.h
@@ -12,6 +12,7 @@
#include <vector>
#include "base/macros.h"
+#include "ui/accessibility/ax_enums.h"
#include "ui/aura/client/focus_change_observer.h"
#include "ui/aura/window_observer.h"
#include "ui/views/views_export.h"
@@ -43,6 +44,8 @@
class Delegate {
public:
virtual void OnChildWindowRemoved(AXAuraObjWrapper* parent) = 0;
+ virtual void OnEvent(AXAuraObjWrapper* aura_obj,
+ ui::AXEvent event_type) = 0;
};
// Get or create an entry in the cache based on an Aura view.
@@ -85,6 +88,9 @@
// Send a notification that the focused view may have changed.
void OnFocusedViewChanged();
+ // Tell our delegate to fire an event on a given object.
+ void FireEvent(AXAuraObjWrapper* aura_obj, ui::AXEvent event_type);
+
// Indicates if this object's currently being destroyed.
bool is_destroying() { return is_destroying_; }
@@ -104,6 +110,7 @@
// aura::WindowObserver override.
void OnWindowDestroying(aura::Window* window) override;
+ void OnWindowHierarchyChanged(const HierarchyChangeParams& params) override;
template <typename AuraViewWrapper, typename AuraView>
AXAuraObjWrapper* CreateInternal(
diff --git a/ui/views/accessibility/ax_window_obj_wrapper.cc b/ui/views/accessibility/ax_window_obj_wrapper.cc
index 4352db01..0f08a3c 100644
--- a/ui/views/accessibility/ax_window_obj_wrapper.cc
+++ b/ui/views/accessibility/ax_window_obj_wrapper.cc
@@ -54,12 +54,24 @@
out_node_data->AddStringAttribute(ui::AX_ATTR_NAME,
base::UTF16ToUTF8(window_->GetTitle()));
out_node_data->state = 0;
- out_node_data->location = gfx::RectF(window_->bounds());
+ out_node_data->location = gfx::RectF(window_->GetBoundsInScreen());
ui::AXTreeIDRegistry::AXTreeID child_ax_tree_id =
window_->GetProperty(ui::kChildAXTreeID);
- if (child_ax_tree_id != ui::AXTreeIDRegistry::kNoAXTreeID)
+ if (child_ax_tree_id != ui::AXTreeIDRegistry::kNoAXTreeID) {
+ // Most often, child AX trees are parented to Views. We need to handle
+ // the case where they're not here, but we don't want the same AX tree
+ // to be a child of two different parents.
+ //
+ // To avoid this double-parenting, only add the child tree ID of this
+ // window if the top-level window doesn't have an associated Widget.
+ if (!window_->GetToplevelWindow() ||
+ Widget::GetWidgetForNativeView(window_->GetToplevelWindow())) {
+ return;
+ }
+
out_node_data->AddIntAttribute(ui::AX_ATTR_CHILD_TREE_ID, child_ax_tree_id);
+ }
}
int32_t AXWindowObjWrapper::GetID() {
@@ -85,6 +97,11 @@
void AXWindowObjWrapper::OnWindowBoundsChanged(aura::Window* window,
const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) {
+ if (window != window_)
+ return;
+
+ AXAuraObjCache::GetInstance()->FireEvent(this, ui::AX_EVENT_LOCATION_CHANGED);
+
Widget* widget = Widget::GetWidgetForNativeView(window);
if (widget) {
widget->GetRootView()->NotifyAccessibilityEvent(
@@ -92,4 +109,13 @@
}
}
+void AXWindowObjWrapper::OnWindowPropertyChanged(aura::Window* window,
+ const void* key,
+ intptr_t old) {
+ if (window == window_ && key == ui::kChildAXTreeID) {
+ AXAuraObjCache::GetInstance()->FireEvent(this,
+ ui::AX_EVENT_CHILDREN_CHANGED);
+ }
+}
+
} // namespace views
diff --git a/ui/views/accessibility/ax_window_obj_wrapper.h b/ui/views/accessibility/ax_window_obj_wrapper.h
index 5508905..b47913d 100644
--- a/ui/views/accessibility/ax_window_obj_wrapper.h
+++ b/ui/views/accessibility/ax_window_obj_wrapper.h
@@ -43,6 +43,9 @@
void OnWindowBoundsChanged(aura::Window* window,
const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) override;
+ void OnWindowPropertyChanged(aura::Window* window,
+ const void* key,
+ intptr_t old) override;
private:
aura::Window* window_;
