New issue
Advanced search Search tips

Issue 871414 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

Destroy AXTree when remote mojo app closes

Project Member Reported by jamescook@chromium.org, Aug 6

Issue description

From discussion with dtseng@ on https://chromium-review.googlesource.com/c/chromium/src/+/1162520

> > A side question: is the remote tree currently exposed on the extension as an AXTree or is it part of the desktop tree? If the former, I don't see a call to destroy the tree itself (should go through AutomationEventRouter::GetInstance()->DispatchTreeDestroyedEvent  in the Chrome process).
> 
> I think it's part of the desktop tree, though I'm not very familiar with the distinction. I set ax::mojom::IntAttribute::kChildTreeId on a views::View in the browser process that represents the remote app, see here:  
> 
> https://cs.chromium.org/chromium/src/ash/wm/non_client_frame_controller.cc?type=cs&sq=package:chromium&g=0&l=257
> 
> The code in this CL runs in the remote app process, not in the browser process, so it can't call directly into AutomationEventRouter. I have a browser-side proxy that speaks mojo to the remote app in AXHostService, see here:
> 
> https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/chromeos/accessibility/ax_host_service.h
> 
> Do you think I should be manually doing DispatchTreeDestroyedEvent() when the remote app closes? Is there some way I can dump the AX extension's final tree state, so I can see what nodes it ended up with?
> 
> 

Yes, you should be dispatching tree destroyed.

The data flows from ash -> chrome -> extension.

The chrome process hooks up the chlid tree to the larger desktop tree. The desktop tree itself never gets destroyed, so you perhaps didn't get that bit when you modeled off of AutomationManagerAura/AXTreeSourceAura.

The extension process, where the data gets unserialized lives at:
chrome/renderer/extension/automation_internal_custom_bindings.cc. Here, we manage a collection of AXTree's from all processes, including your remote app.

You can see other sources of 
DispatchTreeDestroyedEvent
in
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
and
c/b/c/arc/ax_tree_source_arc.cc

The extension process deletes the target (unserialized) tree once this event/message gets received.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6

commit e7a3b5d5c3d5a43008dc251b57ba87019f1485b6
Author: James Cook <jamescook@chromium.org>
Date: Wed Aug 08 19:14:20 2018

chromeos: Destroy AXTree when remote mojo app closes

Observe remote mojo apps (like keyboard shortcut viewer) for disconnect.
Inform accessibility extensions via the automation API that they can
destroy the AX node tree for the app. This handles both normal app
shutdown and app crashes.

Also rename ShowKeyboardShortcutViewer() to Toggle, since that's what
it actually does.

TBR=xiyuan@chromium.org

Bug:  871414 
Test: added to LoggedInSpokenFeedbackTest.KeyboardShortcutViewer
Change-Id: I463d69473a477a9ba2daee5d45a7e65c94981466
Reviewed-on: https://chromium-review.googlesource.com/1166193
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581654}
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/chromeos/accessibility/ax_host_service.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/chromeos/accessibility/ax_host_service.h
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/chromeos/accessibility/spoken_feedback_browsertest.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/extensions/api/automation_internal/automation_event_router.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/extensions/api/automation_internal/automation_event_router.h
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/ui/app_list/internal_app/internal_app_metadata.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/ui/ash/chrome_new_window_client.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/e7a3b5d5c3d5a43008dc251b57ba87019f1485b6/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.h

Status: Fixed (was: Started)

Sign in to add a comment