Skip to content

Commit

Permalink
Revert "saved_desks: Handle blocking modal dialogs when saving a desk…
Browse files Browse the repository at this point in the history 
… for later."

This reverts commit fbb5861.

Reason for revert: Browsertests are failing, e.g., https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-rel/58697/overview

Original change's description:
> saved_desks: Handle blocking modal dialogs when saving a desk for later.
>
> This CL makes a fairly substantial change to the way windows and desks
> are handled when the user saves a desk for later (Save & Recall).
>
> Previously, we would use some of the functionality from CloseAll to
> remove the desk and its windows after the desk definition had been
> saved. The logic there was synchronous and, while simple, did not handle
> blocking dialogs gracefully.
>
> With this change, we will now start a watcher that monitors the windows
> that are to be closed and also detects modal dialogs. The goal is to
> allow the user to deal with these dialogs that may appear. If the user
> elects to close the window(s), they will be taken to the saved desk
> library and the newly created saved desk is focused. If they decide to
> leave windows open, then the watcher terminates itself and the user is
> left on the desk.
>
> If there are no windows that show blocking dialogs, the flow should be
> the same as before this CL.
>
> Tested: Manually and with browser tests.
>
> Bug: 1334926
> Change-Id: I79707b63a0ab2874b2430821dd9b9158998a3b90
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3806033
> Reviewed-by: Sammie Quon <sammiequon@chromium.org>
> Commit-Queue: Daniel Andersson <dandersson@chromium.org>
> Reviewed-by: Richard Chui <richui@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1033619}

Bug: 1334926
Change-Id: I9df83e02745152950fd5cd2cc823ad59837ed40d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3823731
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Tommy Martino <tmartino@chromium.org>
Owners-Override: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1033708}
  • Loading branch information
