Skip to content

Commit

Permalink
applist: Activate the app list window after animating.
Browse files Browse the repository at this point in the history
This will help animations and dragging as the damage rects for the
active window are quite large. Deactivation of the app list window is
still sync.

Test: ash_unittests
Bug: 1022045
Change-Id: I206521dc7c3ef7dbcf8e18a37e3b43d58717fe33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1904760
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716464}
  • Loading branch information
sammiequon71 authored and Commit Bot committed Nov 19, 2019
1 parent a44aaa7 commit b736c1a
Show file tree
Hide file tree
Showing 18 changed files with 134 additions and 29 deletions.
28 changes: 26 additions & 2 deletions ash/app_list/app_list_presenter_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "ui/events/keycodes/keyboard_codes_posix.h"
#include "ui/views/widget/widget.h"
#include "ui/wm/core/coordinate_conversion.h"
#include "ui/wm/public/activation_client.h"

namespace ash {
namespace {
Expand Down Expand Up @@ -90,8 +91,11 @@ void AppListPresenterDelegateImpl::SetPresenter(

void AppListPresenterDelegateImpl::Init(AppListView* view, int64_t display_id) {
view_ = view;
view->InitView(IsTabletMode(),
controller_->GetContainerForDisplayId(display_id));
view->InitView(
IsTabletMode(), controller_->GetContainerForDisplayId(display_id),
base::BindRepeating(
&AppListPresenterDelegateImpl::OnViewBoundsChangedAnimationEnded,
weak_ptr_factory_.GetWeakPtr()));

// By setting us as DnD recipient, the app list knows that we can
// handle items.
Expand Down Expand Up @@ -119,6 +123,7 @@ void AppListPresenterDelegateImpl::ShowForDisplay(int64_t display_id) {
view_->SetShelfHasRoundedCorners(
IsShelfBackgroundTypeWithRoundedCorners(shelf->GetBackgroundType()));
view_->Show(IsSideShelf(shelf), IsTabletMode());
view_->GetWidget()->ShowInactive();

SnapAppListBoundsToDisplayEdge();

Expand Down Expand Up @@ -323,4 +328,23 @@ void AppListPresenterDelegateImpl::SnapAppListBoundsToDisplayEdge() {
window->SetBounds(bounds);
}

void AppListPresenterDelegateImpl::OnViewBoundsChangedAnimationEnded() {
views::Widget* widget = view_->GetWidget();
// If we are currently dragging the applist from the shelf, do not update
// window activation to avoid redrawing during dragging which is also a heavy
// operation. The activation will be handled when this callback is run again
// after the state bounds animation finishes.
if (Shelf::ForWindow(widget->GetNativeWindow())
->shelf_layout_manager()
->IsDraggingApplist()) {
return;
}

// Deactivation after dragging or animating is not supported.
if (!is_visible_)
return;

UpdateActivationForAppListView(view_, IsTabletMode());
}

} // namespace ash
7 changes: 7 additions & 0 deletions ash/app_list/app_list_presenter_delegate_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/shelf/shelf.h"
#include "ash/shelf/shelf_observer.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "ui/display/display_observer.h"
#include "ui/display/screen.h"
Expand Down Expand Up @@ -73,6 +74,10 @@ class ASH_EXPORT AppListPresenterDelegateImpl : public AppListPresenterDelegate,
// https://crbug.com/884889).
void SnapAppListBoundsToDisplayEdge();

// Callback function which is run after a bounds animation on |view_| is
// ended. Handles activation of |view_|'s widget.
void OnViewBoundsChangedAnimationEnded();

// Whether the app list is visible (or in the process of being shown).
bool is_visible_ = false;

Expand All @@ -92,6 +97,8 @@ class ASH_EXPORT AppListPresenterDelegateImpl : public AppListPresenterDelegate,
// An observer that notifies AppListView when the shelf state has changed.
ScopedObserver<Shelf, ShelfObserver> shelf_observer_{this};

base::WeakPtrFactory<AppListPresenterDelegateImpl> weak_ptr_factory_{this};

DISALLOW_COPY_AND_ASSIGN(AppListPresenterDelegateImpl);
};

Expand Down
6 changes: 5 additions & 1 deletion ash/app_list/app_list_presenter_delegate_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "ash/app_list/app_list_controller_impl.h"
#include "ash/app_list/app_list_presenter_impl.h"
#include "ash/app_list/app_list_util.h"
#include "ash/app_list/model/app_list_item.h"
#include "ash/app_list/test/app_list_test_helper.h"
#include "ash/app_list/test/app_list_test_model.h"
Expand Down Expand Up @@ -205,7 +206,10 @@ class PopulatedAppListTest : public AshTestBase,
protected:
void CreateAndOpenAppList() {
app_list_view_ = new AppListView(app_list_test_delegate_.get());
app_list_view_->InitView(false /*is_tablet_mode*/, CurrentContext());
app_list_view_->InitView(
false /*is_tablet_mode*/, CurrentContext(),
base::BindRepeating(&UpdateActivationForAppListView, app_list_view_,
/*is_tablet_mode=*/false));
app_list_view_->Show(false /*is_side_shelf*/, false /*is_tablet_mode*/);
}

Expand Down
6 changes: 5 additions & 1 deletion ash/app_list/app_list_presenter_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>
#include <vector>

#include "ash/app_list/app_list_util.h"
#include "ash/app_list/test/app_list_test_view_delegate.h"
#include "ash/app_list/views/app_list_item_view.h"
#include "ash/app_list/views/app_list_view.h"
Expand Down Expand Up @@ -51,7 +52,10 @@ class AppListPresenterDelegateTest : public AppListPresenterDelegate {
void Init(AppListView* view, int64_t display_id) override {
init_called_ = true;
view_ = view;
view->InitView(/*is_tablet_mode*/ false, container_);
view->InitView(
/*is_tablet_mode*/ false, container_,
base::BindRepeating(&UpdateActivationForAppListView, view_,
/*is_tablet_mode=*/false));
}
void ShowForDisplay(int64_t display_id) override {}
void OnClosing() override { on_dismissed_called_ = true; }
Expand Down
25 changes: 25 additions & 0 deletions ash/app_list/app_list_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@

#include "ash/app_list/app_list_util.h"

#include "ash/app_list/views/app_list_main_view.h"
#include "ash/app_list/views/app_list_view.h"
#include "ash/app_list/views/contents_view.h"
#include "ui/aura/window.h"
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/focus/focus_manager.h"
#include "ui/wm/public/activation_client.h"

namespace ash {

Expand Down Expand Up @@ -100,4 +105,24 @@ bool ProcessLeftRightKeyTraversalForTextfield(views::Textfield* textfield,
return true;
}

void UpdateActivationForAppListView(AppListView* app_list_view,
bool is_tablet_mode) {
views::Widget* widget = app_list_view->GetWidget();
const aura::Window* active_window =
wm::GetActivationClient(widget->GetNativeWindow()->GetRootWindow())
->GetActiveWindow();

// After switching to tablet mode, other app windows may be active. Show the
// app list without activating it to avoid breaking other windows' state.
if (is_tablet_mode && active_window)
return;

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
6 changes: 6 additions & 0 deletions ash/app_list/app_list_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Textfield;
}

namespace ash {
class AppListView;

// Returns true if the key event is an unhandled left or right arrow (unmodified
// by ctrl, shift, or alt)
Expand Down Expand Up @@ -46,6 +47,11 @@ APP_LIST_EXPORT bool ProcessLeftRightKeyTraversalForTextfield(
views::Textfield* textfield,
const ui::KeyEvent& key_event);

// Updates the activation for |app_list_view|. Intended to be a callback
// function for when the view's bounds are finished animating.
APP_LIST_EXPORT void UpdateActivationForAppListView(AppListView* app_list_view,
bool is_tablet_mode);

} // namespace ash

#endif // ASH_APP_LIST_APP_LIST_UTIL_H_
6 changes: 5 additions & 1 deletion ash/app_list/demo/app_list_demo_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <memory>

#include "ash/app_list/app_list_util.h"
#include "ash/app_list/test/app_list_test_model.h"
#include "ash/app_list/test/app_list_test_view_delegate.h"
#include "ash/app_list/views/app_list_view.h"
Expand Down Expand Up @@ -51,7 +52,10 @@ AppListView* DemoAppListViewDelegate::InitView(
gfx::NativeView container = window_context;

view_ = new AppListView(this);
view_->InitView(false /*is_tablet_mode*/, container);
view_->InitView(
/*is_tablet_mode=*/false, container,
base::BindRepeating(&UpdateActivationForAppListView, view_,
/*is_tablet_mode=*/false));
view_->Show(false /*is_side_shelf*/, false /*is_tablet_mode*/);

// Populate some apps.
Expand Down
14 changes: 0 additions & 14 deletions ash/app_list/views/app_list_main_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/widget/widget.h"
#include "ui/wm/public/activation_client.h"

namespace ash {

Expand Down Expand Up @@ -87,19 +86,6 @@ void AppListMainView::AddContentsViews() {
search_box_view_->set_contents_view(contents_view_);
}

void AppListMainView::ShowAppListWhenReady() {
// After switching to tablet mode, other app windows may be active. Show the
// app list without activating it to avoid breaking other windows' state.
const aura::Window* active_window =
wm::GetActivationClient(
app_list_view_->GetWidget()->GetNativeView()->GetRootWindow())
->GetActiveWindow();
if (app_list_view_->is_tablet_mode() && active_window)
GetWidget()->ShowInactive();
else
GetWidget()->Show();
}

void AppListMainView::ModelChanged() {
model_->RemoveObserver(this);
model_ = delegate_->GetModel();
Expand Down
2 changes: 0 additions & 2 deletions ash/app_list/views/app_list_main_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ class APP_LIST_EXPORT AppListMainView

void Init(int initial_apps_page, SearchBoxView* search_box_view);

void ShowAppListWhenReady();

void ModelChanged();

SearchBoxView* search_box_view() const { return search_box_view_; }
Expand Down
14 changes: 12 additions & 2 deletions ash/app_list/views/app_list_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -588,13 +588,19 @@ bool AppListView::ShortAnimationsForTesting() {
return short_animations_for_testing;
}

void AppListView::InitView(bool is_tablet_mode, gfx::NativeView parent) {
void AppListView::InitView(
bool is_tablet_mode,
gfx::NativeView parent,
base::RepeatingClosure on_bounds_animation_ended_callback) {
base::AutoReset<bool> auto_reset(&is_building_, true);
time_shown_ = base::Time::Now();
UpdateAppListConfig(parent);
InitContents(is_tablet_mode);
InitWidget(parent);
InitChildWidget();

on_bounds_animation_ended_callback_ =
std::move(on_bounds_animation_ended_callback);
}

void AppListView::InitContents(bool is_tablet_mode) {
Expand Down Expand Up @@ -699,7 +705,8 @@ void AppListView::Show(bool is_side_shelf, bool is_tablet_mode) {
CloseKeyboardIfVisible();

OnTabletModeChanged(is_tablet_mode);
app_list_main_view_->ShowAppListWhenReady();
// Widget may be activated by |on_bounds_animation_ended_callback_|.
GetWidget()->ShowInactive();

UMA_HISTOGRAM_TIMES(kAppListCreationTimeHistogram,
base::Time::Now() - time_shown_.value());
Expand Down Expand Up @@ -2026,6 +2033,9 @@ void AppListView::OnBoundsAnimationCompleted() {
// Layout if the animation was completed.
if (!was_animation_interrupted)
Layout();

if (on_bounds_animation_ended_callback_)
on_bounds_animation_ended_callback_.Run();
}

gfx::Rect AppListView::GetItemScreenBoundsInFirstGridPage(
Expand Down
7 changes: 6 additions & 1 deletion ash/app_list/views/app_list_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
static float GetTransitionProgressForState(ash::AppListViewState state);

// Initializes the view, only done once per session.
void InitView(bool is_tablet_mode, gfx::NativeView parent);
void InitView(bool is_tablet_mode,
gfx::NativeView parent,
base::RepeatingClosure on_bounds_animation_ended_callback);

// Initializes the contents of the view.
void InitContents(bool is_tablet_mode);
Expand Down Expand Up @@ -574,6 +576,9 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
// instead of the default instance.
std::unique_ptr<AppListConfig> app_list_config_;

// Callback which is run when the bounds animation of the widget is ended.
base::RepeatingClosure on_bounds_animation_ended_callback_;

base::WeakPtrFactory<AppListView> weak_ptr_factory_{this};

DISALLOW_COPY_AND_ASSIGN(AppListView);
Expand Down
9 changes: 7 additions & 2 deletions ash/app_list/views/app_list_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <utility>
#include <vector>

#include "ash/app_list/app_list_util.h"
#include "ash/app_list/model/search/search_box_model.h"
#include "ash/app_list/test/app_list_test_model.h"
#include "ash/app_list/test/app_list_test_view_delegate.h"
Expand Down Expand Up @@ -188,7 +189,9 @@ class AppListViewTest : public views::ViewsTestBase,
void Initialize(bool is_tablet_mode) {
delegate_ = std::make_unique<AppListTestViewDelegate>();
view_ = new AppListView(delegate_.get());
view_->InitView(is_tablet_mode, GetContext());
view_->InitView(is_tablet_mode, GetContext(),
base::BindRepeating(&UpdateActivationForAppListView, view_,
is_tablet_mode));
test_api_.reset(new AppsGridViewTestApi(apps_grid_view()));
EXPECT_FALSE(view_->GetWidget()->IsVisible());
}
Expand Down Expand Up @@ -434,7 +437,9 @@ class AppListViewFocusTest : public views::ViewsTestBase,
"weather", "Unimportant Title"));
delegate_ = std::make_unique<AppListTestViewDelegate>();
view_ = new AppListView(delegate_.get());
view_->InitView(false /*is_tablet_mode*/, GetContext());
view_->InitView(false /*is_tablet_mode*/, GetContext(),
base::BindRepeating(&UpdateActivationForAppListView, view_,
/*is_tablet_mode=*/false));
Show();
test_api_.reset(new AppsGridViewTestApi(apps_grid_view()));
suggestions_container_ = contents_view()
Expand Down
6 changes: 5 additions & 1 deletion ash/app_list/views/apps_grid_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>

#include "ash/app_list/app_list_metrics.h"
#include "ash/app_list/app_list_util.h"
#include "ash/app_list/model/app_list_folder_item.h"
#include "ash/app_list/model/app_list_item.h"
#include "ash/app_list/model/app_list_model.h"
Expand Down Expand Up @@ -200,7 +201,10 @@ class AppsGridViewTest : public views::ViewsTestBase,
parent->SetBounds(gfx::Rect(gfx::Point(0, 0), gfx::Size(1024, 768)));
delegate_ = std::make_unique<AppListTestViewDelegate>();
app_list_view_ = new AppListView(delegate_.get());
app_list_view_->InitView(create_as_tablet_mode_, parent);
app_list_view_->InitView(
create_as_tablet_mode_, parent,
base::BindRepeating(&UpdateActivationForAppListView, app_list_view_,
create_as_tablet_mode_));
app_list_view_->Show(false /*is_side_shelf*/, create_as_tablet_mode_);
contents_view_ = app_list_view_->app_list_main_view()->contents_view();
apps_grid_view_ = contents_view_->GetAppsContainerView()->apps_grid_view();
Expand Down
9 changes: 9 additions & 0 deletions ash/app_list/views/contents_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,15 @@ bool ContentsView::IsShowingEmbeddedAssistantUI() const {
return IsStateActive(ash::AppListState::kStateEmbeddedAssistant);
}

void ContentsView::FocusEmbeddedAssistantPage() {
const int assistant_page =
GetPageIndexForState(ash::AppListState::kStateEmbeddedAssistant);
DCHECK_GE(assistant_page, 0);
auto* page_view = GetPageView(assistant_page);
page_view->RequestFocus();
page_view->SetVisible(true);
}

void ContentsView::InitializeSearchBoxAnimation(
ash::AppListState current_state,
ash::AppListState target_state) {
Expand Down
2 changes: 2 additions & 0 deletions ash/app_list/views/contents_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ class APP_LIST_EXPORT ContentsView : public views::View,
void ShowEmbeddedAssistantUI(bool show);
bool IsShowingEmbeddedAssistantUI() const;

void FocusEmbeddedAssistantPage();

void ShowFolderContent(AppListFolderItem* folder);

// Sets the active launcher page and animates the pages into place.
Expand Down
6 changes: 5 additions & 1 deletion ash/app_list/views/search_box_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>
#include <utility>

#include "ash/app_list/app_list_util.h"
#include "ash/app_list/test/app_list_test_view_delegate.h"
#include "ash/app_list/views/app_list_main_view.h"
#include "ash/app_list/views/app_list_view.h"
Expand Down Expand Up @@ -71,7 +72,10 @@ class SearchBoxViewTest : public views::test::WidgetTest,
views::test::WidgetTest::SetUp();

app_list_view_ = new AppListView(&view_delegate_);
app_list_view_->InitView(false /*is_tablet_mode*/, GetContext());
app_list_view_->InitView(
/*is_tablet_mode=*/false, GetContext(),
base::BindRepeating(&UpdateActivationForAppListView, app_list_view_,
/*is_tablet_mode=*/false));

widget_ = CreateTopLevelPlatformWidget();
view_ =
Expand Down

0 comments on commit b736c1a

Please sign in to comment.