Skip to content

Commit

Permalink
Revert "Don't maximize/fullscreen whe max size is set."
Browse files Browse the repository at this point in the history 
This reverts commit be45544.

Reason for revert: b/232035858 -- breaking borealis

Original change's description:
> Don't maximize/fullscreen whe max size is set.
> Also fixes that did not explicitly set CanMaximize or CanResize even
> though they depended on the behaviour.
>
> Test: covered by new WindowStateTest.ResizeSettingIsRespected.
> Fixed: 1311484
> Change-Id: Ia2650f2c0900e449d2805c977109fdbe547196ab
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584030
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Fred Shih <ffred@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#997395}

Change-Id: I3623eab6ce59803a7713e4b8280119e6429d50ac
Bug: b/232035858
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3639591
Commit-Queue: Fred Shih <ffred@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001788}
  • Loading branch information
Fred Shih authored and Chromium LUCI CQ committed May 10, 2022
1 parent 095a262 commit c9d17d2
Show file tree
Hide file tree
Showing 18 changed files with 59 additions and 187 deletions.
12 changes: 1 addition & 11 deletions ash/assistant/test/assistant_ash_test_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "base/test/task_environment.h"
#include "chromeos/services/assistant/test_support/scoped_assistant_browser_delegate.h"
#include "ui/views/view_utils.h"
#include "ui/views/widget/widget_delegate.h"

