Skip to content

Commit

Permalink
CloseAll: Remove legacy code paths
Browse files Browse the repository at this point in the history
This CL removes all code mentioning the DeskMiniView's close_all_button_
or the DesksCloseAll flag.

Bug: b/263166880
Change-Id: Ie29293c1f1003074ffe3f5485970730f6a978b18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205141
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Daniel Andersson <dandersson@chromium.org>
Commit-Queue: Ben Becker <benbecker@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099478}
  • Loading branch information
Ben Becker authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent df291c6 commit a620ddf
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 188 deletions.
6 changes: 0 additions & 6 deletions ash/constants/ash_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2407,12 +2407,6 @@ bool IsCryptauthAttestationSyncingEnabled() {
return base::FeatureList::IsEnabled(kCryptauthAttestationSyncing);
}

bool IsDesksCloseAllEnabled() {
// TODO(b/263166880): Remove this function and all code paths where this is
// false.
return true;
}

bool IsDnsOverHttpsWithIdentifiersReuseOldPolicyEnabled() {
return base::FeatureList::IsEnabled(
kDnsOverHttpsWithIdentifiersReuseOldPolicy);
Expand Down
1 change: 0 additions & 1 deletion ash/constants/ash_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsCryptohomeRecoverySetupEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsDarkLightModeEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsDarkLightModeKMeansColorEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsDeepLinkingEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsDesksCloseAllEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsDeskTemplateSyncEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsInputDeviceSettingsSplitEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS)
Expand Down
127 changes: 42 additions & 85 deletions ash/wm/desks/desk_mini_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,40 +140,28 @@ DeskMiniView::DeskMiniView(DesksBarView* owner_bar,

desk_name_view_ = AddChildView(std::move(desk_name_view));

if (features::IsDesksCloseAllEnabled()) {
const std::u16string initial_combine_desks_target_name =
desks_controller->GetCombineDesksTargetName(desk_);

desk_action_view_ = AddChildView(std::make_unique<DeskActionView>(
initial_combine_desks_target_name,
/*combine_desks_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk,
base::Unretained(this),
DeskCloseType::kCombineDesks),
/*close_all_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk,
base::Unretained(this),
DeskCloseType::kCloseAllWindowsAndWait)));

context_menu_ = std::make_unique<DeskActionContextMenu>(
initial_combine_desks_target_name,
/*combine_desks_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk,
base::Unretained(this),
DeskCloseType::kCombineDesks),
/*close_all_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk,
base::Unretained(this),
DeskCloseType::kCloseAllWindowsAndWait),
base::BindRepeating(&DeskMiniView::OnContextMenuClosed,
base::Unretained(this)));
} else {
close_desk_button_ = AddChildView(std::make_unique<CloseButton>(
base::BindRepeating(&DeskMiniView::OnRemovingDesk,
base::Unretained(this),
DeskCloseType::kCombineDesks),
CloseButton::Type::kSmall));
}
const std::u16string initial_combine_desks_target_name =
desks_controller->GetCombineDesksTargetName(desk_);

desk_action_view_ = AddChildView(std::make_unique<DeskActionView>(
initial_combine_desks_target_name,
/*combine_desks_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk, base::Unretained(this),
DeskCloseType::kCombineDesks),
/*close_all_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk, base::Unretained(this),
DeskCloseType::kCloseAllWindowsAndWait)));

context_menu_ = std::make_unique<DeskActionContextMenu>(
initial_combine_desks_target_name,
/*combine_desks_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk, base::Unretained(this),
DeskCloseType::kCombineDesks),
/*close_all_callback=*/
base::BindRepeating(&DeskMiniView::OnRemovingDesk, base::Unretained(this),
DeskCloseType::kCloseAllWindowsAndWait),
base::BindRepeating(&DeskMiniView::OnContextMenuClosed,
base::Unretained(this)));
UpdateDeskButtonVisibility();
}

Expand Down Expand Up @@ -212,37 +200,26 @@ void DeskMiniView::UpdateDeskButtonVisibility() {
(IsMouseHovered() || force_show_desk_buttons_ ||
Shell::Get()->accessibility_controller()->IsSwitchAccessRunning());

if (features::IsDesksCloseAllEnabled()) {
// Only show the combine desks button if there are app windows in the desk,
// or if the desk is active and there are windows that should be visible on
// all desks.
desk_action_view_->SetCombineDesksButtonVisibility(
ContainsAppWindows(desk_));
desk_action_view_->SetVisible(visible && !is_context_menu_open_);
} else {
close_desk_button_->SetVisible(visible);
}
// Only show the combine desks button if there are app windows in the desk,
// or if the desk is active and there are windows that should be visible on
// all desks.
desk_action_view_->SetCombineDesksButtonVisibility(ContainsAppWindows(desk_));
desk_action_view_->SetVisible(visible && !is_context_menu_open_);
}

void DeskMiniView::OnWidgetGestureTap(const gfx::Rect& screen_rect,
bool is_long_gesture) {
views::View* view_to_update =
features::IsDesksCloseAllEnabled()
? static_cast<views::View*>(desk_action_view_)
: static_cast<views::View*>(close_desk_button_);

// Note that we don't want to hide the desk buttons if it's a single tap
// within the bounds of an already visible button, which will later be
// handled as a press event on that desk buttons that will result in closing
// the desk.
const bool old_force_show_desk_buttons = force_show_desk_buttons_;
force_show_desk_buttons_ =
!(features::IsDesksCloseAllEnabled() &&
Shell::Get()->tablet_mode_controller()->InTabletMode()) &&
!Shell::Get()->tablet_mode_controller()->InTabletMode() &&
((is_long_gesture && IsPointOnMiniView(screen_rect.CenterPoint())) ||
(!is_long_gesture && view_to_update->GetVisible() &&
view_to_update->HitTestRect(
ConvertScreenRect(view_to_update, screen_rect))));
(!is_long_gesture && desk_action_view_->GetVisible() &&
desk_action_view_->HitTestRect(
ConvertScreenRect(desk_action_view_, screen_rect))));

if (old_force_show_desk_buttons != force_show_desk_buttons_)
UpdateDeskButtonVisibility();
Expand Down Expand Up @@ -326,10 +303,7 @@ void DeskMiniView::OnRemovingDesk(DeskCloseType close_type) {
// longer processing events.
SetCanProcessEventsWithinSubtree(false);

if (features::IsDesksCloseAllEnabled())
desk_action_view_->SetVisible(false);
else
close_desk_button_->SetVisible(false);
desk_action_view_->SetVisible(false);

controller->RemoveDesk(desk_, DesksCreationRemovalSource::kButton,
close_type);
Expand All @@ -344,22 +318,12 @@ void DeskMiniView::Layout() {
desk_preview_->SetBoundsRect(preview_bounds);

LayoutDeskNameView(preview_bounds);
if (features::IsDesksCloseAllEnabled()) {
const gfx::Size desk_action_view_size =
desk_action_view_->GetPreferredSize();
desk_action_view_->SetBounds(
preview_bounds.right() - desk_action_view_size.width() -
kCloseButtonMargin,
kCloseButtonMargin, desk_action_view_size.width(),
desk_action_view_size.height());
} else {
DCHECK(close_desk_button_);
const int close_button_size =
close_desk_button_->GetPreferredSize().width();
close_desk_button_->SetBounds(
preview_bounds.right() - close_button_size - kCloseButtonMargin,
kCloseButtonMargin, close_button_size, close_button_size);
}
const gfx::Size desk_action_view_size = desk_action_view_->GetPreferredSize();
desk_action_view_->SetBounds(
preview_bounds.right() - desk_action_view_size.width() -
kCloseButtonMargin,
kCloseButtonMargin, desk_action_view_size.width(),
desk_action_view_size.height());
}

gfx::Size DeskMiniView::CalculatePreferredSize() const {
Expand Down Expand Up @@ -398,17 +362,10 @@ void DeskMiniView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
if (!DesksController::Get()->CanRemoveDesks())
return;

std::string extra_tip;
if (features::IsDesksCloseAllEnabled()) {
const std::u16string target_desk_name =
DesksController::Get()->GetCombineDesksTargetName(desk_);
extra_tip = l10n_util::GetStringFUTF8(
IDS_ASH_OVERVIEW_CLOSABLE_DESK_MINIVIEW_A11Y_EXTRA_TIP,
target_desk_name);
} else {
extra_tip = l10n_util::GetStringUTF8(
IDS_ASH_OVERVIEW_CLOSABLE_HIGHLIGHT_ITEM_A11Y_EXTRA_TIP);
}
const std::u16string target_desk_name =
DesksController::Get()->GetCombineDesksTargetName(desk_);
const std::string extra_tip = l10n_util::GetStringFUTF8(
IDS_ASH_OVERVIEW_CLOSABLE_DESK_MINIVIEW_A11Y_EXTRA_TIP, target_desk_name);

node_data->AddStringAttribute(ax::mojom::StringAttribute::kDescription,
extra_tip);
Expand Down
15 changes: 2 additions & 13 deletions ash/wm/desks/desk_mini_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

namespace ash {

class CloseButton;
class DeskActionContextMenu;
class DeskActionView;
class DeskNameView;
Expand Down Expand Up @@ -55,8 +54,6 @@ class ASH_EXPORT DeskMiniView : public views::View,

DeskNameView* desk_name_view() { return desk_name_view_; }

const CloseButton* close_desk_button() const { return close_desk_button_; }

const DeskActionView* desk_action_view() const { return desk_action_view_; }
DeskActionView* desk_action_view() { return desk_action_view_; }

Expand Down Expand Up @@ -112,11 +109,7 @@ class ASH_EXPORT DeskMiniView : public views::View,
// Closes context menu on this mini view if one exists.
void MaybeCloseContextMenu();

// Sets either the `desk_action_view_` or the `close_desk_button_` visibility
// to false depending on whether the `kDesksCloseAll` feature is active, and
// then removes the desk. If `close_type` is `kCloseAllWindows*`, this
// function tells the `DesksController` to remove `desk_`'s windows as well,
// and wait for the user to confirm.
// Invoked when the user has clicked a desk close button.
void OnRemovingDesk(DeskCloseType close_type);

// views::View:
Expand Down Expand Up @@ -170,11 +163,7 @@ class ASH_EXPORT DeskMiniView : public views::View,
// The editable desk name.
DeskNameView* desk_name_view_ = nullptr;

// The close button that shows on hover.
CloseButton* close_desk_button_ = nullptr;

// When the Close-All flag is enabled, we store the hover interface for desk
// actions here.
// Stores the hover interface for desk actions.
DeskActionView* desk_action_view_ = nullptr;

// The context menu that appears when `desk_preview_` is right-clicked or
Expand Down
34 changes: 14 additions & 20 deletions ash/wm/desks/desk_preview_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,15 +349,13 @@ DeskPreviewView::DeskPreviewView(PressedCallback callback,
contents_view_layer->SetIsFastRoundedCorner(true);
AddChildView(desk_mirrored_contents_view_);

if (features::IsDesksCloseAllEnabled()) {
highlight_overlay_ = AddChildView(std::make_unique<views::View>());
highlight_overlay_->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
highlight_overlay_->SetVisible(false);
ui::Layer* highlight_overlay_layer = highlight_overlay_->layer();
highlight_overlay_layer->SetName("DeskPreviewView highlight overlay");
highlight_overlay_layer->SetRoundedCornerRadius(GetRoundedCorner());
highlight_overlay_layer->SetIsFastRoundedCorner(true);
}
highlight_overlay_ = AddChildView(std::make_unique<views::View>());
highlight_overlay_->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
highlight_overlay_->SetVisible(false);
ui::Layer* highlight_overlay_layer = highlight_overlay_->layer();
highlight_overlay_layer->SetName("DeskPreviewView highlight overlay");
highlight_overlay_layer->SetRoundedCornerRadius(GetRoundedCorner());
highlight_overlay_layer->SetIsFastRoundedCorner(true);

RecreateDeskContentsMirrorLayers();
}
Expand Down Expand Up @@ -459,8 +457,7 @@ void DeskPreviewView::Layout() {
wallpaper_preview_->SetBoundsRect(bounds);
desk_mirrored_contents_view_->SetBoundsRect(bounds);

if (features::IsDesksCloseAllEnabled())
highlight_overlay_->SetBoundsRect(bounds);
highlight_overlay_->SetBoundsRect(bounds);

// The desk's contents mirrored layer needs to be scaled down so that it fits
// exactly in the center of the view.
Expand All @@ -480,9 +477,8 @@ void DeskPreviewView::Layout() {
}

bool DeskPreviewView::OnMousePressed(const ui::MouseEvent& event) {
// If we have a right click and the `kDesksCloseAll` feature is enabled, we
// should open the context menu.
if (features::IsDesksCloseAllEnabled() && event.IsRightMouseButton()) {
// If we have a right click we should open the context menu.
if (event.IsRightMouseButton()) {
DeskNameView::CommitChanges(GetWidget());
mini_view_->OpenContextMenu(ui::MENU_SOURCE_MOUSE);
} else {
Expand Down Expand Up @@ -533,12 +529,10 @@ void DeskPreviewView::OnGestureEvent(ui::GestureEvent* event) {
void DeskPreviewView::OnThemeChanged() {
views::Button::OnThemeChanged();

if (features::IsDesksCloseAllEnabled()) {
highlight_overlay_->layer()->SetColor(
SkColorSetA(AshColorProvider::Get()->GetControlsLayerColor(
AshColorProvider::ControlsLayerType::kHighlightColor1),
kHighlightTransparency));
}
highlight_overlay_->layer()->SetColor(
SkColorSetA(AshColorProvider::Get()->GetControlsLayerColor(
AshColorProvider::ControlsLayerType::kHighlightColor1),
kHighlightTransparency));
}

void DeskPreviewView::OnFocus() {
Expand Down
9 changes: 2 additions & 7 deletions ash/wm/desks/desks_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -763,8 +763,7 @@ void DesksBarView::HandleLongPressEvent(DeskMiniView* mini_view,
InitDragDesk(mini_view, location);
StartDragDesk(mini_view, location, event.IsMouseEvent());

if (features::IsDesksCloseAllEnabled())
mini_view->OpenContextMenu(ui::MENU_SOURCE_LONG_PRESS);
mini_view->OpenContextMenu(ui::MENU_SOURCE_LONG_PRESS);
}

void DesksBarView::HandleDragEvent(DeskMiniView* mini_view,
Expand All @@ -774,8 +773,7 @@ void DesksBarView::HandleDragEvent(DeskMiniView* mini_view,
if (!drag_proxy_ || mini_view->is_animating_to_remove())
return;

if (features::IsDesksCloseAllEnabled())
mini_view->MaybeCloseContextMenu();
mini_view->MaybeCloseContextMenu();

gfx::PointF location = event.target()->GetScreenLocationF(event);

Expand Down Expand Up @@ -1551,9 +1549,6 @@ void DesksBarView::OnLibraryButtonPressed() {
}

void DesksBarView::MaybeUpdateCombineDesksTooltips() {
if (!features::IsDesksCloseAllEnabled())
return;

for (auto* mini_view : mini_views_) {
// If desk is being removed, do not update the tooltip.
if (mini_view->desk()->is_desk_being_removed()) {
Expand Down
5 changes: 2 additions & 3 deletions ash/wm/desks/desks_bar_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,8 @@ class ASH_EXPORT DesksBarView : public views::View,

void OnLibraryButtonPressed();

// If the `DesksCloseAll` flag is enabled, this function cycles through
// `mini_views_` and updates the tooltip for each mini view's combine desks
// button.
// This function cycles through `mini_views_` and updates the tooltip for each
// mini view's combine desks button.
void MaybeUpdateCombineDesksTooltips();

// Scrollview callbacks.
Expand Down
21 changes: 7 additions & 14 deletions ash/wm/desks/desks_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,23 +160,16 @@ const DesksBarView* GetPrimaryRootDesksBarView() {

const CloseButton* GetCloseDeskButtonForMiniView(
const DeskMiniView* mini_view) {
if (features::IsDesksCloseAllEnabled()) {
// When there are no windows on the desk, the `combine_desks_button` is not
// visible, so we need to use the `close_all_button`
const DeskActionView* desk_action_view = mini_view->desk_action_view();
return desk_action_view->combine_desks_button()->GetVisible()
? desk_action_view->combine_desks_button()
: desk_action_view->close_all_button();
}

return mini_view->close_desk_button();
// When there are no windows on the desk, the `combine_desks_button` is not
// visible, so we need to use the `close_all_button`
const DeskActionView* desk_action_view = mini_view->desk_action_view();
return desk_action_view->combine_desks_button()->GetVisible()
? desk_action_view->combine_desks_button()
: desk_action_view->close_all_button();
}

bool GetDeskActionVisibilityForMiniView(const DeskMiniView* mini_view) {
if (features::IsDesksCloseAllEnabled())
return mini_view->desk_action_view()->GetVisible();

return mini_view->close_desk_button()->GetVisible();
return mini_view->desk_action_view()->GetVisible();
}

void WaitForMilliseconds(int milliseconds) {
Expand Down
10 changes: 3 additions & 7 deletions ash/wm/desks/desks_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,11 @@ void WaitUntilEndingScreenshotTaken(DeskActivationAnimation* animation);
// Returns the desk bar view for the primary display.
const DesksBarView* GetPrimaryRootDesksBarView();

// Returns the legacy close button if `features::kDesksCloseAll` is not enabled,
// and otherwise returns the available button in the `desk_action_view` that
// performs the same action (i.e. the combine desks button if it is available,
// and otherwise the close-all button).
// Returns the combine desks button if it is available, and otherwise the
// close-all button.
const CloseButton* GetCloseDeskButtonForMiniView(const DeskMiniView* mini_view);

// Returns the visibility state of the desk action interface for the mini view
// (i.e. `desk_action_view` if `features::kDesksCloseAll` is enabled,
// `close_desk_button` otherwise).
// Returns the visibility state of the desk action interface for the mini view.
bool GetDeskActionVisibilityForMiniView(const DeskMiniView* mini_view);

// Wait for `milliseconds` to be finished.
Expand Down

0 comments on commit a620ddf

Please sign in to comment.