Skip to content

Commit

Permalink
glanceables: Use a property for overview UI elements
Browse files Browse the repository at this point in the history
Overview UI elements have special properties:
  - not in MRU tracker
  - activation does not exit overview
  - moved to active desk on desk change

Previously, these windows were being marked as special windows using
IDs. This CL uses a window property as a more scalable approach as
we will be adding more shortly. And we wouldn't dig into internals in
the case of the system tray.

Test: existing
Bug: b/304618108
Change-Id: Ic9ad206a897e322d235916ff0cf4fa5064a5d102
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4931438
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209181}
  • Loading branch information
Sammie Quon authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent 4b01666 commit 5417507
Show file tree
Hide file tree
Showing 17 changed files with 79 additions and 160 deletions.
11 changes: 0 additions & 11 deletions ash/public/cpp/shell_window_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,6 @@ enum NonContainerWindowId {
// window (if one exists).
kShellWindowId_CaptureModeFolderSelectionDialogOwner,

// The window that shows the "Save desk as template" button and `Save desk for
// later` button below the Virtual Desks bar. There's only one such window on
// each display when overview mode is active.
kShellWindowId_SaveDeskButtonContainer,

// The window that shows the Saved Desk Library in overview.
kShellWindowId_SavedDeskLibraryWindow,

// The window that shows the "No recent items" label in overview.
kShellWindowId_OverviewNoWindowsLabelWindow,

// The window that notifies the user that an admin user was present on the
// host device when the remote desktop session was curtained.
kShellWindowId_AdminWasPresentNotificationWindow,
Expand Down
2 changes: 2 additions & 0 deletions ash/style/rounded_label_widget.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "ash/public/cpp/window_properties.h"
#include "ash/style/rounded_label.h"
#include "ash/wm/overview/scoped_overview_animation_settings.h"
#include "ash/wm/window_properties.h"
#include "ui/aura/window.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/display/screen.h"
Expand Down Expand Up @@ -36,6 +37,7 @@ void RoundedLabelWidget::Init(InitParams params) {
widget_params.parent = params.parent;
widget_params.init_properties_container.SetProperty(kHideInDeskMiniViewKey,
true);
widget_params.init_properties_container.SetProperty(kOverviewUiKey, true);
set_focus_on_creation(false);
views::Widget::Init(std::move(widget_params));

Expand Down
3 changes: 3 additions & 0 deletions ash/system/message_center/ash_message_popup_collection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "ash/system/tray/tray_utils.h"
#include "ash/system/unified/unified_system_tray.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/work_area_insets.h"
#include "base/check.h"
#include "base/i18n/rtl.h"
Expand Down Expand Up @@ -347,6 +348,8 @@ void AshMessagePopupCollection::ConfigureWidgetInitParamsForContainer(
init_params->activatable = views::Widget::InitParams::Activatable::kYes;
init_params->name = kMessagePopupWidgetName;
init_params->corner_radius = kMessagePopupCornerRadius;
init_params->init_properties_container.SetProperty(
kStayInOverviewOnActivationKey, true);
Shell::Get()->focus_cycler()->AddWidget(widget);
widget->AddObserver(this);
tracked_widgets_.insert(widget);
Expand Down
7 changes: 5 additions & 2 deletions ash/system/message_center/unified_message_center_bubble.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "ash/system/unified/unified_system_tray_bubble.h"
#include "ash/system/unified/unified_system_tray_controller.h"
#include "ash/system/unified/unified_system_tray_view.h"
#include "ash/wm/window_properties.h"
#include "base/i18n/rtl.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/compositor/layer.h"
Expand Down Expand Up @@ -101,11 +102,13 @@ void UnifiedMessageCenterBubble::ShowBubble() {
bubble_widget_->AddObserver(this);
TrayBackgroundView::InitializeBubbleAnimations(bubble_widget_);

aura::Window* bubble_window = bubble_widget_->GetNativeWindow();
bubble_window->SetProperty(kStayInOverviewOnActivationKey, true);

// Stack system tray bubble's window above message center's window, such that
// message center's shadow will not cover on system tray.
tray_->GetBubbleWindowContainer()->StackChildAbove(
tray_->bubble()->GetBubbleWidget()->GetNativeWindow(),
bubble_widget_->GetNativeWindow());
tray_->bubble()->GetBubbleWidget()->GetNativeWindow(), bubble_window);

if (features::IsSystemTrayShadowEnabled()) {
// Create a shadow for bubble widget.
Expand Down
7 changes: 2 additions & 5 deletions ash/wm/desks/desk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <utility>

#include "ash/constants/app_types.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/public/cpp/window_properties.h"
#include "ash/scoped_animation_disabler.h"
#include "ash/shell.h"
Expand All @@ -19,6 +18,7 @@
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/window_positioner.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_util.h"
#include "ash/wm/workspace/backdrop_controller.h"
Expand Down Expand Up @@ -74,10 +74,7 @@ void UpdateBackdropController(aura::Window* desk_container) {
}

bool IsOverviewUiWindow(aura::Window* window) {
return window->GetId() == kShellWindowId_DesksBarWindow ||
window->GetId() == kShellWindowId_SaveDeskButtonContainer ||
window->GetId() == kShellWindowId_OverviewNoWindowsLabelWindow ||
window->GetId() == kShellWindowId_SavedDeskLibraryWindow;
return window->GetProperty(kOverviewUiKey);
}

// Returns true if |window| can be managed by the desk, and therefore can be
Expand Down
5 changes: 3 additions & 2 deletions ash/wm/desks/desk_bar_view_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "ash/wm/overview/overview_session.h"
#include "ash/wm/overview/overview_utils.h"
#include "ash/wm/window_positioning_utils.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/work_area_insets.h"
#include "base/check.h"
#include "base/functional/bind.h"
Expand Down Expand Up @@ -822,7 +823,7 @@ std::unique_ptr<views::Widget> DeskBarViewBase::CreateDeskWidget(
// Even though this widget exists on the active desk container, it should
// not show up in the MRU list, and it should not be mirrored in the desks
// mini_views.
params.init_properties_container.SetProperty(kExcludeInMruKey, true);
params.init_properties_container.SetProperty(kOverviewUiKey, true);
params.init_properties_container.SetProperty(kHideInDeskMiniViewKey, true);
} else {
// Desk button desk bar should live under the shelf bubble container on
Expand All @@ -836,7 +837,7 @@ std::unique_ptr<views::Widget> DeskBarViewBase::CreateDeskWidget(

auto* window = widget->GetNativeWindow();
window->SetId(kShellWindowId_DesksBarWindow);
::wm::SetWindowVisibilityAnimationTransition(window, ::wm::ANIMATE_NONE);
wm::SetWindowVisibilityAnimationTransition(window, wm::ANIMATE_NONE);

return widget;
}
Expand Down
6 changes: 2 additions & 4 deletions ash/wm/desks/desks_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "ash/public/cpp/shelf_model.h"
#include "ash/public/cpp/shelf_prefs.h"
#include "ash/public/cpp/shelf_types.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/public/cpp/system/toast_data.h"
#include "ash/public/cpp/system/toast_manager.h"
#include "ash/public/cpp/window_properties.h"
Expand Down Expand Up @@ -44,6 +43,7 @@
#include "ash/wm/switchable_windows.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_cycle/window_cycle_controller.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_restore/window_restore_controller.h"
#include "ash/wm/window_restore/window_restore_util.h"
#include "ash/wm/window_state.h"
Expand Down Expand Up @@ -1304,9 +1304,7 @@ const Desk* DesksController::CreateNewDeskForSavedDesk(
// list.
auto active_desk_windows = active_desk_->windows();
for (aura::Window* window : active_desk_windows) {
if (window->GetId() == kShellWindowId_DesksBarWindow ||
window->GetId() == kShellWindowId_SaveDeskButtonContainer ||
window->GetId() == kShellWindowId_SavedDeskLibraryWindow) {
if (window->GetProperty(kOverviewUiKey)) {
aura::Window* destination_container =
desk->GetDeskContainerForRoot(window->GetRootWindow());
destination_container->AddChild(window);
Expand Down
35 changes: 4 additions & 31 deletions ash/wm/desks/desks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@
#include "ui/views/background.h"
#include "ui/views/controls/button/button.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/widget/any_widget_observer.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"
#include "ui/views/window/client_view.h"
Expand Down Expand Up @@ -2719,35 +2720,6 @@ TEST_F(DesksWithMultiDisplayOverview, DropOnOtherDeskInOtherDisplay) {
EXPECT_TRUE(base::Contains(desk_2->windows(), win.get()));
}

// DesksBarVisibilityObserver waits for the Desks Bar on the observed root
// window to become visible.
class DesksBarVisibilityObserver : public aura::WindowObserver {
public:
explicit DesksBarVisibilityObserver(aura::Window* root_window)
: root_window_(root_window) {
root_window_->AddObserver(this);
}
DesksBarVisibilityObserver(const DesksBarVisibilityObserver&) = delete;
DesksBarVisibilityObserver& operator=(const DesksBarVisibilityObserver&) =
delete;
~DesksBarVisibilityObserver() override {
DCHECK(root_window_);
root_window_->RemoveObserver(this);
}

void Wait() { run_loop_.Run(); }

void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
if (visible && window->GetId() == kShellWindowId_DesksBarWindow) {
run_loop_.Quit();
}
}

private:
base::RunLoop run_loop_;
raw_ptr<aura::Window> root_window_;
};

// Tests that closing a desk while in overview before the overview starting
// animation finishes on a second display does not cause a crash. Regression
// test for https://crbug.com/1346154.
Expand Down Expand Up @@ -2786,10 +2758,11 @@ TEST_F(DesksWithMultiDisplayOverview, CloseDeskBeforeAnimationFinishes) {
// if that miniview can't be found.
// To prevent flakiness in this test, we wait until `desks_bar_view_2` is
// initialized and shown before we check the state.
DesksBarVisibilityObserver desks_bar_2_observer(root_windows[1]);
views::NamedWidgetShownWaiter waiter(views::test::AnyWidgetTestPasskey{},
"OverviewDeskBarWidget");
auto* desk_1_mini_view = desks_bar_view_1->mini_views()[0];
CloseDeskFromMiniView(desk_1_mini_view, GetEventGenerator());
desks_bar_2_observer.Wait();
waiter.WaitIfNeededAndGet();

// Verify that we are still in overview mode and that both desks bars are in
// the zero state.
Expand Down
3 changes: 3 additions & 0 deletions ash/wm/desks/templates/saved_desk_dialog_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "ash/wm/desks/templates/saved_desk_metrics_util.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_grid.h"
#include "ash/wm/window_properties.h"
#include "base/memory/raw_ptr.h"
#include "chromeos/constants/chromeos_features.h"
#include "ui/aura/env.h"
Expand Down Expand Up @@ -377,6 +378,7 @@ void SavedDeskDialogController::CreateDialogWidget(
views::Widget::InitParams params;
params.type = views::Widget::InitParams::Type::TYPE_WINDOW_FRAMELESS;
params.context = root_window;
params.init_properties_container.SetProperty(kOverviewUiKey, true);
params.delegate = dialog.release();

dialog_widget_ = new views::Widget();
Expand All @@ -385,6 +387,7 @@ void SavedDeskDialogController::CreateDialogWidget(
dialog_widget_ = views::DialogDelegate::CreateDialogWidget(
std::move(dialog),
/*context=*/root_window, /*parent=*/nullptr);
dialog_widget_->GetNativeWindow()->SetProperty(kOverviewUiKey, true);
}
dialog_widget_->GetNativeWindow()->SetName("TemplateDialogForTesting");
dialog_widget_->Show();
Expand Down
9 changes: 4 additions & 5 deletions ash/wm/desks/templates/saved_desk_library_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_grid.h"
#include "ash/wm/overview/overview_grid_event_handler.h"
#include "ash/wm/window_properties.h"
#include "base/functional/callback_helpers.h"
#include "chromeos/constants/chromeos_features.h"
#include "ui/aura/window_targeter.h"
Expand Down Expand Up @@ -251,18 +252,16 @@ SavedDeskLibraryView::CreateSavedDeskLibraryWidget(aura::Window* root) {
root, desks_controller->GetDeskIndex(desks_controller->active_desk()));
params.name = "SavedDeskLibraryWidget";
params.init_properties_container.SetProperty(kHideInDeskMiniViewKey, true);
params.init_properties_container.SetProperty(kExcludeInMruKey, true);
params.init_properties_container.SetProperty(kOverviewUiKey, true);

auto widget = std::make_unique<views::Widget>(std::move(params));
widget->SetContentsView(std::make_unique<SavedDeskLibraryView>());

// Not opaque since we want to view the contents of the layer behind.
widget->GetLayer()->SetFillsBoundsOpaquely(false);

widget->GetNativeWindow()->SetId(kShellWindowId_SavedDeskLibraryWindow);

::wm::SetWindowVisibilityAnimationTransition(widget->GetNativeWindow(),
::wm::ANIMATE_NONE);
wm::SetWindowVisibilityAnimationTransition(widget->GetNativeWindow(),
wm::ANIMATE_NONE);
return widget;
}

Expand Down
35 changes: 3 additions & 32 deletions ash/wm/desks/templates/saved_desk_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1208,35 +1208,6 @@ TEST_F(SavedDeskTest, SaveDeskAsTemplateButtonShowsSavedDeskGrid) {
EXPECT_TRUE(GetOverviewGridList()[0]->IsShowingSavedDeskLibrary());
}

// SaveDeskButtonContainerVisibilityObserver waits for the save desk buttons on
// the observed root window to become visible.
class SaveDeskButtonContainerVisibilityObserver : public aura::WindowObserver {
public:
explicit SaveDeskButtonContainerVisibilityObserver(aura::Window* root_window)
: root_window_(root_window) {
root_window_->AddObserver(this);
}
SaveDeskButtonContainerVisibilityObserver(
const SaveDeskButtonContainerVisibilityObserver&) = delete;
SaveDeskButtonContainerVisibilityObserver& operator=(
const SaveDeskButtonContainerVisibilityObserver&) = delete;
~SaveDeskButtonContainerVisibilityObserver() override {
DCHECK(root_window_);
root_window_->RemoveObserver(this);
}

void Wait() { run_loop_.Run(); }

void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
if (visible && window->GetId() == kShellWindowId_SaveDeskButtonContainer)
run_loop_.Quit();
}

private:
base::RunLoop run_loop_;
raw_ptr<aura::Window> root_window_;
};

// Tests that the desks bar is created before the save desk buttons are visible.
// Regression test for https://crbug.com/1349971.
TEST_F(SavedDeskTest, DesksBarLoadsBeforeSaveDeskButtons) {
Expand All @@ -1250,10 +1221,10 @@ TEST_F(SavedDeskTest, DesksBarLoadsBeforeSaveDeskButtons) {

aura::Window* root_window = Shell::GetPrimaryRootWindow();

SaveDeskButtonContainerVisibilityObserver button_container_observer(
root_window);
views::NamedWidgetShownWaiter waiter(views::test::AnyWidgetTestPasskey{},
"SaveDeskButtonContainerWidget");
EnterOverview();
button_container_observer.Wait();
waiter.WaitIfNeededAndGet();

// Ensure we are in overview.
auto* overview_controller = Shell::Get()->overview_controller();
Expand Down
11 changes: 9 additions & 2 deletions ash/wm/mru_window_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ash/wm/desks/desks_util.h"
#include "ash/wm/float/float_controller.h"
#include "ash/wm/switchable_windows.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_restore/window_restore_controller.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_util.h"
Expand Down Expand Up @@ -60,8 +61,13 @@ class ScopedWindowClosingObserver : public aura::WindowObserver {
};

bool IsNonSysModalWindowConsideredActivatable(aura::Window* window) {
if (window->GetProperty(ash::kExcludeInMruKey))
if (window->GetProperty(kExcludeInMruKey)) {
return false;
}

if (window->GetProperty(kOverviewUiKey)) {
return false;
}

ScopedWindowClosingObserver observer(window);
AshFocusRules* focus_rules = Shell::Get()->focus_rules();
Expand Down Expand Up @@ -215,7 +221,8 @@ bool CanIncludeWindowInMruList(aura::Window* window) {
return true;

return wm::CanActivateWindow(window) &&
!window->GetProperty(ash::kExcludeInMruKey);
!window->GetProperty(kExcludeInMruKey) &&
!window->GetProperty(kOverviewUiKey);
}

//////////////////////////////////////////////////////////////////////////////
Expand Down
5 changes: 2 additions & 3 deletions ash/wm/overview/overview_grid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include "ash/wm/splitview/split_view_divider.h"
#include "ash/wm/splitview/split_view_utils.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_state_delegate.h"
#include "ash/wm/window_util.h"
#include "ash/wm/workspace/backdrop_controller.h"
Expand Down Expand Up @@ -353,7 +354,7 @@ std::unique_ptr<views::Widget> CreateSaveDeskButtonContainerWidget(
params.init_properties_container.SetProperty(kHideInDeskMiniViewKey, true);
// This should not show up in the MRU list. Otherwise, it will be treated as
// unsupported crostini app.
params.init_properties_container.SetProperty(kExcludeInMruKey, true);
params.init_properties_container.SetProperty(kOverviewUiKey, true);

auto widget = std::make_unique<views::Widget>();
widget->set_focus_on_creation(false);
Expand All @@ -363,7 +364,6 @@ std::unique_ptr<views::Widget> CreateSaveDeskButtonContainerWidget(

aura::Window* window = widget->GetNativeWindow();
window->parent()->StackChildAtBottom(window);
window->SetId(kShellWindowId_SaveDeskButtonContainer);
return widget;
}

Expand Down Expand Up @@ -2024,7 +2024,6 @@ void OverviewGrid::UpdateNoWindowsWidget(bool no_items,

aura::Window* widget_window = no_windows_widget_->GetNativeWindow();
widget_window->parent()->StackChildAtBottom(widget_window);
widget_window->SetId(kShellWindowId_OverviewNoWindowsLabelWindow);

ScopedOverviewAnimationSettings settings(
animate && !is_continuous_enter ? OVERVIEW_ANIMATION_NO_RECENTS_FADE
Expand Down

0 comments on commit 5417507

Please sign in to comment.