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

Re-disable GPU, but only on windows #142

Merged
merged 6 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

* Breaking change: `Chromote$is_active()` method now reports if there is an active connection to the underlying chrome instance, rather than whether or not that instance is alive (#94).

* `--disable-gpu` is no longer included in the default Chrome arguments.
* `--disable-gpu` is no longer included in the default Chrome arguments, except on windows where empirically it appears to be necessary (otherwise GHA check runs never terminate).

* `ChromoteSession` now records the `targetId`. This eliminates one round-trip to the browser when viewing or closing a session. You can now call the `$respawn()` method if a session terminates and you want to reconnect to the same target (#94).

Expand Down
19 changes: 13 additions & 6 deletions R/chromote.R
Original file line number Diff line number Diff line change
Expand Up @@ -599,23 +599,27 @@ is_missing_linux_user <- cache_value(function() {
#' Default chromote arguments are composed of the following values (when
#' appropriate):
#'
#' * [`"--disable-gpu"`](https://peter.sh/experiments/chromium-command-line-switches/#disable-gpu)
#' * Only added on Windows, as empirically it appears to be needed
#' (if not, check runs on GHA never terminate).
#' * Disables GPU hardware acceleration. If software renderer is not in place, then the GPU process won't launch.
#' * [`"--no-sandbox"`](https://peter.sh/experiments/chromium-command-line-switches/#no-sandbox)
#' * Only added when `CI` system environment variable is set, when the
#' user on a Linux system is not set, or when executing inside a Docker container.
#' * \verb{Disables the sandbox for all process types that are normally sandboxed. Meant to be used as a browser-level switch for testing purposes only}
#' * Disables the sandbox for all process types that are normally sandboxed. Meant to be used as a browser-level switch for testing purposes only
#' * [`"--disable-dev-shm-usage"`](https://peter.sh/experiments/chromium-command-line-switches/#disable-dev-shm-usage)
#' * Only added when `CI` system environment variable is set or when inside a docker instance.
#' * \verb{The /dev/shm partition is too small in certain VM environments, causing Chrome to fail or crash}
#' * The `/dev/shm` partition is too small in certain VM environments, causing Chrome to fail or crash.
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why these entire sentences were wrapped in \verb

#' * [`"--force-color-profile=srgb"`](https://peter.sh/experiments/chromium-command-line-switches/#force-color-profile)
#' * This means that screenshots taken on a laptop plugged into an external
#' monitor will often have subtly different colors than one taken when
#' the laptop is using its built-in monitor. This problem will be even
#' more likely across machines.
#' * \verb{Force all monitors to be treated as though they have the specified color profile.}
#' * Force all monitors to be treated as though they have the specified color profile.
#' * [`"--disable-extensions"`](https://peter.sh/experiments/chromium-command-line-switches/#disable-extensions)
#' * \verb{Disable extensions.}
#' * Disable extensions.
#' * [`"--mute-audio"`](https://peter.sh/experiments/chromium-command-line-switches/#mute-audio)
#' * \verb{Mutes audio sent to the audio device so it is not audible during automated testing}
#' * Mutes audio sent to the audio device so it is not audible during automated testing.
#'
#' @return A character vector of default command-line arguments to be used with
#' every new [`ChromoteSession`]
Expand All @@ -625,6 +629,9 @@ is_missing_linux_user <- cache_value(function() {
#' @export
default_chrome_args <- function() {
c(
# Empirically, appears to be needed for check runs to terminate on GHA
if (is_windows()) "--disable-gpu",

# > Note: --no-sandbox is not needed if you properly setup a user in the container.
# https://developers.google.com/web/updates/2017/04/headless-chrome
if (is_inside_ci() || is_missing_linux_user() || is_inside_docker()) {
Expand Down Expand Up @@ -676,7 +683,7 @@ reset_chrome_args <- function() {
#' @examples
#' old_chrome_args <- get_chrome_args()
#'
#' # Disable the gpu and use `/dev/shm`
#' # Disable the gpu and use of `/dev/shm`
#' set_chrome_args(c("--disable-gpu", "--disable-dev-shm-usage"))
#'
#' #... Make new `Chrome` or `ChromoteSession` instance
Expand Down
18 changes: 12 additions & 6 deletions man/default_chrome_args.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/testthat/helper.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
skip_if_no_chromote <- function() {
skip_on_cran()
skip_on_os("windows") # currently hangs the test process
skip_if(lacks_chromote(), "chromote not available")
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-default_chromote_args.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

min_chrome_arg_length <- if (is_inside_ci()) 4 else 3
min_chrome_arg_length <- 3 + is_inside_ci() + is_windows()

test_that("default args are retrieved", {
expect_gte(length(default_chrome_args()), min_chrome_arg_length)
Expand Down
Loading