Commit 4b76ba3e authored by David Black's avatar David Black Committed by Commit Bot

Fix window focus issue in AppList w/ regards to Assistant.

Symptom:
In tablet mode, clicking into the embedded web content that hosts
Assistant Settings would cause a change in window focus. This change
in window focus resulted in a change in AppList state back to
kStateApps, making Assistant Settings unusable. The call to modify
AppList state originated here:

https://cs.chromium.org/chromium/src/ash/app_list/app_list_presenter_impl.cc?type=cs&q=applistpresenterimp&g=0&l=454

Solution:
Resetting of AppList state occurred because AppList visibility
incorrectly thought it was not visibility when showing Assistant. Now,
visibility is updated when changing AppList states such that when
Assistant is visible, AppList is rightly considered visible as well.

Bug: b:144056527
Change-Id: I2613758d131c6621df5f61bdd66106399b8d7d9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903901
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715363}
parent e3cbc167
...@@ -525,6 +525,20 @@ void AppListControllerImpl::OnAppListStateChanged(ash::AppListState new_state, ...@@ -525,6 +525,20 @@ void AppListControllerImpl::OnAppListStateChanged(ash::AppListState new_state,
UpdateLauncherContainer(); UpdateLauncherContainer();
// Band-aid for https://b/144056527 to update visibility after AppListState
// change. Otherwise, previously calculated visibility in OnVisibilityChanged
// and OnVisibilityWillChange is not correct and makes focus change handler
// code in AppListPresenterImpl::OnWindowFocused close the app list window
// when focus moves into Assistant web contents.
aura::Window* app_list_window = GetWindow();
if (app_list_window) {
const bool app_list_visible = app_list_window->TargetVisibility();
if (app_list_visible != IsVisible()) {
OnVisibilityWillChange(app_list_visible, last_visible_display_id_);
OnVisibilityChanged(app_list_visible, last_visible_display_id_);
}
}
if (new_state == ash::AppListState::kStateEmbeddedAssistant) { if (new_state == ash::AppListState::kStateEmbeddedAssistant) {
// ShowUi will be no-op if the AssistantUiModel is already visible. // ShowUi will be no-op if the AssistantUiModel is already visible.
Shell::Get()->assistant_controller()->ui_controller()->ShowUi( Shell::Get()->assistant_controller()->ui_controller()->ShowUi(
...@@ -1345,7 +1359,7 @@ void AppListControllerImpl::OnVisibilityChanged(bool visible, ...@@ -1345,7 +1359,7 @@ void AppListControllerImpl::OnVisibilityChanged(bool visible,
// HomeLauncher is only visible when no other app windows are visible, // HomeLauncher is only visible when no other app windows are visible,
// unless we are in the process of animating to (or dragging) the home // unless we are in the process of animating to (or dragging) the home
// launcher. // launcher.
if (IsTabletMode()) if (IsTabletMode() && ShouldLauncherShowBehindApps())
real_visibility &= !HasVisibleWindows(); real_visibility &= !HasVisibleWindows();
DCHECK_EQ(last_target_visible_, real_visibility) DCHECK_EQ(last_target_visible_, real_visibility)
...@@ -1384,8 +1398,9 @@ void AppListControllerImpl::OnVisibilityWillChange(bool visible, ...@@ -1384,8 +1398,9 @@ void AppListControllerImpl::OnVisibilityWillChange(bool visible,
// HomeLauncher is only visible when no other app windows are visible, // HomeLauncher is only visible when no other app windows are visible,
// unless we are in the process of animating to (or dragging) the home // unless we are in the process of animating to (or dragging) the home
// launcher. // launcher.
if (IsTabletMode() && home_launcher_transition_state_ == if (IsTabletMode() && ShouldLauncherShowBehindApps() &&
HomeLauncherTransitionState::kFinished) { home_launcher_transition_state_ ==
HomeLauncherTransitionState::kFinished) {
real_target_visibility &= !HasVisibleWindows(); real_target_visibility &= !HasVisibleWindows();
} }
......
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