Lazy init new desk button label.
Another part of item 3 in the bug's doc.
Bug: b:362835104
Change-Id: Ib24dcb4729dc5baf7a7755c90e48d395e1edd6e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5877518
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Eric Sum <esum@google.com>
Cr-Commit-Position: refs/heads/main@{#1358216}
diff --git a/ash/wm/desks/desk_bar_view_base.cc b/ash/wm/desks/desk_bar_view_base.cc
index 3cf1954..8192f78 100644
--- a/ash/wm/desks/desk_bar_view_base.cc
+++ b/ash/wm/desks/desk_bar_view_base.cc
@@ -228,15 +228,16 @@
void UpdateChildViewsVisibility() {
auto* default_desk_button = bar_view_->default_desk_button();
auto* new_desk_button = bar_view_->new_desk_button();
- auto* new_desk_button_label = bar_view_->new_desk_button_label();
auto* library_button = bar_view_->library_button();
auto* library_button_label = bar_view_->library_button_label();
const bool zero_state = bar_view_->IsZeroState();
default_desk_button->SetVisible(zero_state);
new_desk_button->SetVisible(true);
- new_desk_button_label->SetVisible(!zero_state &&
- new_desk_button->state() ==
- DeskIconButton::State::kActive);
+ bar_view_->UpdateNewDeskButtonLabelVisibility(
+ !zero_state &&
+ new_desk_button->state() == DeskIconButton::State::kActive,
+ // Already in an active layout. No need to trigger another one.
+ /*layout_if_changed=*/false);
if (library_button) {
library_button->SetVisible(bar_view_->ShouldShowLibraryUi());
}
@@ -343,9 +344,11 @@
gfx::Point(is_rtl ? x - new_desk_button_size.width() : x, y),
new_desk_button_size));
new_desk_button->SetBoundsRect(new_desk_button_bounds);
- LayoutDeskIconButtonLabel(bar_view_->new_desk_button_label(),
- new_desk_button_bounds, desk_name_view,
- IDS_ASH_DESKS_NEW_DESK_BUTTON_LABEL);
+ if (bar_view_->new_desk_button_label()) {
+ LayoutDeskIconButtonLabel(bar_view_->new_desk_button_label(),
+ new_desk_button_bounds, desk_name_view,
+ IDS_ASH_DESKS_NEW_DESK_BUTTON_LABEL);
+ }
x +=
(new_desk_button_size.width() + kDeskBarMiniViewsSpacing) * increment;
};
@@ -784,10 +787,6 @@
base::Unretained(this))));
new_desk_button_->SetProperty(views::kElementIdentifierKey,
kOverviewDeskBarNewDeskButtonElementId);
- new_desk_button_label_ =
- contents_view_->AddChildView(std::make_unique<views::Label>());
- new_desk_button_label_->SetPaintToLayer();
- new_desk_button_label_->layer()->SetFillsBoundsOpaquely(false);
contents_view_->SetLayoutManager(
std::make_unique<DeskBarScrollViewLayout>(this));
@@ -1121,9 +1120,12 @@
void DeskBarViewBase::UpdateDeskButtonsVisibility() {
const bool is_zero_state = IsZeroState();
default_desk_button_->SetVisible(is_zero_state);
- new_desk_button_label_->SetVisible(new_desk_button_->state() ==
- DeskIconButton::State::kActive);
+ UpdateNewDeskButtonLabelVisibility(
+ new_desk_button_->state() == DeskIconButton::State::kActive,
+ // If not attached to a widget yet, a layout get automatically run when
+ // `Widget::SetContentsView()` is called.
+ /*layout_if_changed=*/GetWidget());
UpdateLibraryButtonVisibility();
}
@@ -1173,6 +1175,26 @@
InvalidateLayout();
}
+void DeskBarViewBase::UpdateNewDeskButtonLabelVisibility(
+ bool new_visibility,
+ bool layout_if_changed) {
+ const bool current_visibility =
+ new_desk_button_label_ && new_desk_button_label_->GetVisible();
+ if (chromeos::features::AreOverviewSessionInitOptimizationsEnabled()) {
+ if (new_visibility) {
+ GetOrCreateNewDeskButtonLabel().SetVisible(true);
+ } else if (new_desk_button_label_) {
+ new_desk_button_label_->SetVisible(false);
+ }
+ } else {
+ GetOrCreateNewDeskButtonLabel().SetVisible(new_visibility);
+ }
+
+ if (new_visibility != current_visibility && layout_if_changed) {
+ InvalidateLayout();
+ }
+}
+
void DeskBarViewBase::UpdateDeskIconButtonState(
DeskIconButton* button,
DeskIconButton::State target_state) {
@@ -1689,6 +1711,17 @@
return *library_button_;
}
+views::Label& DeskBarViewBase::GetOrCreateNewDeskButtonLabel() {
+ if (new_desk_button_label_) {
+ return *new_desk_button_label_;
+ }
+ new_desk_button_label_ =
+ contents_view_->AddChildView(std::make_unique<views::Label>());
+ new_desk_button_label_->SetPaintToLayer();
+ new_desk_button_label_->layer()->SetFillsBoundsOpaquely(false);
+ return *new_desk_button_label_;
+}
+
void DeskBarViewBase::UpdateBarBounds() {}
int DeskBarViewBase::GetFirstMiniViewXOffset() const {
diff --git a/ash/wm/desks/desk_bar_view_base.h b/ash/wm/desks/desk_bar_view_base.h
index 69c0e31..b397fa83 100644
--- a/ash/wm/desks/desk_bar_view_base.h
+++ b/ash/wm/desks/desk_bar_view_base.h
@@ -184,6 +184,12 @@
// the saved desk feature is enabled and the user has any saved desks.
void UpdateLibraryButtonVisibility();
+ // Updates visibility of the label under the new desk button to
+ // `new_visibility`. If `layout_if_changed` is true and the label's visibility
+ // changes, the desk bar get asynchronously laid out after this call.
+ void UpdateNewDeskButtonLabelVisibility(bool new_visibility,
+ bool layout_if_changed);
+
// Called to update state of `button` and apply the scale animation to the
// button. For the new desk button, this is called when the make the new desk
// button a drop target for the window being dragged or at the end of the
@@ -283,10 +289,11 @@
// done with its animation or when `desk_activation_timer_` fires.
void OnUiUpdateDone();
- // The `library_button_` and its label get lazily constructed for performance
- // reasons. Creates the button if it doesn't exist. Only use if the button
- // should be visible.
+ // Accessors for UI elements that are lazily constructed for performance
+ // reasons. Creates them if they don't exist. Only use if they should be
+ // visible.
DeskIconButton& GetOrCreateLibraryButton();
+ views::Label& GetOrCreateNewDeskButtonLabel();
// Gets full available bounds for the desk bar widget.
virtual gfx::Rect GetAvailableBounds() const = 0;
diff --git a/ash/wm/desks/desk_mini_view_animations.cc b/ash/wm/desks/desk_mini_view_animations.cc
index 6852752e..7849982 100644
--- a/ash/wm/desks/desk_mini_view_animations.cc
+++ b/ash/wm/desks/desk_mini_view_animations.cc
@@ -116,7 +116,7 @@
void FadeInView(views::View* view,
base::TimeDelta duration,
base::TimeDelta delay) {
- if (!view->GetVisible()) {
+ if (!view || !view->GetVisible()) {
return;
}
diff --git a/ash/wm/desks/desks_test_util.cc b/ash/wm/desks/desks_test_util.cc
index c00dcb4..b5009138 100644
--- a/ash/wm/desks/desks_test_util.cc
+++ b/ash/wm/desks/desks_test_util.cc
@@ -25,6 +25,7 @@
#include "ui/events/base_event_utils.h"
#include "ui/events/gesture_detection/gesture_configuration.h"
#include "ui/events/test/event_generator.h"
+#include "ui/views/view.h"
namespace ash {
@@ -166,7 +167,7 @@
// visible, so we need to use the `close_all_button`
const DeskActionView* desk_action_view = mini_view->desk_action_view();
auto* combine_desks_button = desk_action_view->combine_desks_button();
- return (combine_desks_button && combine_desks_button->GetVisible())
+ return IsLazyInitViewVisible(combine_desks_button)
? combine_desks_button
: desk_action_view->close_all_button();
}
@@ -216,4 +217,8 @@
DesksTestApi::GetCloseAllWindowCloseTimeout().InMilliseconds());
}
+bool IsLazyInitViewVisible(const views::View* view) {
+ return view && view->GetVisible();
+}
+
} // namespace ash
diff --git a/ash/wm/desks/desks_test_util.h b/ash/wm/desks/desks_test_util.h
index 13adf2b..ac81f330 100644
--- a/ash/wm/desks/desks_test_util.h
+++ b/ash/wm/desks/desks_test_util.h
@@ -12,6 +12,10 @@
class EventGenerator;
} // namespace ui::test
+namespace views {
+class View;
+} // namespace views
+
namespace ash {
class CloseButton;
@@ -97,6 +101,10 @@
// 2) Wait for the windows to maybe be forcefully closed.
void SimulateWaitForCloseAll();
+// Whether a `view` is visible. `view` is lazy initialized, so can be null
+// (which is equivalent to invisible).
+bool IsLazyInitViewVisible(const views::View* view);
+
} // namespace ash
#endif // ASH_WM_DESKS_DESKS_TEST_UTIL_H_
diff --git a/ash/wm/desks/desks_unittests.cc b/ash/wm/desks/desks_unittests.cc
index dfa288b6..2ab27cb 100644
--- a/ash/wm/desks/desks_unittests.cc
+++ b/ash/wm/desks/desks_unittests.cc
@@ -2039,7 +2039,7 @@
/*drop=*/false);
WaitForMilliseconds(200);
EXPECT_EQ(DeskIconButton::State::kExpanded, new_desk_button->state());
- EXPECT_FALSE(desks_bar_view->new_desk_button_label()->GetVisible());
+ EXPECT_FALSE(IsLazyInitViewVisible(desks_bar_view->new_desk_button_label()));
// Now fire the timer directly to skip the wait time. Verify that the new desk
// button is scaled up and at the active state and the new desk label is shown
// now.
@@ -2049,7 +2049,7 @@
->FireNow();
RunScheduledLayoutForAllOverviewDeskBars();
EXPECT_EQ(DeskIconButton::State::kActive, new_desk_button->state());
- EXPECT_TRUE(desks_bar_view->new_desk_button_label()->GetVisible());
+ EXPECT_TRUE(IsLazyInitViewVisible(desks_bar_view->new_desk_button_label()));
// Keep dragging `overview_item1` to the center of the new desk button to make
// it a drop target.
@@ -2112,7 +2112,7 @@
/*by_touch_gestures=*/false, /*drop=*/false);
EXPECT_FALSE(desks_bar_view->IsZeroState());
EXPECT_EQ(DeskIconButton::State::kExpanded, new_desk_button->state());
- EXPECT_FALSE(desks_bar_view->new_desk_button_label()->GetVisible());
+ EXPECT_FALSE(IsLazyInitViewVisible(desks_bar_view->new_desk_button_label()));
// Keep dragging `overview_item1` to hover on the new desk button, immediately
// fire the time to skip to wait time. Verify that new desk button is
@@ -2130,7 +2130,7 @@
->FireNow();
RunScheduledLayoutForAllOverviewDeskBars();
EXPECT_EQ(DeskIconButton::State::kActive, new_desk_button->state());
- EXPECT_TRUE(desks_bar_view->new_desk_button_label()->GetVisible());
+ EXPECT_TRUE(IsLazyInitViewVisible(desks_bar_view->new_desk_button_label()));
// Now keep dragging `overview_item1` and make it not able to be dropped on
// the new desk, then drop it. Check that `overview_item1` is dropped back to
@@ -2146,7 +2146,7 @@
// The desk bar never goes back to the zero state from the expanded state even
// these's only one desk. Verify that the new desk label is invisible now.
EXPECT_FALSE(desks_bar_view->IsZeroState());
- EXPECT_FALSE(desks_bar_view->new_desk_button_label()->GetVisible());
+ EXPECT_FALSE(IsLazyInitViewVisible(desks_bar_view->new_desk_button_label()));
EXPECT_EQ(1u, controller->desks().size());
EXPECT_TRUE(
base::Contains(controller->GetDeskAtIndex(0)->windows(), win1.get()));
@@ -6156,7 +6156,7 @@
DeskBarViewBase::Type::kOverview);
break;
}
- return scroll_arrow && scroll_arrow->GetVisible();
+ return IsLazyInitViewVisible(scroll_arrow);
};
UpdateDisplay("600x400");
@@ -9546,7 +9546,8 @@
// appearance.
OpenDeskBar();
auto* desk_bar_view = GetDeskBarView();
- ASSERT_TRUE(desk_bar_view && desk_bar_view->GetVisible());
+ ASSERT_TRUE(desk_bar_view);
+ ASSERT_TRUE(desk_bar_view->GetVisible());
auto* desk_bar_widget = desk_bar_view->GetWidget();
ASSERT_TRUE(desk_bar_widget);
if (bar_type_ == DeskBarViewBase::Type::kOverview) {
@@ -9570,8 +9571,7 @@
EXPECT_THAT(new_desk_button->GetEnabled(),
desks_controller->CanCreateDesks());
auto* library_button = desk_bar_view->library_button();
- EXPECT_THAT(library_button && library_button->GetVisible(),
- test.has_saved_desks);
+ EXPECT_THAT(IsLazyInitViewVisible(library_button), test.has_saved_desks);
if (library_button) {
EXPECT_THAT(library_button->state(), expected_button_state);
EXPECT_TRUE(library_button->GetEnabled());
@@ -9598,7 +9598,8 @@
OpenDeskBar(root);
auto* desk_bar_view = GetDeskBarView(root);
- ASSERT_TRUE(desk_bar_view && desk_bar_view->GetVisible());
+ ASSERT_TRUE(desk_bar_view);
+ ASSERT_TRUE(desk_bar_view->GetVisible());
auto* desk_bar_widget = desk_bar_view->GetWidget();
ASSERT_TRUE(desk_bar_widget);
diff --git a/ash/wm/desks/templates/saved_desk_test_util.cc b/ash/wm/desks/templates/saved_desk_test_util.cc
index 375ff30..ce0ddb00 100644
--- a/ash/wm/desks/templates/saved_desk_test_util.cc
+++ b/ash/wm/desks/templates/saved_desk_test_util.cc
@@ -6,6 +6,7 @@
#include "ash/shell.h"
#include "ash/style/icon_button.h"
+#include "ash/wm/desks/desks_test_util.h"
#include "ash/wm/desks/overview_desk_bar_view.h"
#include "ash/wm/desks/templates/saved_desk_controller.h"
#include "ash/wm/desks/templates/saved_desk_dialog_controller.h"
@@ -270,7 +271,7 @@
bool WaitForLibraryButtonVisible() {
return base::test::RunUntil(
- []() { return GetLibraryButton() && GetLibraryButton()->GetVisible(); });
+ []() { return IsLazyInitViewVisible(GetLibraryButton()); });
}
const app_restore::AppRestoreData* QueryRestoreData(
diff --git a/ash/wm/desks/templates/saved_desk_unittest.cc b/ash/wm/desks/templates/saved_desk_unittest.cc
index 936601f0..ceee1230 100644
--- a/ash/wm/desks/templates/saved_desk_unittest.cc
+++ b/ash/wm/desks/templates/saved_desk_unittest.cc
@@ -530,7 +530,7 @@
DeskIconButton::State::kZero);
EXPECT_EQ(
expanded_state_shown,
- library_button && library_button->GetVisible() &&
+ IsLazyInitViewVisible(library_button) &&
library_button->state() == DeskIconButton::State::kExpanded);
EXPECT_EQ(active_state_shown, library_button &&
library_button->GetVisible() &&
@@ -1879,7 +1879,7 @@
ToggleOverview();
aura::Window* root = Shell::GetPrimaryRootWindow();
auto* library_button = GetLibraryButtonForRoot(root);
- EXPECT_FALSE(library_button && library_button->GetVisible());
+ EXPECT_FALSE(IsLazyInitViewVisible(library_button));
EXPECT_FALSE(GetSaveDeskButtonContainerForRoot(root));
}
@@ -2069,7 +2069,7 @@
ToggleOverview();
auto* button = GetLibraryButtonForRoot(Shell::GetPrimaryRootWindow());
- ASSERT_FALSE(button && button->GetVisible());
+ ASSERT_FALSE(IsLazyInitViewVisible(button));
// Test that we do not focus the templates button.
PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
