Issue metadata
Sign in to add a comment
|
Crash when select-to-speak clicks on shortcut viewer window on high-dpi display |
||||||||||||||||||||||||
Issue description
ToT, linux-chromeos
out/Debug/chrome --ash-host-window-bounds="0+0-1400x1000*2" --user-data-dir=/w/udd --no-startup-window
* Turn on STS
* Ctrl-Alt-/ for shortcut viewer
* Click STS icon in shelf
* Click on shortcut viewer
Infinite recursion in AutomationManagerAura:
...
#10883 0x000055555d8ba66e in AutomationManagerAura::PerformAction (this=0x3c9f9c593b60, data=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:144
#10884 0x000055555d8ba981 in AutomationManagerAura::PerformHitTest (this=0x3c9f9c593b60, original_action=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:282
#10885 0x000055555d8ba66e in AutomationManagerAura::PerformAction (this=0x3c9f9c593b60, data=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:144
#10886 0x000055555d8ba981 in AutomationManagerAura::PerformHitTest (this=0x3c9f9c593b60, original_action=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:282
#10887 0x000055555d8ba66e in AutomationManagerAura::PerformAction (this=0x3c9f9c593b60, data=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:144
#10888 0x000055555d8ba981 in AutomationManagerAura::PerformHitTest (this=0x3c9f9c593b60, original_action=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:282
#10889 0x000055555d8ba66e in AutomationManagerAura::PerformAction (this=0x3c9f9c593b60, data=...) at ../../chrome/browser/ui/aura/accessibility/automation_manager_aura.cc:144
#10890 0x000055555fad59d0 in extensions::AutomationInternalPerformActionFunction::Run (this=0x3c9f9ea7dda0) at ../../chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:487
#10891 0x00005555566b9d30 in ExtensionFunction::RunWithValidation (this=0x3c9f9ea7dda0) at ../../extensions/browser/extension_function.cc:451
#10892 0x00005555566bf032 in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int, base::RepeatingCallback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, extensions::functions::HistogramValue)> const&) (
this=0x3c9f9eaff820, params=..., render_frame_host=0x3c9f9daef020, render_process_id=8, callback=...) at ../../extensions/browser/extension_function_dispatcher.cc:486
#10893 0x00005555566be05c in extensions::ExtensionFunctionDispatcher::Dispatch (this=0x3c9f9eaff820, params=..., render_frame_host=0x3c9f9daef020, render_process_id=8)
at ../../extensions/browser/extension_function_dispatcher.cc:380
#10894 0x000055555672a9bd in extensions::ExtensionWebContentsObserver::OnRequest (this=0x3c9f9eaff800, render_frame_host=0x3c9f9daef020, params=...)
at ../../extensions/browser/extension_web_contents_observer.cc:301
I think this might be an event targeting issue. The AutomationManagerAura is-a AXHostDelegate for AXTree == 0. This code seems problematic:
if (ash::Shell::HasRemoteClient(window)) {
// For remote mojo apps, the |window| is a DesktopNativeWidgetAura, so the
// parent is the widget and the widget's contents view has the child tree.
CHECK(window->parent());
views::Widget* widget =
views::Widget::GetWidgetForNativeWindow(window->parent());
CHECK(widget);
ui::AXNodeData node_data;
widget->GetContentsView()->GetAccessibleNodeData(&node_data);
child_ax_tree_id =
node_data.GetIntAttribute(ax::mojom::IntAttribute::kChildTreeId);
} else {
// For normal windows the (optional) child tree is an aura window property.
child_ax_tree_id = window->GetProperty(ui::kChildAXTreeID);
}
// If the window has a child AX tree ID, forward the action to the
// associated AXHostDelegate or RenderFrameHost.
if (child_ax_tree_id != ui::AXTreeIDRegistry::kNoAXTreeID) {
ui::AXTreeIDRegistry* registry = ui::AXTreeIDRegistry::GetInstance();
ui::AXHostDelegate* delegate = registry->GetHostDelegate(child_ax_tree_id);
if (delegate) {
delegate->PerformAction(action);
return;
}
In particular, node_data.GetIntAttribute() will return 0 if there's no kChildTreeId, but 0 == the desktop tree node.
katie / msw - ideas?
,
Aug 10
I found it. AutomationManagerAura is using the wrong "GetContentsView" method, hence not finding the AXTreeId attribute at all. CL up shortly.
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95ce2318e03cc0ffd2058755f75ca38f8f802f3a commit 95ce2318e03cc0ffd2058755f75ca38f8f802f3a Author: James Cook <jamescook@chromium.org> Date: Tue Aug 14 19:42:43 2018 chromeos: Fix crash in select-to-speak for keyboard shortcut dialog The keyboard shortcut dialog is a UI mojo app that runs in a separate process and has its own AXTree. The code in AutomationManagerAura that looks up the AXTree ID for a hit test was looking at the wrong views::View to get the value, due to a confusing condition in how views::Widget looks up contents views. Added DCHECKs to make sure this looks up a proper remote AXTree id. Bug: 872421 Test: Turn on select-to-speak, hit Ctrl-Alt-/, click in dialog Change-Id: Id9aba72b1aa73b7677e0dc52d1a427c202db0a78 Reviewed-on: https://chromium-review.googlesource.com/1171261 Reviewed-by: Katie Dektar <katie@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#582998} [modify] https://crrev.com/95ce2318e03cc0ffd2058755f75ca38f8f802f3a/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc
,
Aug 14
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by katie@chromium.org
, Aug 8At first glance it seems like getIntAttribute() should return -1 (kNoAXTreeID) if there's no ID. Is this an issue with sparely populating the tree? It appears AXNodeData::GetIntAttribute returns 0 if nothing is found, so: A fix could be: 1. Use another version of GetIntAttribute which takes a default value so that 0 isn't the default 2. Check HasIntAttribute to make sure there is a child tree before getting the attribute 3. Populate the child tree ID with kNoAXTreeID for every node 1 or 2 sounds reasonable to me? Then the one line could become something like: child_ax_tree_id = node_data.HasIntAttribute(ax::mojom::IntAttribute::kChildTreeId) ? node_data.GetIntAttribute(ax::mojom::IntAttribute::kChildTreeId) : ui::AXTreeIdRegistry::kNoAxTreeId; What do you think?