Skip to content

Commit

Permalink
X11: X11Window must use bounds in px from display when fullscreen.
Browse files Browse the repository at this point in the history
When X11Window goes to fullscreen, it takes bounds of the display
and provides XServer with updated size and origin of itself.

XServer expects the bounds to be in pixels, but X11Window got the
bounds in dip.

This CL fixes the issue by taking display's size in pixels instead.
Also, a test was written to verify this functionality.

Bug: 1055057
Change-Id: If964856386e700a4a651303fe0e4ca81c769e851
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124710
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#755375}
  • Loading branch information
msisov authored and Commit Bot committed Apr 1, 2020
1 parent fae433e commit b393ab9
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 35 deletions.
65 changes: 31 additions & 34 deletions ui/ozone/platform/x11/x11_window_ozone_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "base/test/task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/display/screen_base.h"
#include "ui/events/devices/x11/touch_factory_x11.h"
#include "ui/events/event.h"
#include "ui/events/platform/x11/x11_event_source.h"
Expand Down Expand Up @@ -43,41 +43,21 @@ ACTION_P(CloneEvent, event_ptr) {
// ScreenOzone, but it is impossible.
// We are not really interested in sending back real displays. Thus, default one
// is more than enough.
class TestScreen : public display::Screen {
class TestScreen : public display::ScreenBase {
public:
TestScreen() : displays_({}) {}
~TestScreen() override = default;

// display::Screen interface.
gfx::Point GetCursorScreenPoint() override { return {}; }
bool IsWindowUnderCursor(gfx::NativeWindow window) override { return false; }
gfx::NativeWindow GetWindowAtScreenPoint(const gfx::Point& point) override {
return gfx::NativeWindow();
}
int GetNumDisplays() const override { return GetAllDisplays().size(); }
const std::vector<display::Display>& GetAllDisplays() const override {
return displays_;
}
display::Display GetDisplayNearestWindow(
gfx::NativeWindow window) const override {
return {};
}
display::Display GetDisplayNearestPoint(
const gfx::Point& point) const override {
return {};
TestScreen() {
ProcessDisplayChanged({}, true);
}
display::Display GetDisplayMatching(
const gfx::Rect& match_rect) const override {
return {};
~TestScreen() override = default;
TestScreen(const TestScreen& screen) = delete;
TestScreen& operator=(const TestScreen& screen) = delete;

void SetScaleAndBoundsForPrimaryDisplay(float scale,
const gfx::Rect& bounds_in_pixels) {
auto display = GetPrimaryDisplay();
display.SetScaleAndBounds(scale, bounds_in_pixels);
ProcessDisplayChanged(display, true);
}
display::Display GetPrimaryDisplay() const override { return {}; }
void AddObserver(display::DisplayObserver* observer) override {}
void RemoveObserver(display::DisplayObserver* observer) override {}

private:
std::vector<display::Display> displays_;

DISALLOW_COPY_AND_ASSIGN(TestScreen);
}; // namespace

} // namespace
Expand All @@ -94,7 +74,8 @@ class X11WindowOzoneTest : public testing::Test {
XDisplay* display = gfx::GetXDisplay();
event_source_ = std::make_unique<X11EventSource>(display);

display::Screen::SetScreenInstance(new TestScreen());
test_screen_ = new TestScreen();
display::Screen::SetScreenInstance(test_screen_);

TouchFactory::GetInstance()->SetPointerDeviceForTest({kPointerDeviceId});
}
Expand Down Expand Up @@ -126,6 +107,8 @@ class X11WindowOzoneTest : public testing::Test {
return window_manager;
}

TestScreen* test_screen_ = nullptr;

private:
std::unique_ptr<base::test::TaskEnvironment> task_env_;
std::unique_ptr<X11EventSource> event_source_;
Expand Down Expand Up @@ -268,4 +251,18 @@ TEST_F(X11WindowOzoneTest, MouseEnterAndDelete) {
EXPECT_FALSE(window_manager()->window_mouse_currently_on_for_test());
}

// Verifies X11Window sets fullscreen bounds in pixels when going to fullscreen.
TEST_F(X11WindowOzoneTest, ToggleFullscreen) {
constexpr gfx::Rect screen_bounds_in_px(640, 480, 1280, 720);
test_screen_->SetScaleAndBoundsForPrimaryDisplay(2, screen_bounds_in_px);

MockPlatformWindowDelegate delegate;
gfx::AcceleratedWidget widget;
constexpr gfx::Rect bounds(30, 80, 800, 600);
auto window = CreatePlatformWindow(&delegate, bounds, &widget);

EXPECT_CALL(delegate, OnBoundsChanged(screen_bounds_in_px));
window->ToggleFullscreen();
}

} // namespace ui
5 changes: 4 additions & 1 deletion ui/platform_window/x11/x11_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,10 @@ void X11Window::ToggleFullscreen() {
const display::Display display =
screen->GetDisplayMatching(bounds_in_pixels);
SetRestoredBoundsInPixels(bounds_in_pixels);
bounds_in_pixels = display.bounds();
bounds_in_pixels =
gfx::Rect(gfx::ScaleToFlooredPoint(display.bounds().origin(),
display.device_scale_factor()),
display.GetSizeInPixel());
} else {
// Exiting "browser fullscreen mode", but the X11 window is not necessarily
// in fullscreen state (e.g: a WM keybinding might have been used to toggle
Expand Down

0 comments on commit b393ab9

Please sign in to comment.