Commit c816f4a8 authored by Sammie Quon's avatar Sammie Quon Committed by Chromium LUCI CQ

desks: Close context menu when activating desks.

This requires adding support to context menus to close without animation,
otherwise it will still be visible as the default close has a fade out
animation, and the screenshot will capture a near-opaque version of the
menu.

Accelerators by default will also close menus with fade out, so add the
desk activation acclerator to the exception list and let the desk
animation code handle the closing.

Fixed: 991939
Test: manual
Change-Id: I273042a216c651022215929594a2cedd1efecc82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586013
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836362}
parent 6684f984
......@@ -379,6 +379,12 @@ const AcceleratorAction kActionsKeepingMenuOpen[] = {
BRIGHTNESS_UP,
DEBUG_TOGGLE_TOUCH_PAD,
DEBUG_TOGGLE_TOUCH_SCREEN,
// Keep the menu open when switching desks. The desk activation code will
// close the menu without animation manually. Otherwise, the menu will fade
// out and a trace will be visible while switching desks.
DESKS_ACTIVATE_DESK,
DESKS_NEW_DESK,
DESKS_REMOVE_CURRENT_DESK,
DISABLE_CAPS_LOCK,
KEYBOARD_BRIGHTNESS_DOWN,
KEYBOARD_BRIGHTNESS_UP,
......
......@@ -14,6 +14,7 @@
#include "ui/views/controls/menu/menu_item_view.h"
#include "ui/views/controls/menu/menu_model_adapter.h"
#include "ui/views/controls/menu/menu_runner.h"
#include "ui/views/controls/menu/submenu_view.h"
#include "ui/views/widget/widget.h"
namespace ash {
......@@ -93,6 +94,12 @@ base::TimeTicks AppMenuModelAdapter::GetClosingEventTime() {
return menu_runner_->closing_event_time();
}
views::Widget* AppMenuModelAdapter::GetSubmenuWidget() {
if (root_ && root_->GetSubmenu())
return root_->GetSubmenu()->GetWidget();
return nullptr;
}
void AppMenuModelAdapter::ExecuteCommand(int id, int mouse_event_flags) {
// Note that the command execution may cause this to get deleted - for
// example, for search result menus, the command could open an app window
......
......@@ -54,7 +54,10 @@ class APP_MENU_EXPORT AppMenuModelAdapter : public views::MenuModelAdapter {
base::TimeTicks GetClosingEventTime();
// Overridden from views::MenuModelAdapter:
// Gets the widget associated with the submenu. May return nullptr.
views::Widget* GetSubmenuWidget();
// views::MenuModelAdapter:
void ExecuteCommand(int id, int mouse_event_flags) override;
void OnMenuClosed(views::MenuItemView* menu) override;
......
......@@ -31,6 +31,7 @@
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/public/cpp/window_properties.h"
#include "ash/root_window_settings.h"
#include "ash/scoped_animation_disabler.h"
#include "ash/screen_util.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shelf/shelf.h"
......@@ -805,6 +806,17 @@ void RootWindowController::HideContextMenu() {
root_window_menu_model_adapter_->Cancel();
}
void RootWindowController::HideContextMenuNoAnimation() {
if (!IsContextMenuShown())
return;
views::Widget* submenu_widget =
root_window_menu_model_adapter_->GetSubmenuWidget();
DCHECK(submenu_widget);
ScopedAnimationDisabler disable(submenu_widget->GetNativeWindow());
root_window_menu_model_adapter_->Cancel();
}
bool RootWindowController::IsContextMenuShown() const {
return root_window_menu_model_adapter_ &&
root_window_menu_model_adapter_->IsShowingMenu();
......
......@@ -218,6 +218,7 @@ class ASH_EXPORT RootWindowController {
void ShowContextMenu(const gfx::Point& location_in_screen,
ui::MenuSourceType source_type);
void HideContextMenu();
void HideContextMenuNoAnimation();
bool IsContextMenuShown() const;
// Called when the login status changes after login (such as lock/unlock).
......
......@@ -6,6 +6,7 @@
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/presentation_time_recorder.h"
#include "ash/root_window_controller.h"
#include "ash/shell.h"
#include "ash/wm/desks/desk.h"
#include "ash/wm/desks/desks_controller.h"
......@@ -212,6 +213,9 @@ metrics_util::ReportCallback DeskActivationAnimation::GetReportCallback()
}
void DeskActivationAnimation::PrepareDeskForScreenshot(int index) {
for (auto* root_window_controller : Shell::GetAllRootWindowControllers())
root_window_controller->HideContextMenuNoAnimation();
// The order here matters. Overview must end before ending tablet split view
// before switching desks. (If clamshell split view is active on one or more
// displays, then it simply will end when we end overview.) That's because
......@@ -282,6 +286,9 @@ void DeskRemovalAnimation::OnStartingDeskScreenshotTakenInternal(
split_view_controller->EndSplitView(
SplitViewController::EndReason::kDesksChange);
for (auto* root_window_controller : Shell::GetAllRootWindowControllers())
root_window_controller->HideContextMenuNoAnimation();
// At the end of phase (1), we activate the target desk (i.e. the desk that
// will be activated after the active desk `desk_to_remove_index_` is
// removed). This means that phase (2) will take a screenshot of that desk
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment