Skip to content

Commit

Permalink
[fuchsia][a11y] Remove attempt to "clear" a11y bridge inspect node.
Browse files Browse the repository at this point in the history
We try to ensure that the AXTree dump inspect node is only inflated when
semantics are enabled by resetting the node's value to inspect::LazyNode().
However, doing so doesn't actually dissociate the old callback from the
node, so the old callback will still run on subsequent queries. It's
unclear whether the issue affects prod, but it can cause crashes on dev
builds.

Instead of attempting to "clear" the LazyNode when semantic updates are
disabled, I think it's best to leave it intact. We still clear ax_trees_
when semantic updates are disabled, so the node will not produce any
output when semantic updates are disabled (which is the desired
outcome).

(cherry picked from commit 2f4262b)

Test: manually verified on device
Bug: fuchsia:91565
Change-Id: Ib82259ce033bd1b79ea8e6a7204613a804c7ac92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3389996
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#989614}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3575363
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/4692@{#1550}
Cr-Branched-From: 038cd96-refs/heads/main@{#938553}
  • Loading branch information
SergeyUlanov authored and Chromium LUCI CQ committed Apr 8, 2022
1 parent b5c033e commit fd641c6
Showing 1 changed file with 5 additions and 7 deletions.
12 changes: 5 additions & 7 deletions fuchsia/engine/browser/accessibility_bridge.cc
Expand Up @@ -71,11 +71,14 @@ AccessibilityBridge::AccessibilityBridge(
ZX_LOG(ERROR, status) << "SemanticTree disconnected";
std::move(on_error_callback_).Run(ZX_ERR_INTERNAL);
});

// Set up inspect node for semantic trees.
inspect_node_tree_dump_ = inspect_node_.CreateLazyNode(
kSemanticTreesInspectNodeName,
[this]() { return fit::make_ok_promise(FillInspectData()); });
}

inspect::Inspector AccessibilityBridge::FillInspectData() {
DCHECK(enable_semantic_updates_);

inspect::Inspector inspector;

// Add a node for each AXTree of which the accessibility bridge is aware.
Expand Down Expand Up @@ -310,10 +313,6 @@ void AccessibilityBridge::OnSemanticsModeChanged(
// The first call to AccessibilityEventReceived after this call will be
// the entire semantic tree.
web_contents_->EnableWebContentsOnlyAccessibilityMode();
// Set up inspect node for semantic trees.
inspect_node_tree_dump_ = inspect_node_.CreateLazyNode(
kSemanticTreesInspectNodeName,
[this]() { return fit::make_ok_promise(FillInspectData()); });
} else {
// The SemanticsManager will clear all state in this case, which is
// mirrored here.
Expand All @@ -327,7 +326,6 @@ void AccessibilityBridge::OnSemanticsModeChanged(
tree_connections_.clear();
frame_id_to_tree_id_.clear();
InterruptPendingActions();
inspect_node_tree_dump_ = inspect::LazyNode();
}

// Notify the SemanticsManager that this request was handled.
Expand Down

0 comments on commit fd641c6

Please sign in to comment.