namespace ash {

Expand Down Expand Up @@ -284,16 +283,7 @@ aura::Window* AssistantAshTestBase::SwitchToNewAppWindow() {
}

views::Widget* AssistantAshTestBase::SwitchToNewWidget() {
class TestWidgetDelegate : public views::WidgetDelegate {
public:
TestWidgetDelegate() {
SetOwnedByWidget(true);
// Maximize must be set to false. Otherwise the widget would start
// maximized in tablet mode and does not handle gesture events correctly.
SetCanMaximize(false);
};
};
widgets_.push_back(CreateTestWidget(new TestWidgetDelegate));
widgets_.push_back(CreateTestWidget());

views::Widget* result = widgets_.back().get();
// Give the widget a non-zero size, otherwise things like tapping and clicking
Expand Down
8 changes: 2 additions & 6 deletions ash/frame/non_client_frame_view_ash_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ using ::chromeos::kTrackDefaultFrameColors;
class NonClientFrameViewAshTestWidgetDelegate
: public views::WidgetDelegateView {
public:
NonClientFrameViewAshTestWidgetDelegate() : views::WidgetDelegateView() {
SetCanMaximize(true);
SetCanResize(true);
}
NonClientFrameViewAshTestWidgetDelegate() = default;

NonClientFrameViewAshTestWidgetDelegate(
const NonClientFrameViewAshTestWidgetDelegate&) = delete;
Expand Down Expand Up @@ -319,7 +316,6 @@ TEST_F(NonClientFrameViewAshTest, FrameHiddenInTabletModeForMaximizedWindows) {
TEST_F(NonClientFrameViewAshTest,
FrameShownInTabletModeForNonMaximizedWindows) {
auto* delegate = new NonClientFrameViewAshTestWidgetDelegate();
delegate->SetCanMaximize(false);
std::unique_ptr<views::Widget> widget = CreateTestWidget(delegate);

Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
Expand Down Expand Up @@ -609,7 +605,7 @@ TEST_F(NonClientFrameViewAshTest, CustomButtonModel) {
EXPECT_TRUE(test_api.close_button()->GetVisible());

EXPECT_FALSE(test_api.minimize_button()->GetVisible());
EXPECT_TRUE(test_api.size_button()->GetVisible());
EXPECT_FALSE(test_api.size_button()->GetVisible());
EXPECT_FALSE(test_api.menu_button()->GetVisible());

// Back button
Expand Down
3 changes: 1 addition & 2 deletions ash/frame/wide_frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ void WideFrameView::Layout() {

void WideFrameView::OnMouseEvent(ui::MouseEvent* event) {
if (event->IsOnlyLeftMouseButton()) {
if ((event->flags() & ui::EF_IS_DOUBLE_CLICK) &&
event->type() == ui::ET_MOUSE_PRESSED) {
if ((event->flags() & ui::EF_IS_DOUBLE_CLICK)) {
base::RecordAction(
base::UserMetricsAction("Caption_ClickTogglesMaximize"));
const WMEvent wm_event(WM_EVENT_TOGGLE_MAXIMIZE_CAPTION);
Expand Down
14 changes: 1 addition & 13 deletions ash/root_window_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include "ui/base/ime/input_method.h"
#include "ui/base/ime/text_input_client.h"
#include "ui/base/ui_base_features.h"
#include "ui/base/ui_base_types.h"
#include "ui/display/manager/display_manager.h"
#include "ui/display/test/display_manager_test_api.h"
#include "ui/events/test/event_generator.h"
Expand Down Expand Up @@ -91,17 +90,8 @@ aura::LayoutManager* GetLayoutManager(RootWindowController* controller,
class RootWindowControllerTest : public AshTestBase {
public:
views::Widget* CreateTestWidget(const gfx::Rect& bounds) {
auto default_delegate = std::make_unique<views::WidgetDelegateView>();
default_delegate->SetCanMaximize(true);
default_delegate->SetCanResize(true);
default_delegate->SetOwnedByWidget(true);
return CreateTestWidget(bounds, default_delegate.release());
}

views::Widget* CreateTestWidget(const gfx::Rect& bounds,
views::WidgetDelegateView* delegate) {
views::Widget* widget =
views::Widget::CreateWindowWithContext(delegate, GetContext());
views::Widget::CreateWindowWithContext(nullptr, GetContext());
// Any initial bounds are constrained to the screen work area or the parent.
// See Widget::InitialBounds() & Widget::SetBoundsConstrained(). Explicitly
// setting the bounds here will allow the view to be positioned such that it
Expand All @@ -113,9 +103,7 @@ class RootWindowControllerTest : public AshTestBase {

views::WidgetDelegate* CreateModalWidgetDelegate() {
auto delegate = std::make_unique<views::WidgetDelegateView>();
delegate->SetCanMaximize(true);
delegate->SetModalType(ui::MODAL_TYPE_SYSTEM);
delegate->SetOwnedByWidget(true);
return delegate.release();
}

Expand Down
35 changes: 16 additions & 19 deletions ash/shelf/shelf_layout_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
#include "base/test/scoped_feature_list.h"
#include "chromeos/ui/base/window_properties.h"
#include "components/prefs/pref_service.h"
#include "components/session_manager/session_manager_types.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/client/drag_drop_client.h"
Expand Down Expand Up @@ -421,9 +420,6 @@ TEST_F(ShelfLayoutManagerTest, AutoHide) {
UpdateAutoHideStateNow();
EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());

// Maximize the widget so that we don't accidentally go into overview mode
// (e.g.: if the window happened to be snapped to the edge).
widget->Maximize();
// Switch to tablet mode should hide the AUTO_HIDE_SHOWN shelf even the mouse
// cursor is inside the shelf area.
EXPECT_FALSE(TabletModeControllerTestApi().IsTabletModeStarted());
Expand Down Expand Up @@ -648,7 +644,8 @@ TEST_F(ShelfLayoutManagerTest, VisibleInOverview) {

// Tests that the shelf is visible when in overview mode.
EnterOverview();
WaitForOverviewAnimation(/*enter=*/true);
ShellTestApi().WaitForOverviewAnimationState(
OverviewAnimationState::kEnterAnimationComplete);

EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
Expand All @@ -657,7 +654,8 @@ TEST_F(ShelfLayoutManagerTest, VisibleInOverview) {

// Test that on exiting overview mode, the shelf returns to auto hide state.
ExitOverview();
WaitForOverviewAnimation(/*enter=*/false);
ShellTestApi().WaitForOverviewAnimationState(
OverviewAnimationState::kExitAnimationComplete);

EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
Expand Down Expand Up @@ -1020,12 +1018,12 @@ TEST_F(ShelfLayoutManagerTest, DualDisplayOpenAppListWithShelfAutoHideState) {
shelf_2->shelf_layout_manager()->LayoutShelf();

// Create a window in each display and show them in maximized state.
aura::Window* window_1 =
CreateTestWindowInParent(root_windows[0], gfx::Rect(0, 0, 100, 100));
aura::Window* window_1 = CreateTestWindowInParent(root_windows[0]);
window_1->SetBounds(gfx::Rect(0, 0, 100, 100));
window_1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
window_1->Show();
aura::Window* window_2 =
CreateTestWindowInParent(root_windows[1], gfx::Rect(201, 0, 100, 100));
aura::Window* window_2 = CreateTestWindowInParent(root_windows[1]);
window_2->SetBounds(gfx::Rect(201, 0, 100, 100));
window_2->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
window_2->Show();

Expand Down Expand Up @@ -1287,12 +1285,12 @@ TEST_F(ShelfLayoutManagerTest, ShelfWithSystemModalWindowDualDisplay) {
shelf_2->shelf_layout_manager()->LayoutShelf();

// Create a window in each display and show them in maximized state.
aura::Window* window_1 =
CreateTestWindowInParent(root_windows[0], gfx::Rect(0, 0, 100, 100));
aura::Window* window_1 = CreateTestWindowInParent(root_windows[0]);
window_1->SetBounds(gfx::Rect(0, 0, 100, 100));
window_1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
window_1->Show();
aura::Window* window_2 =
CreateTestWindowInParent(root_windows[1], gfx::Rect(201, 0, 100, 100));
aura::Window* window_2 = CreateTestWindowInParent(root_windows[1]);
window_2->SetBounds(gfx::Rect(201, 0, 100, 100));
window_2->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
window_2->Show();

Expand All @@ -1315,19 +1313,18 @@ TEST_F(ShelfLayoutManagerTest, ShelfWithSystemModalWindowDualDisplay) {

TEST_F(ShelfLayoutManagerTest, FullscreenWidgetHidesShelf) {
Shelf* shelf = GetPrimaryShelf();

// Create a normal window.
views::Widget* widget = CreateTestWidget();
widget->SetBounds(gfx::Rect(11, 22, 300, 400));
views::Widget* widget = TestWidgetBuilder()
.SetBounds(gfx::Rect(11, 22, 300, 400))
.BuildOwnedByNativeWidget();
ASSERT_FALSE(widget->IsFullscreen());

// Shelf defaults to visible.
EXPECT_EQ(SHELF_VISIBLE, shelf->GetVisibilityState());

// Fullscreen window hides it.
widget->SetFullscreen(true);
EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
EXPECT_EQ(SHELF_HIDDEN, shelf->GetVisibilityState());

// Restoring the window restores it.
widget->Restore();
Expand Down
61 changes: 24 additions & 37 deletions ash/shelf/test/shelf_layout_manager_test_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "ash/shelf/shelf_layout_manager.h"
#include "ash/shelf/shelf_view.h"
#include "ash/shell.h"
#include "ash/test/test_widget_builder.h"
#include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
#include "ash/wm/window_state.h"
#include "ash/wm/wm_event.h"
Expand All @@ -19,12 +18,9 @@
#include "components/prefs/pref_service.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/client/window_parenting_client.h"
#include "ui/aura/test/test_window_delegate.h"
#include "ui/aura/window.h"
#include "ui/aura/window_delegate.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/view.h"
#include "ui/wm/core/window_util.h"

Expand Down Expand Up @@ -186,37 +182,29 @@ aura::Window* ShelfLayoutManagerTestBase::CreateTestWindow() {
aura::Window* window = new aura::Window(nullptr);
window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
window->SetType(aura::client::WINDOW_TYPE_NORMAL);
window->SetProperty(aura::client::kResizeBehaviorKey,
aura::client::kResizeBehaviorCanMaximize);
window->Init(ui::LAYER_TEXTURED);
ParentWindowInPrimaryRootWindow(window);
return window;
}

aura::Window* ShelfLayoutManagerTestBase::CreateTestWindowInParent(
aura::Window* root_window) {
return CreateTestWindowInParent(root_window, gfx::Rect());
}

aura::Window* ShelfLayoutManagerTestBase::CreateTestWindowInParent(
aura::Window* root_window,
const gfx::Rect& bounds) {
aura::Window* window = new aura::Window(nullptr);
window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
window->SetType(aura::client::WINDOW_TYPE_NORMAL);
window->Init(ui::LAYER_TEXTURED);
aura::client::ParentWindowWithContext(window, root_window, bounds);
aura::client::ParentWindowWithContext(window, root_window, gfx::Rect());
return window;
}

views::Widget* ShelfLayoutManagerTestBase::CreateTestWidget() {
return TestWidgetBuilder()
.SetWidgetType(views::Widget::InitParams::TYPE_WINDOW)
.SetTestWidgetDelegate()
.SetContext(GetContext())
.SetBounds(gfx::Rect(200, 200))
.SetShow(true)
.BuildOwnedByNativeWidget();
views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
params.bounds = gfx::Rect(0, 0, 200, 200);
params.context = GetContext();
views::Widget* widget = new views::Widget;
widget->Init(std::move(params));
widget->Show();
return widget;
}

gfx::Rect ShelfLayoutManagerTestBase::GetVisibleShelfWidgetBoundsInScreen() {
Expand Down Expand Up @@ -411,8 +399,7 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
views::Widget* widget = CreateTestWidget();
widget->Maximize();

// The time delta should be large enough to prevent accidental fling
// creation.
// The time delta should be large enough to prevent accidental fling creation.
const base::TimeDelta kTimeDelta = base::Milliseconds(100);

aura::Window* window = widget->GetNativeWindow();
Expand Down Expand Up @@ -502,11 +489,11 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
}
EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
// Gesture drag should not change the auto hide behavior of shelf, even
// though its visibility has been changed.
// Gesture drag should not change the auto hide behavior of shelf, even though
// its visibility has been changed.
EXPECT_EQ(ShelfAutoHideBehavior::kAlways, shelf->auto_hide_behavior());
// The auto-hide shelf is above the window, which should not change the
// bounds of the window.
// The auto-hide shelf is above the window, which should not change the bounds
// of the window.
EXPECT_EQ(window_bounds_with_noshelf.ToString(), window->bounds().ToString());
EXPECT_EQ(shelf_shown.ToString(),
GetShelfWidget()->GetWindowBoundsInScreen().ToString());
Expand Down Expand Up @@ -615,8 +602,8 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
EXPECT_EQ(shelf_hidden.ToString(),
GetShelfWidget()->GetWindowBoundsInScreen().ToString());

// Put |widget| into fullscreen. Set the shelf to be auto hidden when
// |widget| is fullscreen. (eg browser immersive fullscreen).
// Put |widget| into fullscreen. Set the shelf to be auto hidden when |widget|
// is fullscreen. (eg browser immersive fullscreen).
widget->SetFullscreen(true);
WindowState::Get(window)->SetHideShelfWhenFullscreen(false);
layout_manager->UpdateVisibilityState();
Expand Down Expand Up @@ -659,8 +646,8 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
GetShelfWidget()->GetWindowBoundsInScreen().ToString());
EXPECT_EQ(window_bounds_fullscreen.ToString(), window->bounds().ToString());

// Set the shelf to be hidden when |widget| is fullscreen. (eg tab
// fullscreen with or without immersive browser fullscreen).
// Set the shelf to be hidden when |widget| is fullscreen. (eg tab fullscreen
// with or without immersive browser fullscreen).
WindowState::Get(window)->SetHideShelfWhenFullscreen(true);

layout_manager->UpdateVisibilityState();
Expand Down Expand Up @@ -690,8 +677,8 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
EXPECT_EQ(ShelfAutoHideBehavior::kAlways, shelf->auto_hide_behavior());

// Swipe-down to hide. This should have no effect because there are no
// visible windows.
// Swipe-down to hide. This should have no effect because there are no visible
// windows.
{
SCOPED_TRACE("SWIPE_DOWN_AUTO_HIDE_5");
generator->GestureScrollSequenceWithCallback(
Expand All @@ -715,9 +702,9 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(layout_manager->HasVisibleWindow());

// Swipe up on the shelf. This should show the shelf but should not change
// the auto-hide behavior, since auto-hide behavior can only be changed
// through context menu of the shelf.
// Swipe up on the shelf. This should show the shelf but should not change the
// auto-hide behavior, since auto-hide behavior can only be changed through
// context menu of the shelf.
{
SCOPED_TRACE("SWIPE_UP_AUTO_HIDE_2");
// Do not check bounds because the events outside of the bounds
Expand All @@ -736,8 +723,8 @@ void ShelfLayoutManagerTestBase::RunGestureDragTests(
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(layout_manager->HasVisibleWindow());

// Swipe-down to hide. This should have no effect because there are no
// visible windows.
// Swipe-down to hide. This should have no effect because there are no visible
// windows.
{
SCOPED_TRACE("SWIPE_DOWN_AUTO_HIDE_6");
generator->GestureScrollSequenceWithCallback(
Expand Down
2 changes: 0 additions & 2 deletions ash/shelf/test/shelf_layout_manager_test_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ class ShelfLayoutManagerTestBase : public AshTestBase {

aura::Window* CreateTestWindow();
aura::Window* CreateTestWindowInParent(aura::Window* root_window);
aura::Window* CreateTestWindowInParent(aura::Window* root_window,
const gfx::Rect& bounds);

// Create a simple widget in the current context (will delete on TearDown).
views::Widget* CreateTestWidget();
Expand Down
2 changes: 0 additions & 2 deletions ash/system/network/network_detailed_view_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ class NetworkDetailedViewTest : public AshTestBase {
list_type_);

widget_ = CreateFramelessTestWidget();
ASSERT_TRUE(widget_->widget_delegate()->CanMaximize());
widget_->SetFullscreen(true);
DCHECK(widget_->IsFullscreen());
widget_->SetContentsView(network_detailed_view_);

base::RunLoop().RunUntilIdle();
Expand Down

0 comments on commit c9d17d2

Please sign in to comment.