ash: Add AppListClient::StartZeroStateSearch() method
For productivity launcher we need to refresh the zero state search
results before we build the continue section views. Suggested files may
be deleted, and we don't want the view to disappear in the middle of
the show animation, or after the launcher is on-screen.
Add an AppListClient method to delegate back into //chrome/browser to
trigger the search. The method has a fixed 16ms timeout. For now,
always wait the full 16ms, so we can see how this feels in practice.
Bug: 1269115
Test: added to ash_unittests
Change-Id: I9fc059082a33dfe37671b2bb18c61e54b0ec61e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3288040
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Tony Yeoman <tby@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#943259}
diff --git a/ash/app_list/app_list_bubble_presenter.cc b/ash/app_list/app_list_bubble_presenter.cc
index 216451a..34d48a8 100644
--- a/ash/app_list/app_list_bubble_presenter.cc
+++ b/ash/app_list/app_list_bubble_presenter.cc
@@ -13,6 +13,7 @@
#include "ash/app_list/views/app_list_bubble_view.h"
#include "ash/app_list/views/app_list_drag_and_drop_host.h"
#include "ash/constants/ash_features.h"
+#include "ash/public/cpp/app_list/app_list_client.h"
#include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shelf/home_button.h"
@@ -26,6 +27,7 @@
#include "base/i18n/rtl.h"
#include "base/logging.h"
#include "base/metrics/histogram_functions.h"
+#include "base/time/time.h"
#include "chromeos/services/assistant/public/cpp/assistant_enums.h"
#include "ui/aura/client/focus_client.h"
#include "ui/display/display.h"
@@ -39,6 +41,10 @@
using chromeos::assistant::AssistantExitPoint;
+// Maximum amount of time to spend refreshing zero state search results before
+// opening the launcher.
+constexpr base::TimeDelta kZeroStateSearchTimeout = base::Milliseconds(16);
+
// Space between the edge of the bubble and the edge of the work area.
constexpr int kWorkAreaPadding = 8;
@@ -160,9 +166,27 @@
if (bubble_widget_)
return;
- base::Time time_shown = base::Time::Now();
+ // Refresh the continue tasks before opening the launcher. If a file doesn't
+ // exist on disk anymore then the launcher should not create or animate the
+ // continue task view for that suggestion.
+ controller_->GetClient()->StartZeroStateSearch(
+ base::BindOnce(&AppListBubblePresenter::OnZeroStateSearchDone,
+ weak_factory_.GetWeakPtr(), display_id),
+ kZeroStateSearchTimeout);
+}
+
+void AppListBubblePresenter::OnZeroStateSearchDone(int64_t display_id) {
+ // Bubble might be open if Show() was called repeatedly.
+ if (bubble_widget_)
+ return;
aura::Window* root_window = Shell::GetRootWindowForDisplayId(display_id);
+ // Display might have disconnected during zero state refresh.
+ if (!root_window)
+ return;
+
+ base::TimeTicks time_shown = base::TimeTicks::Now();
+
bubble_widget_ = CreateBubbleWidget(root_window);
bubble_widget_->GetNativeWindow()->SetEventTargeter(
std::make_unique<AppListEventTargeter>(controller_));
@@ -203,7 +227,7 @@
base::Unretained(this)));
UmaHistogramTimes("Apps.AppListBubbleCreationTime",
- base::Time::Now() - time_shown);
+ base::TimeTicks::Now() - time_shown);
}
ShelfAction AppListBubblePresenter::Toggle(int64_t display_id) {
diff --git a/ash/app_list/app_list_bubble_presenter.h b/ash/app_list/app_list_bubble_presenter.h
index d6b0618d..74c2ea3 100644
--- a/ash/app_list/app_list_bubble_presenter.h
+++ b/ash/app_list/app_list_bubble_presenter.h
@@ -39,7 +39,11 @@
AppListBubblePresenter& operator=(const AppListBubblePresenter&) = delete;
~AppListBubblePresenter() override;
- // Shows the bubble on the display with `display_id`.
+ // Shows the bubble on the display with `display_id`. The bubble is shown
+ // asynchronously (after a delay) because the continue suggestions need to be
+ // refreshed before the bubble views can be created and animated. This delay
+ // is skipped in unit tests (see TestAppListClient) for convenience. Larger
+ // tests (e.g. browser_tests) may need to wait for the window to open.
void Show(int64_t display_id);
// Shows or hides the bubble on the display with `display_id`. Returns the
@@ -76,6 +80,10 @@
AppListBubbleView* bubble_view_for_test() { return bubble_view_; }
private:
+ // Callback for zero state search update. Builds the bubble widget and views
+ // on display `display_id` and triggers the show animation.
+ void OnZeroStateSearchDone(int64_t display_id);
+
// Callback for AppListBubbleEventFilter, used to notify this of presses
// outside the bubble.
void OnPressOutsideBubble();
diff --git a/ash/app_list/app_list_bubble_presenter_unittest.cc b/ash/app_list/app_list_bubble_presenter_unittest.cc
index f67714f..236dfac4 100644
--- a/ash/app_list/app_list_bubble_presenter_unittest.cc
+++ b/ash/app_list/app_list_bubble_presenter_unittest.cc
@@ -8,6 +8,7 @@
#include "ash/app_list/app_list_controller_impl.h"
#include "ash/app_list/test/app_list_test_helper.h"
+#include "ash/app_list/test_app_list_client.h"
#include "ash/app_list/views/app_list_bubble_view.h"
#include "ash/constants/ash_features.h"
#include "ash/public/cpp/shell_window_ids.h"
@@ -103,6 +104,16 @@
EXPECT_EQ(1u, NumberOfWidgetsInAppListContainer());
}
+TEST_F(AppListBubblePresenterTest, ShowStartsZeroStateSearch) {
+ AppListBubblePresenter* presenter = GetBubblePresenter();
+ presenter->Show(GetPrimaryDisplay().id());
+ EXPECT_EQ(1, GetTestAppListClient()->start_zero_state_search_count());
+
+ presenter->Dismiss();
+ presenter->Show(GetPrimaryDisplay().id());
+ EXPECT_EQ(2, GetTestAppListClient()->start_zero_state_search_count());
+}
+
TEST_F(AppListBubblePresenterTest, ShowRecordsCreationTimeHistogram) {
base::HistogramTester histogram_tester;
AppListBubblePresenter* presenter = GetBubblePresenter();
diff --git a/ash/app_list/app_list_controller_impl.cc b/ash/app_list/app_list_controller_impl.cc
index 6b668766..18eca98 100644
--- a/ash/app_list/app_list_controller_impl.cc
+++ b/ash/app_list/app_list_controller_impl.cc
@@ -1760,6 +1760,7 @@
fullscreen_presenter_->GetView()->CloseOpenedPage();
// Refresh the suggestion chips with empty query.
+ // TODO(crbug.com/1269115): Switch to client_->StartZeroStateSearch()?
StartSearch(std::u16string());
}
diff --git a/ash/app_list/app_list_test_api.cc b/ash/app_list/app_list_test_api.cc
index 39126e9..3992a02 100644
--- a/ash/app_list/app_list_test_api.cc
+++ b/ash/app_list/app_list_test_api.cc
@@ -5,6 +5,7 @@
#include "ash/public/cpp/test/app_list_test_api.h"
#include <string>
+#include <utility>
#include <vector>
#include "ash/app_list/app_list_bubble_presenter.h"
@@ -26,6 +27,9 @@
#include "ash/app_list/views/scrollable_apps_grid_view.h"
#include "ash/constants/ash_features.h"
#include "ash/shell.h"
+#include "base/callback.h"
+#include "base/run_loop.h"
+#include "ui/aura/window_observer.h"
#include "ui/views/view_model.h"
namespace ash {
@@ -52,11 +56,59 @@
return AppListModelProvider::Get()->model();
}
+// Waits until a child window is added to a container window.
+class WindowAddedWaiter : public aura::WindowObserver {
+ public:
+ explicit WindowAddedWaiter(aura::Window* container) : container_(container) {
+ container_->AddObserver(this);
+ }
+ WindowAddedWaiter(const WindowAddedWaiter&) = delete;
+ WindowAddedWaiter& operator=(const WindowAddedWaiter&) = delete;
+ ~WindowAddedWaiter() override { container_->RemoveObserver(this); }
+
+ void Wait() { run_loop_.Run(); }
+
+ aura::Window* added_window() { return added_window_; }
+
+ private:
+ // aura::WindowObserver:
+ void OnWindowAdded(aura::Window* new_window) override {
+ added_window_ = new_window;
+ DCHECK(run_loop_.IsRunningOnCurrentThread());
+ run_loop_.Quit();
+ }
+
+ aura::Window* const container_;
+ aura::Window* added_window_ = nullptr;
+ base::RunLoop run_loop_;
+};
+
} // namespace
AppListTestApi::AppListTestApi() = default;
AppListTestApi::~AppListTestApi() = default;
+void AppListTestApi::WaitForBubbleWindow() {
+ DCHECK(features::IsProductivityLauncherEnabled());
+ DCHECK(!Shell::Get()->IsInTabletMode());
+
+ // Do nothing if the window already exists.
+ auto* app_list_controller = Shell::Get()->app_list_controller();
+ if (app_list_controller->GetWindow())
+ return;
+
+ // Wait for a child window to be added to the app list container.
+ aura::Window* container = Shell::GetContainer(
+ Shell::GetPrimaryRootWindow(), kShellWindowId_AppListContainer);
+ WindowAddedWaiter waiter(container);
+ waiter.Wait();
+
+ // App list window exists.
+ aura::Window* app_list_window = app_list_controller->GetWindow();
+ DCHECK(app_list_window);
+ DCHECK_EQ(app_list_window, waiter.added_window());
+}
+
bool AppListTestApi::HasApp(const std::string& app_id) {
return GetAppListModel()->FindItem(app_id);
}
diff --git a/ash/app_list/test_app_list_client.cc b/ash/app_list/test_app_list_client.cc
index 740e643..a2b7ee95 100644
--- a/ash/app_list/test_app_list_client.cc
+++ b/ash/app_list/test_app_list_client.cc
@@ -16,6 +16,14 @@
TestAppListClient::~TestAppListClient() = default;
+void TestAppListClient::StartZeroStateSearch(base::OnceClosure on_done,
+ base::TimeDelta timeout) {
+ start_zero_state_search_count_++;
+ // Unit tests generally expect the launcher to open immediately, so run the
+ // callback synchronously.
+ std::move(on_done).Run();
+}
+
void TestAppListClient::StartSearch(const std::u16string& trimmed_query) {
last_search_query_ = trimmed_query;
}
@@ -34,7 +42,7 @@
void TestAppListClient::InvokeSearchResultAction(
const std::string& result_id,
SearchResultActionType action) {
- invoked_result_actions_.push_back(std::make_pair(result_id, action));
+ invoked_result_actions_.emplace_back(result_id, action);
}
void TestAppListClient::GetSearchResultContextMenuModel(
diff --git a/ash/app_list/test_app_list_client.h b/ash/app_list/test_app_list_client.h
index 74cc820..d8ca7b8 100644
--- a/ash/app_list/test_app_list_client.h
+++ b/ash/app_list/test_app_list_client.h
@@ -28,6 +28,8 @@
// AppListClient:
void OnAppListControllerDestroyed() override {}
+ void StartZeroStateSearch(base::OnceClosure on_done,
+ base::TimeDelta timeout) override;
void StartSearch(const std::u16string& trimmed_query) override;
void OpenSearchResult(int profile_id,
const std::string& result_id,
@@ -65,7 +67,10 @@
AppListNotifier* GetNotifier() override;
void LoadIcon(int profile_id, const std::string& app_id) override {}
- std::u16string last_search_query() { return last_search_query_; }
+ int start_zero_state_search_count() const {
+ return start_zero_state_search_count_;
+ }
+ std::u16string last_search_query() const { return last_search_query_; }
// Returns the number of AppItems that have been activated. These items could
// live in search, RecentAppsView, or ScrollableAppsGridView.
@@ -83,6 +88,7 @@
std::vector<SearchResultActionId> GetAndClearInvokedResultActions();
private:
+ int start_zero_state_search_count_ = 0;
std::u16string last_search_query_;
std::vector<SearchResultActionId> invoked_result_actions_;
int activate_item_count_ = 0;
diff --git a/ash/public/cpp/app_list/app_list_client.h b/ash/public/cpp/app_list/app_list_client.h
index 6c25c69..9011a2e9 100644
--- a/ash/public/cpp/app_list/app_list_client.h
+++ b/ash/public/cpp/app_list/app_list_client.h
@@ -14,6 +14,7 @@
#include "ash/public/cpp/app_list/app_list_types.h"
#include "ash/public/cpp/ash_public_export.h"
#include "base/callback_forward.h"
+#include "base/time/time.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "ui/base/models/simple_menu_model.h"
@@ -39,6 +40,14 @@
//////////////////////////////////////////////////////////////////////////////
// Interfaces on searching:
+
+ // Refreshes the search zero-state suggestions and invokes `on_done` when
+ // complete. The client must run `on_done` before `timeout` because this
+ // method is called when the user tries to open the launcher and the UI waits
+ // until `on_done` before opening it.
+ virtual void StartZeroStateSearch(base::OnceClosure on_done,
+ base::TimeDelta timeout) = 0;
+
// Triggers a search query.
// |trimmed_query|: the trimmed input texts from the search text field.
virtual void StartSearch(const std::u16string& trimmed_query) = 0;
diff --git a/ash/public/cpp/test/app_list_test_api.h b/ash/public/cpp/test/app_list_test_api.h
index 47ad3a3..954ab2f2 100644
--- a/ash/public/cpp/test/app_list_test_api.h
+++ b/ash/public/cpp/test/app_list_test_api.h
@@ -27,6 +27,11 @@
AppListTestApi(const AppListTestApi& other) = delete;
AppListTestApi& operator=(const AppListTestApi& other) = delete;
+ // Waits for the bubble launcher window to open on the primary display.
+ // See AppListBubblePresenter::Show(). Only used with productivity launcher
+ // in clamshell mode.
+ void WaitForBubbleWindow();
+
// Returns whether there is an item for |app_id|.
bool HasApp(const std::string& app_id);
diff --git a/chrome/browser/ui/app_list/app_list_client_impl.cc b/chrome/browser/ui/app_list/app_list_client_impl.cc
index 8c8e212..a745326 100644
--- a/chrome/browser/ui/app_list/app_list_client_impl.cc
+++ b/chrome/browser/ui/app_list/app_list_client_impl.cc
@@ -18,6 +18,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/strings/strcat.h"
+#include "base/threading/sequenced_task_runner_handle.h"
#include "chrome/browser/ash/crosapi/browser_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
@@ -124,6 +125,13 @@
current_model_updater_->SetActive(false);
}
+void AppListClientImpl::StartZeroStateSearch(base::OnceClosure on_done,
+ base::TimeDelta timeout) {
+ // TODO(https://crbug.com/1269115): Refresh the zero state results.
+ base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, std::move(on_done), timeout);
+}
+
void AppListClientImpl::StartSearch(const std::u16string& trimmed_query) {
if (search_controller_) {
search_controller_->Start(trimmed_query);
diff --git a/chrome/browser/ui/app_list/app_list_client_impl.h b/chrome/browser/ui/app_list/app_list_client_impl.h
index aff6c0d..d4efa84 100644
--- a/chrome/browser/ui/app_list/app_list_client_impl.h
+++ b/chrome/browser/ui/app_list/app_list_client_impl.h
@@ -74,6 +74,8 @@
// ash::AppListClient:
void OnAppListControllerDestroyed() override;
+ void StartZeroStateSearch(base::OnceClosure on_done,
+ base::TimeDelta timeout) override;
void StartSearch(const std::u16string& trimmed_query) override;
void OpenSearchResult(int profile_id,
const std::string& result_id,
diff --git a/chrome/browser/ui/app_list/app_list_sort_browsertest.cc b/chrome/browser/ui/app_list/app_list_sort_browsertest.cc
index f808067..0a5bef4 100644
--- a/chrome/browser/ui/app_list/app_list_sort_browsertest.cc
+++ b/chrome/browser/ui/app_list/app_list_sort_browsertest.cc
@@ -175,6 +175,7 @@
ash::ShellTestApi().SetTabletModeEnabledForTest(false);
ash::AcceleratorController::Get()->PerformActionIfEnabled(
ash::TOGGLE_APP_LIST_FULLSCREEN, {});
+ app_list_test_api_.WaitForBubbleWindow();
ash::AppsGridView* apps_grid_view =
app_list_test_api_.GetTopLevelAppsGridView();
@@ -247,6 +248,7 @@
ash::ShellTestApi().SetTabletModeEnabledForTest(false);
ash::AcceleratorController::Get()->PerformActionIfEnabled(
ash::TOGGLE_APP_LIST_FULLSCREEN, {});
+ app_list_test_api_.WaitForBubbleWindow();
// Move apps to one folder.
const std::string folder_id =
@@ -324,6 +326,7 @@
ash::ShellTestApi().SetTabletModeEnabledForTest(false);
ash::AcceleratorController::Get()->PerformActionIfEnabled(
ash::TOGGLE_APP_LIST_FULLSCREEN, {});
+ app_list_test_api_.WaitForBubbleWindow();
ash::AppListItemView* item_view =
app_list_test_api_.GetTopLevelAppsGridView()->view_model()->view_at(0);
@@ -411,6 +414,7 @@
ash::ShellTestApi().SetTabletModeEnabledForTest(false);
ash::AcceleratorController::Get()->PerformActionIfEnabled(
ash::TOGGLE_APP_LIST_FULLSCREEN, {});
+ app_list_test_api_.WaitForBubbleWindow();
// Move apps to one folder.
app_list_test_api_.CreateFolderWithApps({app1_id_, app2_id_, app3_id_});
