Commit 7e48dfeb authored by wutao's avatar wutao Committed by Commit Bot

assistant: Remove DCHECK of Assistant structure

This cl fixes two issues:
1. When app_list is not active, we should not request focus to
assistant_page_view. Otherwise, the active window info will lose so that
we cannot get the assistant structure of the active window.

2. The DCHECK of |assistant_tree_| or |assistant_extra_| is not valid
because they could be nullptr if there is not such an active window.

Bug: b/139363094
Test: manual
Change-Id: I3b78688b15047925dc92f613fe9e46b198d864fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017982
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738284}
parent c805aa2d
...@@ -118,11 +118,6 @@ void UpdateActivationForAppListView(AppListView* app_list_view, ...@@ -118,11 +118,6 @@ void UpdateActivationForAppListView(AppListView* app_list_view,
return; return;
widget->Show(); widget->Show();
// Refocus the embedded assistant page after widget activation.
auto* contents_view = app_list_view->app_list_main_view()->contents_view();
if (contents_view->IsShowingEmbeddedAssistantUI())
contents_view->FocusEmbeddedAssistantPage();
} }
} // namespace ash } // namespace ash
...@@ -1328,7 +1328,15 @@ AppListStateTransitionSource AppListView::GetAppListStateTransitionSource( ...@@ -1328,7 +1328,15 @@ AppListStateTransitionSource AppListView::GetAppListStateTransitionSource(
} }
views::View* AppListView::GetInitiallyFocusedView() { views::View* AppListView::GetInitiallyFocusedView() {
return app_list_main_view_->search_box_view()->search_box(); views::View* initial_view;
if (IsShowingEmbeddedAssistantUI()) {
// Assistant page will redirect focus to its subviews.
auto* content = app_list_main_view_->contents_view();
initial_view = content->GetPageView(content->GetActivePageIndex());
} else {
initial_view = app_list_main_view_->search_box_view()->search_box();
}
return initial_view;
} }
void AppListView::OnScrollEvent(ui::ScrollEvent* event) { void AppListView::OnScrollEvent(ui::ScrollEvent* event) {
...@@ -1577,7 +1585,7 @@ void AppListView::SetState(AppListViewState new_state) { ...@@ -1577,7 +1585,7 @@ void AppListView::SetState(AppListViewState new_state) {
if (is_in_drag_ && app_list_state_ != AppListViewState::kClosed) if (is_in_drag_ && app_list_state_ != AppListViewState::kClosed)
app_list_main_view_->contents_view()->UpdateYPositionAndOpacity(); app_list_main_view_->contents_view()->UpdateYPositionAndOpacity();
if (GetWidget()->IsActive() && !IsShowingEmbeddedAssistantUI()) { if (GetWidget()->IsActive()) {
// Reset the focus to initially focused view. This should be // Reset the focus to initially focused view. This should be
// done before updating visibility of views, because setting // done before updating visibility of views, because setting
// focused view invisible automatically moves focus to next // focused view invisible automatically moves focus to next
......
...@@ -234,6 +234,10 @@ void AssistantPageView::OnUiVisibilityChanged( ...@@ -234,6 +234,10 @@ void AssistantPageView::OnUiVisibilityChanged(
return; return;
} }
// Assistant page will get focus when widget shown.
if (GetWidget() && GetWidget()->IsActive())
RequestFocus();
const bool prefer_voice = const bool prefer_voice =
assistant_view_delegate_->IsTabletMode() || assistant_view_delegate_->IsTabletMode() ||
AssistantState::Get()->launch_with_mic_open().value_or(false); AssistantState::Get()->launch_with_mic_open().value_or(false);
......
...@@ -357,14 +357,6 @@ void ContentsView::ShowEmbeddedAssistantUI(bool show) { ...@@ -357,14 +357,6 @@ void ContentsView::ShowEmbeddedAssistantUI(bool show) {
// Hide or Show results. // Hide or Show results.
auto* page_view = GetPageView(assistant_page); auto* page_view = GetPageView(assistant_page);
page_view->SetVisible(show); page_view->SetVisible(show);
if (show) {
page_view->RequestFocus();
// RequestFocus() might cause ResetForShow() method through
// AppListView::OnHomeLauncherGainingFocusWithoutAnimation() and it can hide
// |page_view|. Thus |page_view|'s visibility should be set again. See
// b/140831868.
page_view->SetVisible(show);
}
const int search_results_page = const int search_results_page =
GetPageIndexForState(AppListState::kStateSearchResults); GetPageIndexForState(AppListState::kStateSearchResults);
......
...@@ -455,9 +455,9 @@ void AssistantManagerServiceImpl::StartCachedScreenContextInteraction() { ...@@ -455,9 +455,9 @@ void AssistantManagerServiceImpl::StartCachedScreenContextInteraction() {
return; return;
// It is illegal to call this method without having first cached screen // It is illegal to call this method without having first cached screen
// context (see CacheScreenContext()). // context (see CacheScreenContext()). |assistant_extra_| and
DCHECK(assistant_extra_); // |assistant_tree_| could be nullptr if there is no active browser or ARC.
DCHECK(assistant_tree_); // window.
DCHECK(!assistant_screenshot_.empty()); DCHECK(!assistant_screenshot_.empty());
SendScreenContextRequest(assistant_extra_.get(), assistant_tree_.get(), SendScreenContextRequest(assistant_extra_.get(), assistant_tree_.get(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment