Skip to content

Commit

Permalink
[headless] Fix screen.width/height and scale on macOS
Browse files Browse the repository at this point in the history
Headless shell used the physical screen representation
implemented in //ui/display/mac/screen_mac.mm making it
dependent on the run time platform graphics properties.

This CL replaces headless shell screen representation with the
platform independent variant already used on Linux and Windows
platforms.

Bug: 347324963, 346931329, 346952284
Change-Id: I5fe930c7a1e1186fd62697b528dcacb4e47169fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5637879
Commit-Queue: Peter Kvitek <[email protected]>
Reviewed-by: Andrey Kosyakov <[email protected]>
Auto-Submit: Peter Kvitek <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1316635}
  • Loading branch information
Peter Kvitek authored and Chromium LUCI CQ committed Jun 18, 2024
1 parent 4a486f0 commit 7c8da15
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 13 deletions.
4 changes: 2 additions & 2 deletions headless/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,8 @@ component("headless_non_renderer") {
"lib/browser/headless_platform_event_source.h",
"lib/browser/headless_request_context_manager.cc",
"lib/browser/headless_request_context_manager.h",
"lib/browser/headless_screen.cc",
"lib/browser/headless_screen.h",
"lib/browser/headless_web_contents_impl.cc",
"lib/browser/headless_web_contents_impl.h",
"lib/browser/headless_window_tree_host.h",
Expand Down Expand Up @@ -309,8 +311,6 @@ component("headless_non_renderer") {
"lib/browser/headless_browser_impl_aura.cc",
"lib/browser/headless_focus_client.cc",
"lib/browser/headless_focus_client.h",
"lib/browser/headless_screen.cc",
"lib/browser/headless_screen.h",
"lib/browser/headless_window_parenting_client.cc",
"lib/browser/headless_window_parenting_client.h",
"lib/browser/headless_window_tree_host.cc",
Expand Down
3 changes: 0 additions & 3 deletions headless/lib/browser/headless_browser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class PrefService;
#endif

#if BUILDFLAG(IS_MAC)
#include "ui/display/screen.h"

namespace device {
class GeolocationSystemPermissionManager;
} // namespace device
Expand Down Expand Up @@ -145,7 +143,6 @@ class HEADLESS_EXPORT HeadlessBrowserImpl : public HeadlessBrowser {
base::OnceClosure quit_main_message_loop_;

#if BUILDFLAG(IS_MAC)
std::unique_ptr<display::ScopedNativeScreen> screen_;
std::unique_ptr<device::GeolocationSystemPermissionManager>
geolocation_system_permission_manager_;
#endif
Expand Down
7 changes: 6 additions & 1 deletion headless/lib/browser/headless_browser_impl_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
#include "headless/lib/browser/headless_screen.h"
#include "headless/lib/browser/headless_web_contents_impl.h"
#include "services/device/public/cpp/geolocation/system_geolocation_source_apple.h"
#import "ui/base/cocoa/base_view.h"
#include "ui/display/screen.h"
#import "ui/gfx/mac/coordinate_conversion.h"

// Overrides events and actions for NSPopUpButtonCell.
Expand Down Expand Up @@ -74,7 +76,10 @@ static void Init() {
device::SystemGeolocationSourceApple::
CreateGeolocationSystemPermissionManager();
}
screen_ = std::make_unique<display::ScopedNativeScreen>();

HeadlessScreen* screen = HeadlessScreen::Create(options()->window_size);
display::Screen::SetScreenInstance(screen);

HeadlessPopUpMethods::Init();
}

Expand Down
13 changes: 11 additions & 2 deletions headless/lib/browser/headless_screen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@

#include <stdint.h>

#include "ui/aura/env.h"
#include "ui/aura/window.h"
#include "ui/base/ime/input_method.h"
#include "ui/gfx/geometry/rect_conversions.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/native_widget_types.h"

#if !BUILDFLAG(IS_MAC)
#include "ui/aura/env.h"
#include "ui/aura/window.h"
#endif

namespace headless {

// static
Expand All @@ -23,7 +26,13 @@ HeadlessScreen* HeadlessScreen::Create(const gfx::Size& size) {
HeadlessScreen::~HeadlessScreen() = default;

gfx::Point HeadlessScreen::GetCursorScreenPoint() {
#if BUILDFLAG(IS_MAC)
return gfx::Point();
#else
// This does not look right: headless screen abstraction should not depend on
// a higher level window manager abstraction. See https://crbug.com/347789968.
return aura::Env::GetInstance()->last_mouse_location();
#endif
}

bool HeadlessScreen::IsWindowUnderCursor(gfx::NativeWindow window) {
Expand Down
5 changes: 0 additions & 5 deletions headless/test/headless_browser_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,10 @@ IN_PROC_BROWSER_TEST_F(HeadlessBrowserTest, DefaultSizes) {
const int expected_width = kDefaultOptions.window_size.width();
const int expected_height = kDefaultOptions.window_size.height();

// The screen size on macOS appears to be set to the actual desktop size, not
// the headless default window size as it is on other platforms. See
// https://crbug.com/347324963
#if !BUILDFLAG(IS_MAC)
EXPECT_THAT(EvaluateScript(web_contents, "screen.width"),
DictHasValue("result.result.value", expected_width));
EXPECT_THAT(EvaluateScript(web_contents, "screen.height"),
DictHasValue("result.result.value", expected_height));
#endif

EXPECT_THAT(EvaluateScript(web_contents, "window.outerWidth"),
DictHasValue("result.result.value", expected_width));
Expand Down

0 comments on commit 7c8da15

Please sign in to comment.