Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] Adjust the bindings-exposed screen size in headless browser mode #13998

Merged
merged 1 commit into from May 18, 2023

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented May 18, 2023

e82dfc6

[iOS] Adjust the bindings-exposed screen size in headless browser mode
https://bugs.webkit.org/show_bug.cgi?id=256939

Reviewed by Alan Baradlay.

Add different, platform-specific strategies when returning an appropriate screen size for bindings
in headless browsing. See below for more details.

Changes covered by a new API test (and adjustments to some existing tests).

* Source/WebCore/page/ChromeClient.h:
(WebCore::ChromeClient::screenSizeForHeadlessMode const):

Add a chrome client hook to return a screen size specifically for "headless mode", given the
requesting frame and the actual screen size.

* Source/WebCore/page/LocalFrame.cpp:
(WebCore::LocalFrame::screenSize const):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::screenSizeForHeadlessMode const):

Add some plumbing out to `WebPage`.

* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::screenSizeForHeadlessMode const):

Move the logic currently in `LocalFrame::screenSize` over to this method; additionally, instead of
trying to counterscale the innerWidth/innerHeight, simply return the unobscured content rect; this
does not change with `pageScaleFactor` on macOS, eliminating the need for counterscaling.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::screenSizeForHeadlessMode const):

Implement an iOS-specific strategy for `screenSizeForHeadlessMode`, returning one of three canned
values on iPhone (based on the current user idiom), and falling back to the initial containing block
(ignoring the current scale factor, by using `ViewportConfiguration::minimumLayoutSize()`) on iPad.

Canonical link: https://commits.webkit.org/264200@main

c958a4c

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@whsieh whsieh requested a review from cdumez as a code owner May 18, 2023 01:46
@whsieh whsieh self-assigned this May 18, 2023
@whsieh whsieh added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label May 18, 2023
return m_viewportConfiguration.minimumLayoutSize();

static constexpr std::array fixedSizes {
FloatSize { 320, 568 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did these values come from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values represent device metrics from several iPhone models. While somewhat arbitrary, I did make sure to pick the most prevalent width values, by number of models with that width.

@whsieh
Copy link
Member Author

whsieh commented May 18, 2023

Thanks for the review!

@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label May 18, 2023
https://bugs.webkit.org/show_bug.cgi?id=256939

Reviewed by Alan Baradlay.

Add different, platform-specific strategies when returning an appropriate screen size for bindings
in headless browsing. See below for more details.

Changes covered by a new API test (and adjustments to some existing tests).

* Source/WebCore/page/ChromeClient.h:
(WebCore::ChromeClient::screenSizeForHeadlessMode const):

Add a chrome client hook to return a screen size specifically for "headless mode", given the
requesting frame and the actual screen size.

* Source/WebCore/page/LocalFrame.cpp:
(WebCore::LocalFrame::screenSize const):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::screenSizeForHeadlessMode const):

Add some plumbing out to `WebPage`.

* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::screenSizeForHeadlessMode const):

Move the logic currently in `LocalFrame::screenSize` over to this method; additionally, instead of
trying to counterscale the innerWidth/innerHeight, simply return the unobscured content rect; this
does not change with `pageScaleFactor` on macOS, eliminating the need for counterscaling.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::screenSizeForHeadlessMode const):

Implement an iOS-specific strategy for `screenSizeForHeadlessMode`, returning one of three canned
values on iPhone (based on the current user idiom), and falling back to the initial containing block
(ignoring the current scale factor, by using `ViewportConfiguration::minimumLayoutSize()`) on iPad.

Canonical link: https://commits.webkit.org/264200@main
@webkit-commit-queue
Copy link
Collaborator

Committed 264200@main (e82dfc6): https://commits.webkit.org/264200@main

Reviewed commits have been landed. Closing PR #13998 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit e82dfc6 into WebKit:main May 18, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
Projects
None yet
5 participants