Skip to content

Commit

Permalink
[Chromecast] Scope node removed from cache if no name node children.
Browse files Browse the repository at this point in the history
This CL add in some extra condition in updating scopes_route_cache_.
A node should not be considered as an scoped routes node if it does not
has any children that is a named routes node. This will solved the
problem of not triggering edge transitions when scoped routes node is
unchanged but its children come and go.

Test: build cast_shell and unittest
Bug: b/171499941, b/168548001
Change-Id: I3b48179762871d8d9faa5df9afff0a2ac3c935c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495913
Commit-Queue: Yuanyao Zhong <yyzhong@google.com>
Reviewed-by: Randy Rossi <rmrossi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821224}
  • Loading branch information
yyzhong-g authored and Commit Bot committed Oct 27, 2020
1 parent be5810d commit 527d6f0
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -662,20 +662,20 @@ void AXTreeSourceFlutter::HandleRoutes(std::vector<ui::AXEvent>* events) {
focus_event.event_type = ax::mojom::Event::kFocus;
focus_event.id = focused_id_;
focus_event.event_from = ax::mojom::EventFrom::kNone;

// Speak it.
SubmitTTS(name);
}
}
}

// Detect any removed nodes with scopes_route flag.
// Clean up the cache for those nodes that should not be in the cache anymore.
bool need_refocus = false;
for (std::vector<int32_t>::iterator it = scopes_route_cache_.begin();
it != scopes_route_cache_.end();) {
int32_t id = *it;
if (GetFromId(id) == nullptr) {
// This was removed.
FlutterSemanticsNode* scopes_route_node = GetFromId(id);
if (scopes_route_node == nullptr || !scopes_route_node->HasScopesRoute() ||
FindRoutesNode(scopes_route_node) == nullptr) {
// This was removed in the latest tree update or there are no more
// RoutesNode child.
it = scopes_route_cache_.erase(it++);
need_refocus = true;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,17 +726,6 @@ TEST_F(AXTreeSourceFlutterTest, NoClickable) {
// Tests a new node with scopes route will focus and speak
// a child with names route set.
TEST_F(AXTreeSourceFlutterTest, ScopesRoute) {
// Install a mock tts platform
auto* tts_controller = content::TtsController::GetInstance();
content::MockTtsPlatformImpl mock_tts_platform;
tts_controller->SetTtsPlatform(&mock_tts_platform);

// Setup some mocks required for tts platform impl
content::MockContentBrowserClient mock_content_browser_client;
content::MockContentClient client;
content::SetContentClient(&client);
content::SetBrowserClientForTesting(&mock_content_browser_client);

// Add node with scopes route and child with names route. Focus
// should move to node with names route.
//
Expand Down Expand Up @@ -790,16 +779,9 @@ TEST_F(AXTreeSourceFlutterTest, ScopesRoute) {
CallGetTreeData(&tree_data);
ASSERT_EQ(6, tree_data.focus_id);

// Child 3 should have been spoken
ASSERT_TRUE(mock_tts_platform.GetLastSpokenUtterance() == child_6_label);

mock_tts_platform.ClearLastSpokenUtterance();

// Same tree should not speak the same scopes_route/names_route
CallNotifyAccessibilityEvent(&event);

ASSERT_TRUE(mock_tts_platform.GetLastSpokenUtterance() == "");

// Now setup another tree but with 5&6 removed. This should
// make the tree source focus (but not speak) 3

Expand Down Expand Up @@ -828,11 +810,9 @@ TEST_F(AXTreeSourceFlutterTest, ScopesRoute) {
CallGetTreeData(&tree_data);
ASSERT_EQ(3, tree_data.focus_id);

// Nothings spoken
ASSERT_TRUE(mock_tts_platform.GetLastSpokenUtterance() == "");
// Now step to have removed node 3 and re-add node 3 back in the tree. In this
// case, node 3 should be refocused and spoken.

// Finally, remove 2&3
//
OnAccessibilityEventRequest event3;
event3.set_source_id(0);
event3.set_window_id(1);
Expand All @@ -841,8 +821,52 @@ TEST_F(AXTreeSourceFlutterTest, ScopesRoute) {
root = event3.add_node_data();
root->set_node_id(0);

child1 = AddChild(&event3, root, 1, 0, 0, 1, 1, false);
child2 = AddChild(&event3, child1, 2, 0, 0, 1, 1, false);

// Set scopes on child2
boolean_properties = child2->mutable_boolean_properties();
boolean_properties->set_scopes_route(true);

CallNotifyAccessibilityEvent(&event3);

OnAccessibilityEventRequest event4;
event4.set_source_id(0);
event4.set_window_id(1);
event4.set_event_type(OnAccessibilityEventRequest_EventType_CONTENT_CHANGED);

root = event4.add_node_data();
root->set_node_id(0);

child1 = AddChild(&event4, root, 1, 0, 0, 1, 1, false);
child2 = AddChild(&event4, child1, 2, 0, 0, 1, 1, false);
child3 = AddChild(&event4, child2, 3, 0, 0, 1, 1, false);
child3->set_label(child_3_label);

// Set scopes on child2, names on child3
boolean_properties = child2->mutable_boolean_properties();
boolean_properties->set_scopes_route(true);
boolean_properties = child3->mutable_boolean_properties();
boolean_properties->set_names_route(true);

CallNotifyAccessibilityEvent(&event4);

// Focus moves to 3
CallGetTreeData(&tree_data);
ASSERT_EQ(3, tree_data.focus_id);

// Finally, remove 2
//
OnAccessibilityEventRequest event5;
event5.set_source_id(0);
event5.set_window_id(1);
event5.set_event_type(OnAccessibilityEventRequest_EventType_CONTENT_CHANGED);

root = event5.add_node_data();
root->set_node_id(0);

CallNotifyAccessibilityEvent(&event5);

CallGetTreeData(&tree_data);
ASSERT_EQ(0, tree_data.focus_id);
}
Expand Down

0 comments on commit 527d6f0

Please sign in to comment.