saved_desks: Improving launch functionality for admin templates
This CL implements logic to make sure that windows launched from an
admin template appear stacked on top of existing windows. Among the
windows from a launch, the windows are stacked according to the order
they appear in the template definition. This means that the window with
the lowest id will appear at the top of the stack.
This CL also adds a browser test. It lays the groundwork with testing
utilities that will be used by upcoming admin template CLs.
BUG=b:273803538
Change-Id: I03940f02c94349927914835ee99545853c34d358
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371006
Reviewed-by: Yongshun Liu <yongshun@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Daniel Andersson <dandersson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1127736}
diff --git a/ash/wm/desks/templates/saved_desk_constants.h b/ash/wm/desks/templates/saved_desk_constants.h
index 36dcd4e..b19ad5a 100644
--- a/ash/wm/desks/templates/saved_desk_constants.h
+++ b/ash/wm/desks/templates/saved_desk_constants.h
@@ -21,6 +21,12 @@
// Used in desk grid padding, SavedDeskItemView horizontal padding.
constexpr int kSaveDeskPaddingDp = 24;
+// Windows launched from admin templates will start with this activation index,
+// and then proceed downwards. In other words, all windows launched from admin
+// templates will have an activation index that is equal to or lower than this
+// value.
+constexpr int kAdminTemplateStartingActivationIndex = -1000000;
+
} // namespace ash
#endif // ASH_WM_DESKS_TEMPLATES_SAVED_DESK_CONSTANTS_H_
diff --git a/ash/wm/desks/templates/saved_desk_controller.cc b/ash/wm/desks/templates/saved_desk_controller.cc
index 1300cb1..2595676 100644
--- a/ash/wm/desks/templates/saved_desk_controller.cc
+++ b/ash/wm/desks/templates/saved_desk_controller.cc
@@ -8,10 +8,13 @@
#include "ash/public/cpp/saved_desk_delegate.h"
#include "ash/shell.h"
#include "ash/wm/desks/desks_controller.h"
+#include "ash/wm/desks/templates/saved_desk_constants.h"
#include "base/check.h"
+#include "base/containers/adapters.h"
#include "base/json/json_string_value_serializer.h"
#include "base/time/time.h"
#include "base/values.h"
+#include "components/app_restore/app_restore_data.h"
#include "components/app_restore/restore_data.h"
namespace ash {
@@ -59,6 +62,27 @@
// Pointer to the global `SavedDeskController` instance.
SavedDeskController* g_instance = nullptr;
+// The next activation index to assign to an admin template window.
+int32_t g_admin_template_next_activation_index =
+ kAdminTemplateStartingActivationIndex;
+
+// This function updates the activation indices of all the windows in an admin
+// template so that windows launched from it will stack in the order they are
+// defined, while also stacking on top of any existing windows.
+void UpdateAdminTemplateActivationIndices(DeskTemplate& saved_desk) {
+ auto& app_id_to_launch_list =
+ saved_desk.mutable_desk_restore_data()->mutable_app_id_to_launch_list();
+ // Go through the windows as defined in the saved desk in reverse order so
+ // that the window with the lowest id gets the lowest activation index. NB:
+ // for now, we expect admin templates to only contain a single app.
+ for (auto& [app_id, launch_list] : app_id_to_launch_list) {
+ for (auto& [window_id, app_restore_data] : base::Reversed(launch_list)) {
+ app_restore_data->activation_index =
+ g_admin_template_next_activation_index--;
+ }
+ }
+}
+
} // namespace
SavedDeskController::SavedDeskController() {
@@ -82,8 +106,8 @@
}
bool SavedDeskController::LaunchAdminTemplate(const base::GUID& template_uuid) {
- auto placeholder_template = CreatePlaceholderTemplate();
- if (!placeholder_template || template_uuid != placeholder_template->uuid()) {
+ auto admin_template = GetAdminTemplate(template_uuid);
+ if (!admin_template) {
return false;
}
@@ -91,12 +115,34 @@
auto* desks_controller = DesksController::Get();
const int desk_index =
desks_controller->GetDeskIndex(desks_controller->active_desk());
- placeholder_template->SetDeskIndex(desk_index);
+ admin_template->SetDeskIndex(desk_index);
+
+ UpdateAdminTemplateActivationIndices(*admin_template);
Shell::Get()->saved_desk_delegate()->LaunchAppsFromSavedDesk(
- std::move(placeholder_template));
+ std::move(admin_template));
return true;
}
+std::unique_ptr<DeskTemplate> SavedDeskController::GetAdminTemplate(
+ const base::GUID& template_uuid) const {
+ if (admin_template_for_testing_ &&
+ admin_template_for_testing_->uuid() == template_uuid) {
+ return admin_template_for_testing_->Clone();
+ }
+
+ auto placeholder_template = CreatePlaceholderTemplate();
+ if (placeholder_template && template_uuid == placeholder_template->uuid()) {
+ return placeholder_template;
+ }
+
+ return nullptr;
+}
+
+void SavedDeskController::SetAdminTemplateForTesting(
+ std::unique_ptr<DeskTemplate> admin_template) {
+ admin_template_for_testing_ = std::move(admin_template);
+}
+
} // namespace ash
diff --git a/ash/wm/desks/templates/saved_desk_controller.h b/ash/wm/desks/templates/saved_desk_controller.h
index 70da972..43da70f2 100644
--- a/ash/wm/desks/templates/saved_desk_controller.h
+++ b/ash/wm/desks/templates/saved_desk_controller.h
@@ -13,6 +13,8 @@
namespace ash {
+class DeskTemplate;
+
struct AdminTemplateMetadata {
// Uniquely identifies the template.
base::GUID uuid;
@@ -38,6 +40,18 @@
// Launch the template identified by `template_uuid`. Returns false if the
// template doesn't exist.
virtual bool LaunchAdminTemplate(const base::GUID& template_uuid);
+
+ private:
+ friend class SavedDeskControllerTestApi;
+
+ std::unique_ptr<DeskTemplate> GetAdminTemplate(
+ const base::GUID& template_uuid) const;
+
+ // Install an admin template that can be used by `LaunchAdminTemplate`.
+ void SetAdminTemplateForTesting(std::unique_ptr<DeskTemplate> admin_template);
+
+ // An optional admin template used for testing.
+ std::unique_ptr<DeskTemplate> admin_template_for_testing_;
};
} // namespace ash
diff --git a/ash/wm/desks/templates/saved_desk_test_util.cc b/ash/wm/desks/templates/saved_desk_test_util.cc
index b2feefc..203cd8c1 100644
--- a/ash/wm/desks/templates/saved_desk_test_util.cc
+++ b/ash/wm/desks/templates/saved_desk_test_util.cc
@@ -8,6 +8,7 @@
#include "ash/style/icon_button.h"
#include "ash/wm/desks/desks_bar_view.h"
#include "ash/wm/desks/expanded_desks_bar_button.h"
+#include "ash/wm/desks/templates/saved_desk_controller.h"
#include "ash/wm/desks/templates/saved_desk_dialog_controller.h"
#include "ash/wm/desks/templates/saved_desk_icon_container.h"
#include "ash/wm/desks/templates/saved_desk_item_view.h"
@@ -146,6 +147,17 @@
SavedDeskIconViewTestApi::~SavedDeskIconViewTestApi() = default;
+SavedDeskControllerTestApi::SavedDeskControllerTestApi(
+ SavedDeskController* saved_desk_controller)
+ : saved_desk_controller_(saved_desk_controller) {}
+
+SavedDeskControllerTestApi::~SavedDeskControllerTestApi() = default;
+
+void SavedDeskControllerTestApi::SetAdminTemplate(
+ std::unique_ptr<DeskTemplate> admin_template) {
+ saved_desk_controller_->SetAdminTemplateForTesting(std::move(admin_template));
+}
+
std::vector<SavedDeskItemView*> GetItemViewsFromDeskLibrary(
const OverviewGrid* overview_grid) {
SavedDeskLibraryView* saved_desk_library_view =
diff --git a/ash/wm/desks/templates/saved_desk_test_util.h b/ash/wm/desks/templates/saved_desk_test_util.h
index 0af8fd2..5baf56b 100644
--- a/ash/wm/desks/templates/saved_desk_test_util.h
+++ b/ash/wm/desks/templates/saved_desk_test_util.h
@@ -25,6 +25,7 @@
class IconButton;
class OverviewGrid;
class PillButton;
+class SavedDeskController;
class SavedDeskPresenter;
// Wrapper for `SavedDeskPresenter` that exposes internal state to test
@@ -142,6 +143,22 @@
const SavedDeskIconView* saved_desk_icon_view_;
};
+// Test API for `SavedDeskController`.
+class SavedDeskControllerTestApi {
+ public:
+ explicit SavedDeskControllerTestApi(
+ SavedDeskController* saved_desk_controller);
+ SavedDeskControllerTestApi(const SavedDeskControllerTestApi&) = delete;
+ SavedDeskControllerTestApi& operator=(const SavedDeskControllerTestApi&) =
+ delete;
+ ~SavedDeskControllerTestApi();
+
+ void SetAdminTemplate(std::unique_ptr<DeskTemplate> admin_template);
+
+ private:
+ SavedDeskController* saved_desk_controller_;
+};
+
// Returns all saved desk item views from the desk library on the given
// `overview_grid`.
std::vector<SavedDeskItemView*> GetItemViewsFromDeskLibrary(
diff --git a/ash/wm/desks/templates/saved_desk_util.cc b/ash/wm/desks/templates/saved_desk_util.cc
index a4409d2..1b25b82e 100644
--- a/ash/wm/desks/templates/saved_desk_util.cc
+++ b/ash/wm/desks/templates/saved_desk_util.cc
@@ -9,8 +9,10 @@
#include "ash/public/cpp/session/session_types.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
+#include "ash/wm/desks/templates/saved_desk_constants.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_session.h"
+#include "components/app_restore/window_properties.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
@@ -81,5 +83,12 @@
return presenter;
}
+bool IsAdminTemplateWindow(aura::Window* window) {
+ const int32_t* activation_index =
+ window->GetProperty(app_restore::kActivationIndexKey);
+ return activation_index &&
+ *activation_index <= kAdminTemplateStartingActivationIndex;
+}
+
} // namespace saved_desk_util
} // namespace ash
diff --git a/ash/wm/desks/templates/saved_desk_util.h b/ash/wm/desks/templates/saved_desk_util.h
index 101fc3d..7e026d0 100644
--- a/ash/wm/desks/templates/saved_desk_util.h
+++ b/ash/wm/desks/templates/saved_desk_util.h
@@ -9,6 +9,10 @@
class PrefRegistrySimple;
+namespace aura {
+class Window;
+}
+
namespace ash {
class SavedDeskDialogController;
@@ -30,6 +34,9 @@
// Will return null if overview mode is not active.
ASH_EXPORT SavedDeskPresenter* GetSavedDeskPresenter();
+// Returns true if `window` was launched from an admin template.
+bool IsAdminTemplateWindow(aura::Window* window);
+
} // namespace saved_desk_util
} // namespace ash
diff --git a/ash/wm/window_restore/window_restore_controller.cc b/ash/wm/window_restore/window_restore_controller.cc
index 6bf0dff..91375c9 100644
--- a/ash/wm/window_restore/window_restore_controller.cc
+++ b/ash/wm/window_restore/window_restore_controller.cc
@@ -15,6 +15,7 @@
#include "ash/shell.h"
#include "ash/wm/container_finder.h"
#include "ash/wm/desks/desks_util.h"
+#include "ash/wm/desks/templates/saved_desk_util.h"
#include "ash/wm/float/float_controller.h"
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
@@ -230,13 +231,26 @@
WindowRestoreController::GetWindowToInsertBefore(
aura::Window* window,
const std::vector<aura::Window*>& windows) {
- int32_t* activation_index =
+ const int32_t* activation_index =
window->GetProperty(app_restore::kActivationIndexKey);
DCHECK(activation_index);
+ // If this is an admin template window, it should be placed on top of existing
+ // windows (but relative to other desk template windows).
+ if (saved_desk_util::IsAdminTemplateWindow(window)) {
+ for (auto it = windows.begin(); it != windows.end(); ++it) {
+ const int32_t* next_activation_index =
+ (*it)->GetProperty(app_restore::kActivationIndexKey);
+ if (next_activation_index && *activation_index > *next_activation_index) {
+ return it;
+ }
+ }
+ return windows.end();
+ }
+
auto it = windows.begin();
while (it != windows.end()) {
- int32_t* next_activation_index =
+ const int32_t* next_activation_index =
(*it)->GetProperty(app_restore::kActivationIndexKey);
if (!next_activation_index || *activation_index > *next_activation_index) {
diff --git a/chrome/browser/ui/ash/desks/desks_client_browsertest.cc b/chrome/browser/ui/ash/desks/desks_client_browsertest.cc
index d7799ee..6c5f92b 100644
--- a/chrome/browser/ui/ash/desks/desks_client_browsertest.cc
+++ b/chrome/browser/ui/ash/desks/desks_client_browsertest.cc
@@ -21,6 +21,7 @@
#include "ash/wm/desks/desk.h"
#include "ash/wm/desks/desks_controller.h"
#include "ash/wm/desks/desks_test_util.h"
+#include "ash/wm/desks/templates/saved_desk_controller.h"
#include "ash/wm/desks/templates/saved_desk_metrics_util.h"
#include "ash/wm/desks/templates/saved_desk_presenter.h"
#include "ash/wm/desks/templates/saved_desk_test_util.h"
@@ -3384,3 +3385,106 @@
// Verify that the admin templates is removed.
EXPECT_FALSE(ContainUuidInTemplates(admin_template_uuid, GetDeskTemplates()));
}
+
+class AdminTemplateTest : public extensions::PlatformAppBrowserTest {
+ public:
+ AdminTemplateTest()
+ : scoped_feature_list_(ash::features::kAppLaunchAutomation) {
+ // Suppress the multitask menu nudge as we'll be checking the stacking order
+ // and the count of the active desk children.
+ chromeos::MultitaskMenuNudgeController::SetSuppressNudgeForTesting(true);
+ }
+ AdminTemplateTest(const AdminTemplateTest&) = delete;
+ AdminTemplateTest& operator=(const AdminTemplateTest&) = delete;
+ ~AdminTemplateTest() override = default;
+
+ // The definition of an admin template for a test.
+ struct AdminTemplateDefinition {
+ struct WindowDefinition {
+ std::vector<std::string> urls;
+ };
+
+ std::vector<WindowDefinition> windows;
+ };
+
+ // Creates an admin template with the windows and URLs given by `definition`.
+ std::unique_ptr<ash::DeskTemplate> CreateAdminTemplate(
+ const AdminTemplateDefinition& definition) {
+ // Common set of bounds to use for now. Later, get from `definition`.
+ base::Value::List bounds;
+ for (int b : {100, 50, 400, 300}) {
+ bounds.Append(b);
+ }
+
+ base::Value::Dict windows;
+ for (size_t i = 0; i != definition.windows.size(); ++i) {
+ base::Value::Dict window;
+ window.Set("title", "Chrome");
+ window.Set("window_state_type", 0);
+ window.Set("bounds", bounds.Clone());
+
+ base::Value::List urls;
+ for (const std::string& url : definition.windows[i].urls) {
+ urls.Append(url);
+ }
+ window.Set("urls", std::move(urls));
+
+ windows.Set(base::NumberToString(i + 1), std::move(window));
+ }
+
+ base::Value::Dict root;
+ root.Set(app_constants::kChromeAppId, std::move(windows));
+
+ auto admin_template = std::make_unique<ash::DeskTemplate>(
+ base::GUID::GenerateRandomV4(), ash::DeskTemplateSource::kPolicy,
+ "Admin template", base::Time::Now(), ash::DeskTemplateType::kTemplate);
+
+ admin_template->set_desk_restore_data(
+ std::make_unique<app_restore::RestoreData>(
+ base::Value(std::move(root))));
+
+ return admin_template;
+ }
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// TODO(b/273803538): Add tests for lacros.
+IN_PROC_BROWSER_TEST_F(AdminTemplateTest, LaunchAdminTemplate) {
+ // Launch an admin template with a single browser. Verifies that a browser was
+ // actually launched.
+ auto admin_template =
+ CreateAdminTemplate({.windows = {{.urls = {kExampleUrl1}}}});
+ ASSERT_NE(admin_template, nullptr);
+
+ base::GUID template_uuid = admin_template->uuid();
+
+ auto* saved_desk_controller = ash::Shell::Get()->saved_desk_controller();
+ ash::SavedDeskControllerTestApi(saved_desk_controller)
+ .SetAdminTemplate(std::move(admin_template));
+
+ saved_desk_controller->LaunchAdminTemplate(template_uuid);
+
+ // Verify that there are two browsers (one from the suite and one from the
+ // test), and verify that our launched browser is stacked on top.
+ Browser* new_browser = FindLaunchedBrowserByURLs({GURL(kExampleUrl1)});
+ ASSERT_TRUE(new_browser);
+
+ aura::Window* old_browser_window = browser()->window()->GetNativeWindow();
+ aura::Window* new_browser_window = new_browser->window()->GetNativeWindow();
+
+ // Both browsers should be on the same desk.
+ ASSERT_EQ(old_browser_window->parent(), new_browser_window->parent());
+
+ // Verify that the new browser window is stacked in front of the
+ // existing. Children are ordered from bottommost to topmost. We therefore
+ // expect the new window to have an index that is higher than the old.
+ const auto& container = new_browser_window->parent()->children();
+ size_t new_index =
+ base::ranges::find(container, new_browser_window) - container.begin();
+ size_t old_index =
+ base::ranges::find(container, old_browser_window) - container.begin();
+
+ EXPECT_GT(new_index, old_index);
+}
diff --git a/components/app_restore/restore_data.h b/components/app_restore/restore_data.h
index f65e965..356129a9 100644
--- a/components/app_restore/restore_data.h
+++ b/components/app_restore/restore_data.h
@@ -192,6 +192,9 @@
const AppIdToLaunchList& app_id_to_launch_list() const {
return app_id_to_launch_list_;
}
+ AppIdToLaunchList& mutable_app_id_to_launch_list() {
+ return app_id_to_launch_list_;
+ }
void set_removing_desk_guid(const base::Uuid& removing_desk_guid) {
removing_desk_guid_ = removing_desk_guid;
