Skip to content

Commit

Permalink
Re-disable GPU, but only on windows (#142)
Browse files Browse the repository at this point in the history
Revert #141 and selectively re-apply to Windows.
  • Loading branch information
hadley authored Feb 5, 2024
1 parent e84718a commit 3d01fff
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,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 @@ -628,23 +628,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.
#' * [`"--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 @@ -654,6 +658,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 @@ -705,7 +712,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

0 comments on commit 3d01fff

Please sign in to comment.