From 2ba5d60ee541c5d4db3ea54b838a11bae45ff1e5 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 21 Jul 2023 12:26:19 -0400 Subject: [PATCH] Exposed ApplicationSelector and wrote better tests --- .../miral}/miral/application_selector.h | 8 +- src/miral/CMakeLists.txt | 2 +- src/miral/application_selector.cpp | 20 +- src/miral/basic_window_manager.cpp | 1 - src/miral/minimal_window_manager.cpp | 2 +- tests/miral/application_selector.cpp | 186 +++++++----------- 6 files changed, 102 insertions(+), 117 deletions(-) rename {src => include/miral}/miral/application_selector.h (94%) diff --git a/src/miral/application_selector.h b/include/miral/miral/application_selector.h similarity index 94% rename from src/miral/application_selector.h rename to include/miral/miral/application_selector.h index df560823115..0a6b96c3e17 100644 --- a/src/miral/application_selector.h +++ b/include/miral/miral/application_selector.h @@ -17,8 +17,8 @@ #ifndef MIR_APPLICATION_SELECTOR_H #define MIR_APPLICATION_SELECTOR_H -#include -#include +#include "window_manager_tools.h" +#include "application.h" namespace miral { @@ -66,6 +66,10 @@ class ApplicationSelector /// \return true if it is running, otherwise false auto is_active() -> bool; + /// Retrieve the focused application. + /// \returns The focused application + auto get_focused() -> Application; + private: WindowManagerTools tools; diff --git a/src/miral/CMakeLists.txt b/src/miral/CMakeLists.txt index 1b67ee3c4f5..1138989d57b 100644 --- a/src/miral/CMakeLists.txt +++ b/src/miral/CMakeLists.txt @@ -29,7 +29,7 @@ add_library(miral-internal STATIC join_client_threads.h window_info_defaults.h window_specification_internal.cpp window_specification_internal.h - application_selector.cpp application_selector.h + application_selector.cpp ) # Already implied by the linker's symbol version script, but can avoid accidents diff --git a/src/miral/application_selector.cpp b/src/miral/application_selector.cpp index 7cceb0593c9..9a4b8cc4e8b 100644 --- a/src/miral/application_selector.cpp +++ b/src/miral/application_selector.cpp @@ -14,7 +14,7 @@ * along with this program. If not, see . */ -#include "application_selector.h" +#include #include #include #include @@ -28,7 +28,6 @@ ApplicationSelector::ApplicationSelector(const miral::WindowManagerTools& in_too void ApplicationSelector::advise_new_app(Application const& application) { focus_list.push_back(application); - printf("Online: %d\n", pid_of(application)); } void ApplicationSelector::advise_focus_gained(WindowInfo const& window_info) @@ -88,6 +87,18 @@ void ApplicationSelector::advise_delete_app(Application const& application) return; } + if (application == selected) + { + auto new_selected_it = it + 1; + if (new_selected_it == focus_list.end()) + new_selected_it = focus_list.begin(); + + if (focus_list.size() > 1) + selected = *new_selected_it; + else + selected = nullptr; + } + focus_list.erase(it); } @@ -163,4 +174,9 @@ void ApplicationSelector::cancel() auto ApplicationSelector::is_active() -> bool { return originally_selected != nullptr; +} + +auto ApplicationSelector::get_focused() -> Application +{ + return selected; } \ No newline at end of file diff --git a/src/miral/basic_window_manager.cpp b/src/miral/basic_window_manager.cpp index 6c8f5d3230a..0b90433d043 100644 --- a/src/miral/basic_window_manager.cpp +++ b/src/miral/basic_window_manager.cpp @@ -244,7 +244,6 @@ void miral::BasicWindowManager::remove_window(Application const& application, mi policy->advise_delete_window(info); info_for(application).remove_window(info.window()); - printf("erased\n"); mru_active_windows.erase(info.window()); fullscreen_surfaces.erase(info.window()); for (auto& area : display_areas) diff --git a/src/miral/minimal_window_manager.cpp b/src/miral/minimal_window_manager.cpp index 335e7a52afa..e47d4521f8c 100644 --- a/src/miral/minimal_window_manager.cpp +++ b/src/miral/minimal_window_manager.cpp @@ -17,7 +17,7 @@ #include #include #include -#include "application_selector.h" +#include #include #include diff --git a/tests/miral/application_selector.cpp b/tests/miral/application_selector.cpp index 5b2dc550ee5..d62e813a831 100644 --- a/tests/miral/application_selector.cpp +++ b/tests/miral/application_selector.cpp @@ -14,7 +14,7 @@ * along with this program. If not, see . */ -#include "application_selector.h" +#include #include "test_window_manager_tools.h" #include @@ -30,134 +30,100 @@ struct ApplicationSelectorTest : mt::TestWindowManagerTools : mt::TestWindowManagerTools(), application_selector(window_manager_tools) {} + + auto create_window() -> Window + { + mir::shell::SurfaceSpecification creation_parameters; + creation_parameters.type = mir_window_type_normal; + creation_parameters.focus_mode = MirFocusMode::mir_focus_mode_focusable; + creation_parameters.set_size({600, 400}); + auto window = create_and_select_window(creation_parameters); + + // Simulates a new application opening and gaining focus + application_selector.advise_new_app(window.application()); + application_selector.advise_focus_gained(window_manager_tools.info_for(window)); + return window; + } + + auto create_window_list(int num_windows) -> std::vector + { + std::vector window_list; + for (int index = 0; index < num_windows; index++) + { + window_list.push_back(create_window()); + } + + return window_list; + } }; -/// Testing if we can cycle through 3 windows and then select one -/// with a "forward" selection strategy. -TEST_F(ApplicationSelectorTest, run_forward) +TEST_F(ApplicationSelectorTest, focused_app_is_latest_selected) { - // Create three windows on our display - mir::shell::SurfaceSpecification creation_parameters; - creation_parameters.name = "window1"; - creation_parameters.type = mir_window_type_normal; - creation_parameters.focus_mode = MirFocusMode::mir_focus_mode_focusable; - creation_parameters.set_size({600, 400}); - auto window1 = create_and_select_window(creation_parameters); - creation_parameters.name = "window2"; - auto window2 = create_and_select_window(creation_parameters); - creation_parameters.name = "window3"; - auto window3 = create_and_select_window(creation_parameters); - - // Inform the application selector about the windows - application_selector.advise_new_app(window1.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window1)); - application_selector.advise_new_app(window2.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window2)); - application_selector.advise_new_app(window3.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window3)); - - // Make sure that window3 (the last one added) is the active one - EXPECT_TRUE(window3 == basic_window_manager.active_window()); - - // Start the selector and assert that the selected application is window2 - auto application = application_selector.next(false); - EXPECT_TRUE(application == window2.application()); - - // Call next and assert that we have moved to window1 - application = application_selector.next(false); - EXPECT_TRUE(application == window1.application()); - - // Stop the selector and assert that window2 is selected - application = application_selector.next(false); - EXPECT_TRUE(application == window3.application()); + auto windows = create_window_list(3); + EXPECT_TRUE(windows[2].application() == application_selector.get_focused()); +} - application_selector.advise_delete_app(window1.application()); - application_selector.advise_delete_app(window2.application()); - application_selector.advise_delete_app(window3.application()); +TEST_F(ApplicationSelectorTest, moving_forward_once_selects_previously_selected_application) +{ + auto windows = create_window_list(3); + auto application = application_selector.next(false); + EXPECT_TRUE(application == windows[1].application()); } -/// Testing if we can cycle through 3 windows and then select one -/// with a "reverse" selection strategy. -TEST_F(ApplicationSelectorTest, run_backward) +TEST_F(ApplicationSelectorTest, moving_backward_once_selects_least_recently_selected_app) { - // Create three windows on our display - mir::shell::SurfaceSpecification creation_parameters; - creation_parameters.name = "window1"; - creation_parameters.type = mir_window_type_normal; - creation_parameters.focus_mode = MirFocusMode::mir_focus_mode_focusable; - creation_parameters.set_size({600, 400}); - auto window1 = create_and_select_window(creation_parameters); - creation_parameters.name = "window2"; - auto window2 = create_and_select_window(creation_parameters); - creation_parameters.name = "window3"; - auto window3 = create_and_select_window(creation_parameters); - - // Inform the application selector about the windows - application_selector.advise_new_app(window1.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window1)); - application_selector.advise_new_app(window2.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window2)); - application_selector.advise_new_app(window3.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window3)); - - // Make sure that window3 (the last one added) is the active one - EXPECT_TRUE(window3 == basic_window_manager.active_window()); - - // Start the selector and assert that the raised application is window2 + auto windows = create_window_list(3); auto application = application_selector.next(true); - EXPECT_TRUE(application == window1.application()); - - // Call next and assert that we have moved to window1 - application = application_selector.next(true); - EXPECT_TRUE(application == window2.application()); + EXPECT_TRUE(application == windows[0].application()); +} - // Stop the selector and assert that window1 is selected - application = application_selector.complete(); - EXPECT_TRUE(application == window2.application()); +TEST_F(ApplicationSelectorTest, can_move_forward_through_all_windows_in_the_list_to_arrive_at_original_window) +{ + int num_windows = 3; + auto windows = create_window_list(num_windows); + for (int i = 0; i < 3; i++) + application_selector.next(true); + auto application = application_selector.complete(); + EXPECT_TRUE(application == windows[2].application()); +} - application_selector.advise_delete_app(window1.application()); - application_selector.advise_delete_app(window2.application()); - application_selector.advise_delete_app(window3.application()); +TEST_F(ApplicationSelectorTest, new_selector_is_not_active) +{ + auto windows = create_window_list(3); + EXPECT_FALSE(application_selector.is_active()); } -/// Testing if we can start the selector when there are no sessions started. -TEST_F(ApplicationSelectorTest, run_with_no_sessions) +TEST_F(ApplicationSelectorTest, calling_next_with_no_windows_results_in_nullptr) { - // Start the selector and assert that an application could not be raised auto application = application_selector.next(false); EXPECT_TRUE(application == nullptr); EXPECT_FALSE(application_selector.is_active()); } -/// Testing if we can cycle through a single window successfully -TEST_F(ApplicationSelectorTest, run_in_circle) +TEST_F(ApplicationSelectorTest, focusing_a_new_window_while_application_selector_is_active_focuses_the_new_app) { - // Create three windows on our display - mir::shell::SurfaceSpecification creation_parameters; - creation_parameters.name = "window1"; - creation_parameters.type = mir_window_type_normal; - creation_parameters.focus_mode = MirFocusMode::mir_focus_mode_focusable; - creation_parameters.set_size({600, 400}); - auto window1 = create_and_select_window(creation_parameters); - - // Inform the application selector about the window - application_selector.advise_new_app(window1.application()); - application_selector.advise_focus_gained(window_manager_tools.info_for(window1)); - - // Make sure that window3 (the last one added) is the active one - EXPECT_TRUE(window1 == basic_window_manager.active_window()); - - // Start the selector and assert that the raised application is window2 - auto application = application_selector.next(true); - EXPECT_TRUE(application == window1.application()); - - // Call next and assert that we have moved to window1 - application = application_selector.next(true); - EXPECT_TRUE(application == window1.application()); + auto windows = create_window_list(3); + application_selector.next(true); + auto new_window = create_window(); + auto focused = application_selector.get_focused(); + EXPECT_TRUE(focused == new_window.application()); +} - // Stop the selector and assert that window1 is selected - application = application_selector.complete(); - EXPECT_TRUE(application == window1.application()); +TEST_F(ApplicationSelectorTest, apps_added_while_selection_is_active_get_added_to_end_of_list) +{ + auto windows = create_window_list(3); + application_selector.next(false); + auto new_window = create_window(); + auto application = application_selector.next(true); + EXPECT_TRUE(application == windows[0].application()); +} - application_selector.advise_delete_app(window1.application()); +TEST_F(ApplicationSelectorTest, deleting_selected_app_makes_the_next_app_selected) +{ + auto windows = create_window_list(3); + application_selector.next(false); // windows[1] is selected + application_selector.advise_focus_gained(window_manager_tools.info_for(windows[1])); + application_selector.advise_delete_app(windows[1].application()); + auto application = application_selector.get_focused(); + EXPECT_TRUE(application == windows[0].application()); } \ No newline at end of file