Tommy Martino authored and Chromium LUCI CQ committed Aug 10, 2022
1 parent f4c25aa commit 914a4c5
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 407 deletions.
9 changes: 4 additions & 5 deletions ash/wm/desks/desks_controller.cc
View file
Expand Up @@ -552,11 +552,10 @@ void DesksController::RemoveDesk(const Desk* desk,

auto* overview_controller = Shell::Get()->overview_controller();
const bool in_overview = overview_controller->InOverviewSession();
if (!in_overview && active_desk_ == desk &&
source != DesksCreationRemovalSource::kSaveAndRecall) {
// When removing the active desk outside of overview (and the source is not
// Save & Recall), we trigger the remove desk animation. We will activate
// the desk to its left if any, otherwise, we activate one on the right.
if (!in_overview && active_desk_ == desk) {
// When removing the active desk outside of overview, we trigger the remove
// desk animation. We will activate the desk to its left if any, otherwise,
// we activate one on the right.
const int current_desk_index = GetDeskIndex(active_desk_);
const int target_desk_index =
current_desk_index + ((current_desk_index > 0) ? -1 : 1);
Expand Down
269 changes: 18 additions & 251 deletions ash/wm/desks/templates/saved_desk_presenter.cc
View file
Expand Up @@ -22,20 +22,15 @@
#include "ash/wm/desks/templates/saved_desk_metrics_util.h"
#include "ash/wm/desks/templates/saved_desk_name_view.h"
#include "ash/wm/desks/zero_state_button.h"
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_grid.h"
#include "ash/wm/overview/overview_session.h"
#include "base/bind.h"
#include "base/containers/cxx20_erase_vector.h"
#include "base/i18n/number_formatting.h"
#include "base/scoped_multi_source_observation.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h"
#include "components/desks_storage/core/desk_template_util.h"
#include "third_party/re2/src/re2/re2.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/wm/core/window_util.h"

namespace ash {

Expand All @@ -61,210 +56,6 @@ desks_storage::DeskModel* GetDeskModel() {
return desk_model;
}

// The WindowCloseObserver helper is used in the Save & Recall save flow. When
// the user saves a desk, we will try to close all windows on that desk, and
// then finally remove the desk. This is an asynchronous task that may trigger
// confirmation dialogs to pop up.
//
// The flow is as follows:
// 1. The user initiates Save & Recall by clicking the save button.
// 2. Windows are enumerated and saved to a saved desk definition.
// 3. We start a `WindowCloseObserver` and call `Close()` on all windows.
// The observer then deals with three different cases:
// a. All windows close automatically.
// b. A confirmation dialog appears and the user decides to close the window.
// c. A confirmation dialog appears and the user decides to keep the window.
//
// For cases a & b, we remove the desk and transition the user into the saved
// desk library. For case c, we leave the desk alone. Also note that for cases b
// & c, the confirmation dialog will take the user out of overview mode.

class WindowCloseObserver;

// This is a global raw pointer since the lifetime of the watcher cannot be tied
// to the presenter. The presenter's lifetime is indirectly tied to the overview
// session, and the watcher must survive going out of overview mode.
WindowCloseObserver* g_window_close_observer = nullptr;

class WindowCloseObserver : public aura::WindowObserver {
public:
WindowCloseObserver(aura::Window* root_window,
const base::GUID& saved_desk_guid,
const std::u16string& saved_desk_name,
const std::vector<aura::Window*>& windows)
: root_window_(root_window),
saved_desk_guid_(saved_desk_guid),
saved_desk_name_(saved_desk_name) {
DCHECK(g_window_close_observer == nullptr);

auto* desks_controller = DesksController::Get();
desk_to_remove_ = desks_controller->active_desk();

// Observe the windows that we are going to close.
for (aura::Window* window : windows)
window_observer_.AddObservation(window);

OverviewController* overview_controller =
Shell::Get()->overview_controller();
if (OverviewSession* overview_session =
overview_controller->overview_session()) {
overview_session->set_allow_empty_desk_without_exiting(true);
}

system_modal_container_ = Shell::Get()->GetContainer(
root_window, kShellWindowId_SystemModalContainer);
window_observer_.AddObservation(system_modal_container_);
}

~WindowCloseObserver() override {
OverviewController* overview_controller =
Shell::Get()->overview_controller();
if (OverviewSession* overview_session =
overview_controller->overview_session()) {
overview_session->set_allow_empty_desk_without_exiting(false);
}
g_window_close_observer = nullptr;
}

void SetModalDialogCallbackForTesting(base::OnceClosure closure) {
modal_dialog_closure_for_testing_ = std::move(closure);
}

void FireWindowWatcherTimerForTesting() {
DCHECK(auto_transition_timer_.IsRunning());
auto_transition_timer_.FireNow();
}

private:
// aura::WindowObserver:
void OnWindowAdded(aura::Window* new_window) override {
if (new_window->parent() != system_modal_container_)
return;

if (modal_dialog_closure_for_testing_)
std::move(modal_dialog_closure_for_testing_).Run();

modal_dialog_showed_ = true;
}

void OnWindowRemoved(aura::Window* removed_window) override {
if (!modal_dialog_showed_ || !system_modal_container_->children().empty())
return;

// The last modal dialog has been dismissed. At this point we don't know if
// the user has dismissed the windows or decided to keep them. We will now
// allow a short time for windows to close. If, after this time, windows
// have not been closed, we'll proceed to the library but leave the desk.
//
// If all windows *do* close before the timer hits, then this will be picked
// up by `OnWindowDestroyed` and we will show the saved desk library.
if (!auto_transition_timer_.IsRunning()) {
auto_transition_timer_.Start(
FROM_HERE, base::Seconds(1), this,
&WindowCloseObserver::ShowLibraryWithoutRemovingDesk);
}
}

void OnWindowDestroyed(aura::Window* window) override {
window_observer_.RemoveObservation(window);

// In the unexpected case that the system modal container is destroyed, we
// will bail.
if (window == system_modal_container_) {
Terminate();
return;
}

// The observer is used for the windows on the desk that we are closing, as
// well as the system modal container. When there's only one window left
// (the system modal container) in the observing set, we're done.
if (window_observer_.GetSourcesCount() == 1) {
// We're ready to transition into the saved desk library and highlight the
// item. After this has been done, we'll remove ourselves.
ShowLibrary(/*remove_desk=*/true);
Terminate();
}
}

void ShowLibraryWithoutRemovingDesk() {
ShowLibrary(/*remove_desk=*/false);
Terminate();
}

void ShowLibrary(bool remove_desk) {
OverviewController* overview_controller =
Shell::Get()->overview_controller();
OverviewSession* overview_session = overview_controller->overview_session();
if (!overview_session) {
if (!overview_controller->StartOverview(
OverviewStartAction::kOverviewButton,
OverviewEnterExitType::kImmediateEnterWithoutFocus)) {
// If for whatever reason we didn't enter overview mode, bail.
return;
}

overview_session = overview_controller->overview_session();
DCHECK(overview_session);
}

// Show the library, this should highlight the newly saved item.
OverviewGrid* overview_grid =
overview_session->GetGridWithRootWindow(root_window_);
overview_session->ShowDesksTemplatesGrids(
overview_grid->desks_bar_view()->IsZeroState(), saved_desk_guid_,
saved_desk_name_, root_window_);

// Remove the current desk, this will be done without animation.
if (remove_desk) {
auto* desks_controller = DesksController::Get();
if (DeskExists(desks_controller, desk_to_remove_)) {
if (!desks_controller->CanRemoveDesks())
desks_controller->NewDesk(DesksCreationRemovalSource::kSaveAndRecall);

desks_controller->RemoveDesk(desk_to_remove_,
DesksCreationRemovalSource::kSaveAndRecall,
DeskCloseType::kCloseAllWindows);
}
}
}

bool DeskExists(DesksController* desks_controller, const Desk* desk) {
return base::Contains(
desks_controller->desks(), desk,
[](const std::unique_ptr<Desk>& desk) { return desk.get(); });
}

void Terminate() { delete this; }

aura::Window* root_window_;

aura::Window* system_modal_container_ = nullptr;

// Tracks whether a modal "confirm close" dialog has been showed.
bool modal_dialog_showed_ = false;

// Used to automatically transition the user to the library after a modal
// dialog has been dismissed.
base::OneShotTimer auto_transition_timer_{
base::DefaultTickClock::GetInstance()};

// The desk that the user has saved and that we will remove once windows have
// been removed.
const Desk* desk_to_remove_ = nullptr;

// GUID and name of the saved desk.
const base::GUID saved_desk_guid_;
const std::u16string saved_desk_name_;

// Used by unit tests to wait for modal dialogs.
base::OnceClosure modal_dialog_closure_for_testing_;

// Used to watch windows on the desk to remove, as well as the system modal
// container.
base::ScopedMultiSourceObservation<aura::Window, aura::WindowObserver>
window_observer_{this};
};

} // namespace

SavedDeskPresenter::SavedDeskPresenter(OverviewSession* overview_session)
Expand Down Expand Up @@ -644,7 +435,7 @@ void SavedDeskPresenter::OnAddOrUpdateEntry(

if (on_update_ui_closure_for_testing_)
std::move(on_update_ui_closure_for_testing_).Run();
} else if (desk_template->type() != DeskTemplateType::kSaveAndRecall) {
} else {
// This will update the templates button and save as desks button too. This
// will call `GetAllEntries`.
overview_session_->ShowDesksTemplatesGrids(
Expand All @@ -653,37 +444,27 @@ void SavedDeskPresenter::OnAddOrUpdateEntry(

if (!was_update) {
const auto saved_desk_type = desk_template->type();
if (saved_desk_type == DeskTemplateType::kSaveAndRecall) {
// We have successfully created a *new* desk template for Save & Recall,
// so we are now going to close all the windows on the active desk and
// also remove the desk.
auto* desks_controller = DesksController::Get();
auto* active_desk = desks_controller->active_desk();

// If this is the only desk, we have to create a new desk before we can
// remove the current one.
if (!desks_controller->CanRemoveDesks())
desks_controller->NewDesk(DesksCreationRemovalSource::kSaveAndRecall);

desks_controller->RemoveDesk(active_desk,
DesksCreationRemovalSource::kSaveAndRecall,
DeskCloseType::kCloseAllWindows);
}

RecordNewSavedDeskHistogram(saved_desk_type);
RecordUserSavedDeskCountHistogram(saved_desk_type,
GetEntryCount(saved_desk_type),
GetMaxEntryCount(saved_desk_type));

if (saved_desk_type == DeskTemplateType::kSaveAndRecall) {
std::vector<aura::Window*> windows =
Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk);

// Get rid of transient windows.
base::EraseIf(windows, [](aura::Window* window) {
return wm::GetTransientParent(window) != nullptr;
});

// Start observing current windows.
if (g_window_close_observer)
delete g_window_close_observer;
g_window_close_observer = new WindowCloseObserver(
root_window, desk_template->uuid(), saved_desk_name, windows);

// Go through windows and attempt to close them.
for (aura::Window* window : windows) {
if (views::Widget* widget =
views::Widget::GetWidgetForNativeView(window)) {
widget->Close();
}
}

if (on_update_ui_closure_for_testing_)
std::move(on_update_ui_closure_for_testing_).Run();
}
}

// Note we do not run `on_update_ui_closure_for_testing` here as we want to
Expand Down Expand Up @@ -745,18 +526,4 @@ std::u16string SavedDeskPresenter::AppendDuplicateNumberToDuplicateName(
return base::UTF8ToUTF16(duplicate_name);
}

// static
void SavedDeskPresenter::SetModalDialogCallbackForTesting(
base::OnceClosure closure) {
DCHECK(g_window_close_observer);
g_window_close_observer->SetModalDialogCallbackForTesting( // IN-TEST
std::move(closure));
}

// static
void SavedDeskPresenter::FireWindowWatcherTimerForTesting() {
DCHECK(g_window_close_observer);
g_window_close_observer->FireWindowWatcherTimerForTesting(); // IN-TEST
}

} // namespace ash
5 changes: 0 additions & 5 deletions ash/wm/desks/templates/saved_desk_presenter.h
View file
Expand Up @@ -146,11 +146,6 @@ class ASH_EXPORT SavedDeskPresenter : desks_storage::DeskModelObserver {
std::u16string AppendDuplicateNumberToDuplicateName(
const std::u16string& duplicate_name_u16);

// Sets `closure` to be invoked when Save & Recall triggers a modal dialog.
static void SetModalDialogCallbackForTesting(base::OnceClosure closure);
// Immediately fires the window watcher auto transition timer.
static void FireWindowWatcherTimerForTesting();

// Pointer to the session which owns `this`.
OverviewSession* const overview_session_;

Expand Down
17 changes: 0 additions & 17 deletions ash/wm/desks/templates/saved_desk_test_util.cc
View file
Expand Up @@ -79,18 +79,6 @@ SavedDeskPresenterTestApi::SavedDeskPresenterTestApi(

SavedDeskPresenterTestApi::~SavedDeskPresenterTestApi() = default;

// static
void SavedDeskPresenterTestApi::WaitForSaveAndRecallBlockingDialog() {
base::RunLoop loop;
SavedDeskPresenter::SetModalDialogCallbackForTesting(loop.QuitClosure());
loop.Run();
}

// static
void SavedDeskPresenterTestApi::FireWindowWatcherTimer() {
SavedDeskPresenter::FireWindowWatcherTimerForTesting();
}

void SavedDeskPresenterTestApi::SetOnUpdateUiClosure(
base::OnceClosure closure) {
DCHECK(!presenter_->on_update_ui_closure_for_testing_);
Expand Down Expand Up @@ -211,11 +199,6 @@ views::Button* GetSaveDeskAsTemplateButton() {
return overview_grid->GetSaveDeskAsTemplateButton();
}

views::Button* GetSaveDeskForLaterButton() {
const auto* overview_grid = GetPrimaryOverviewGrid();
return overview_grid ? overview_grid->GetSaveDeskForLaterButton() : nullptr;
}

views::Button* GetTemplateItemButton(int index) {
auto* item = GetItemViewFromTemplatesGrid(index);
return item ? static_cast<views::Button*>(item) : nullptr;
Expand Down
9 changes: 0 additions & 9 deletions ash/wm/desks/templates/saved_desk_test_util.h
View file
Expand Up @@ -43,14 +43,6 @@ class SavedDeskPresenterTestApi {
delete;
~SavedDeskPresenterTestApi();

// When Save & Recall closes windows, they might object and present a blocking
// dialog. This function waits until that dialog is detected.
static void WaitForSaveAndRecallBlockingDialog();

// Fire window watcher's timer to automatically transition into the saved desk
// library. This will DCHECK if the timer is not active.
static void FireWindowWatcherTimer();

void SetOnUpdateUiClosure(base::OnceClosure closure);

// If there are outstanding operations on the desk model, this blocks until
Expand Down Expand Up @@ -171,7 +163,6 @@ SavedDeskItemView* GetItemViewFromTemplatesGrid(size_t grid_item_index);
views::Button* GetZeroStateDesksTemplatesButton();
views::Button* GetExpandedStateDesksTemplatesButton();
views::Button* GetSaveDeskAsTemplateButton();
views::Button* GetSaveDeskForLaterButton();
views::Button* GetTemplateItemButton(int index);
views::Button* GetTemplateItemDeleteButton(int index);
views::Button* GetSavedDeskDialogAcceptButton();
Expand Down

0 comments on commit 914a4c5

Please sign in to comment.