Commit 69e6bccb authored by Jay Harris's avatar Jay Harris Committed by Commit Bot

Fixes a bug where ChromeOS popups were incorrectly sized.

We weren't taking the TopContainer's (where the window close buttons live)
height into account. This adds the result of GetTopInset to
GetWindowBoundsFromClientBounds on ChromeOS, which other platforms were
doing already. In addition, it ensures that the FrameHeader is correctly
setup, so the size constraints will be correct.

Before and After screenshots for opening a 480x300px popup
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=394786&signed_aid=1sRqzN7-EINNjSfsWHhj6A==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=394787&signed_aid=EIwCu-QkBAjpW1dGVHvu_Q==&inline=1

A second CL (https://chromium-review.googlesource.com/c/chromium/src/+/1626441)
fixes a separate but related bug and adds tests to ensure this does not regress.

Bug: 777854
Change-Id: Ia4f9170a5bf538fd40bd1ce34056a5693f773047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626909
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662977}
parent 8ad86466
...@@ -118,6 +118,12 @@ class ASH_EXPORT NonClientFrameViewAsh : public views::NonClientFrameView, ...@@ -118,6 +118,12 @@ class ASH_EXPORT NonClientFrameViewAsh : public views::NonClientFrameView,
views::Widget* frame() { return frame_; } views::Widget* frame() { return frame_; }
// Methods for testing.
static void set_use_empty_minimum_size_for_test(
bool use_empty_minimum_size_for_test) {
use_empty_minimum_size_for_test_ = use_empty_minimum_size_for_test;
}
protected: protected:
// Called when overview mode or split view state changed. If overview mode // Called when overview mode or split view state changed. If overview mode
// and split view mode are both active at the same time, the header of the // and split view mode are both active at the same time, the header of the
......
...@@ -238,6 +238,9 @@ void FrameHeader::SetCaptionButtonContainer( ...@@ -238,6 +238,9 @@ void FrameHeader::SetCaptionButtonContainer(
views::CAPTION_BUTTON_ICON_LEFT_SNAPPED, kWindowControlLeftSnappedIcon); views::CAPTION_BUTTON_ICON_LEFT_SNAPPED, kWindowControlLeftSnappedIcon);
caption_button_container_->SetButtonImage( caption_button_container_->SetButtonImage(
views::CAPTION_BUTTON_ICON_RIGHT_SNAPPED, kWindowControlRightSnappedIcon); views::CAPTION_BUTTON_ICON_RIGHT_SNAPPED, kWindowControlRightSnappedIcon);
// Perform layout to ensure the container height is correct.
LayoutHeaderInternal();
} }
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "ash/display/screen_orientation_controller.h" #include "ash/display/screen_orientation_controller.h"
#include "ash/display/screen_orientation_controller_test_api.h" #include "ash/display/screen_orientation_controller_test_api.h"
#include "ash/drag_drop/drag_drop_controller.h" #include "ash/drag_drop/drag_drop_controller.h"
#include "ash/frame/non_client_frame_view_ash.h"
#include "ash/public/cpp/app_types.h" #include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_features.h" #include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/fps_counter.h" #include "ash/public/cpp/fps_counter.h"
...@@ -143,6 +144,8 @@ class OverviewSessionTest : public AshTestBase { ...@@ -143,6 +144,8 @@ class OverviewSessionTest : public AshTestBase {
// AshTestBase: // AshTestBase:
void SetUp() override { void SetUp() override {
AshTestBase::SetUp(); AshTestBase::SetUp();
NonClientFrameViewAsh::set_use_empty_minimum_size_for_test(true);
aura::Env::GetInstance()->set_throttle_input_on_resize_for_testing(false); aura::Env::GetInstance()->set_throttle_input_on_resize_for_testing(false);
shelf_view_test_api_ = std::make_unique<ShelfViewTestAPI>( shelf_view_test_api_ = std::make_unique<ShelfViewTestAPI>(
GetPrimaryShelf()->GetShelfViewForTesting()); GetPrimaryShelf()->GetShelfViewForTesting());
...@@ -158,6 +161,7 @@ class OverviewSessionTest : public AshTestBase { ...@@ -158,6 +161,7 @@ class OverviewSessionTest : public AshTestBase {
false); false);
FpsCounter::SetForceReportZeroAnimationForTest(false); FpsCounter::SetForceReportZeroAnimationForTest(false);
trace_names_.clear(); trace_names_.clear();
NonClientFrameViewAsh::set_use_empty_minimum_size_for_test(false);
AshTestBase::TearDown(); AshTestBase::TearDown();
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "ash/wm/overview/scoped_overview_transform_window.h" #include "ash/wm/overview/scoped_overview_transform_window.h"
#include "ash/frame/non_client_frame_view_ash.h"
#include "ash/public/cpp/ash_features.h" #include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "ash/test/ash_test_base.h" #include "ash/test/ash_test_base.h"
...@@ -211,6 +212,8 @@ TEST_F(ScopedOverviewTransformWindowTest, TransformingPillaredRect) { ...@@ -211,6 +212,8 @@ TEST_F(ScopedOverviewTransformWindowTest, TransformingPillaredRect) {
// Tests the cases when very wide or tall windows enter overview mode. // Tests the cases when very wide or tall windows enter overview mode.
TEST_F(ScopedOverviewTransformWindowTest, ExtremeWindowBounds) { TEST_F(ScopedOverviewTransformWindowTest, ExtremeWindowBounds) {
NonClientFrameViewAsh::set_use_empty_minimum_size_for_test(true);
// Add three windows which in overview mode will be considered wide, tall and // Add three windows which in overview mode will be considered wide, tall and
// normal. Window |wide|, with size (400, 160) will be resized to (200, 160) // normal. Window |wide|, with size (400, 160) will be resized to (200, 160)
// when the 400x200 is rotated to 200x400, and should be considered a normal // when the 400x200 is rotated to 200x400, and should be considered a normal
...@@ -248,6 +251,8 @@ TEST_F(ScopedOverviewTransformWindowTest, ExtremeWindowBounds) { ...@@ -248,6 +251,8 @@ TEST_F(ScopedOverviewTransformWindowTest, ExtremeWindowBounds) {
EXPECT_EQ(GridWindowFillMode::kNormal, scoped_wide.type()); EXPECT_EQ(GridWindowFillMode::kNormal, scoped_wide.type());
EXPECT_EQ(GridWindowFillMode::kPillarBoxed, scoped_tall.type()); EXPECT_EQ(GridWindowFillMode::kPillarBoxed, scoped_tall.type());
EXPECT_EQ(GridWindowFillMode::kNormal, scoped_normal.type()); EXPECT_EQ(GridWindowFillMode::kNormal, scoped_normal.type());
NonClientFrameViewAsh::set_use_empty_minimum_size_for_test(false);
} }
// Tests that transients which should be invisible in overview do not have their // Tests that transients which should be invisible in overview do not have their
......
...@@ -55,6 +55,7 @@ ...@@ -55,6 +55,7 @@
#include "ui/events/gestures/gesture_recognizer.h" #include "ui/events/gestures/gesture_recognizer.h"
#include "ui/gfx/canvas.h" #include "ui/gfx/canvas.h"
#include "ui/gfx/color_palette.h" #include "ui/gfx/color_palette.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia.h"
#include "ui/gfx/scoped_canvas.h" #include "ui/gfx/scoped_canvas.h"
#include "ui/views/controls/label.h" #include "ui/views/controls/label.h"
...@@ -300,7 +301,10 @@ gfx::Rect BrowserNonClientFrameViewAsh::GetBoundsForClientView() const { ...@@ -300,7 +301,10 @@ gfx::Rect BrowserNonClientFrameViewAsh::GetBoundsForClientView() const {
gfx::Rect BrowserNonClientFrameViewAsh::GetWindowBoundsForClientBounds( gfx::Rect BrowserNonClientFrameViewAsh::GetWindowBoundsForClientBounds(
const gfx::Rect& client_bounds) const { const gfx::Rect& client_bounds) const {
return client_bounds; const int top_inset = GetTopInset(false);
return gfx::Rect(client_bounds.x(),
std::max(0, client_bounds.y() - top_inset),
client_bounds.width(), client_bounds.height() + top_inset);
} }
int BrowserNonClientFrameViewAsh::NonClientHitTest(const gfx::Point& point) { int BrowserNonClientFrameViewAsh::NonClientHitTest(const gfx::Point& point) {
......
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