Use private event loop for chrome_print(async=FALSE) (#127)

* Use private event loop for chrome_print(async=FALSE)

* update NEWS

DESCRIPTION

@@ -19,7 +19,7 @@ Description: Use the paged media properties in CSS and the JavaScript
   pages. Each page can have its page size, page numbers, margin boxes, and
   running headers, etc. Applications of this package include books, letters,
   reports, papers, business cards, resumes, and posters.
-Imports: rmarkdown (>= 1.14), bookdown (>= 0.8), htmltools, jsonlite, later,
+Imports: rmarkdown (>= 1.11), bookdown (>= 0.8), htmltools, jsonlite, later (>= 1.0.0),
   processx, servr (>= 0.13), httpuv, xfun, websocket
 Suggests: promises, testit, xaringan
 License: MIT + file LICENSE

NEWS.md

@@ -1,6 +1,8 @@
 # CHANGES IN pagedown VERSION 0.6
 
+## MINOR CHANGES
 
+- The `chrome_print()` function internally uses a private event loop provided by `later` 1.0.0 when `async=FALSE` (thanks, @jcheng5, #127).
 
 # CHANGES IN pagedown VERSION 0.5
 

R/chrome.R

@@ -86,86 +86,104 @@ chrome_print = function(
   if (!is_remote_protocol_ok(debug_port, verbose = verbose))
     stop('A more recent version of Chrome is required. ')
 
-  ws = websocket::WebSocket$new(get_entrypoint(debug_port), autoConnect = FALSE)
-  ws$onClose(kill_chrome)
-  ws$onError(kill_chrome)
-  close_ws = function() {
-    if (verbose >= 1) message('Closing websocket connection')
-    ws$close()
-  }
-
-  if (file.exists(input)) {
-    is_html = function(x) grepl('[.]html?$', x)
-    url = if (is_html(input)) input else rmarkdown::render(
-      input, envir = parent.frame(), encoding = 'UTF-8'
-    )
-    if (!is_html(url)) stop(
-      "The file '", url, "' should have the '.html' or '.htm' extension."
-    )
-    svr = servr::httd(
-      dirname(url), daemon = TRUE, browser = FALSE, verbose = verbose >= 1,
-      port = random_port(), initpath = httpuv::encodeURIComponent(basename(url))
-    )
-    stop_server = function(...) {
-      if (verbose >= 1) message('Closing local webserver')
-      svr$stop_server()
+  # If !async, use a private event loop to drive the websocket. This is
+  # necessary to separate later callbacks relevant to chrome_print, from any
+  # other callbacks that have been scheduled before entering chrome_print; the
+  # latter must not be invoked while inside of any synchronous function,
+  # including chrome_print(async=FALSE).
+  #
+  # It's also critical that none of the code inside with_temp_loop waits on a
+  # promise that originates from outside the with_temp_loop, as it will cause
+  # the code inside to hang. And finally, no promise from inside with_temp_loop
+  # should escape to the outside either, as with_temp_loop uses a truly "temp"
+  # loop--it will be destroyed when with_temp_loop completes.
+  #
+  # Therefore, if async, it's important NOT to use a private event loop.
+  with_temp_loop_maybe <- if (async) identity else later::with_temp_loop
+
+  with_temp_loop_maybe({
+
+    ws = websocket::WebSocket$new(get_entrypoint(debug_port), autoConnect = FALSE)
+    ws$onClose(kill_chrome)
+    ws$onError(kill_chrome)
+    close_ws = function() {
+      if (verbose >= 1) message('Closing websocket connection')
+      ws$close()
     }
-    on.exit(stop_server(), add = TRUE)
-    ws$onClose(stop_server)
-    ws$onError(stop_server)
-    url = svr$url
-  } else url = input  # the input is not a local file; assume it is just a URL
-
-  format = match.arg(format)
-  # remove hash/query parameters in url
-  if (missing(output) && !file.exists(input))
-    output = xfun::with_ext(basename(gsub('[#?].*', '', url)), format)
-  output2 = normalizePath(output, mustWork = FALSE)
-  if (!dir.exists(d <- dirname(output2)) && !dir.create(d, recursive = TRUE)) stop(
-    'Cannot create the directory for the output file: ', d
-  )
 
-  if ((format == 'pdf') && !all(c(missing(selector), missing(box_model), missing(scale))))
-    warning('For "pdf" format, arguments `selector`, `box_model` and `scale` are ignored.', call. = FALSE)
-
-  box_model = match.arg(box_model)
-
-  pr = NULL
-  res_fun = function(value) {} # default: do nothing
-  rej_fun = function(reason) {} # default: do nothing
-  if (async) {
-    pr_print = promises::promise(function(resolve, reject) {
-      res_fun <<- resolve
-      rej_fun <<- function(reason) reject(paste('Failed to generate output. Reason:', reason))
-    })
-    pr_timeout = promises::promise(function(resolve, reject) {
-      later::later(
-        ~reject(paste('Failed to generate output in', timeout, 'seconds (timeout).')),
-        timeout
+    if (file.exists(input)) {
+      is_html = function(x) grepl('[.]html?$', x)
+      url = if (is_html(input)) input else rmarkdown::render(
+        input, envir = parent.frame(), encoding = 'UTF-8'
       )
-    })
-    pr = promises::promise_race(pr_print, pr_timeout)
-    promises::finally(pr, close_ws)
-  }
+      if (!is_html(url)) stop(
+        "The file '", url, "' should have the '.html' or '.htm' extension."
+      )
+      svr = servr::httd(
+        dirname(url), daemon = TRUE, browser = FALSE, verbose = verbose >= 1,
+        port = random_port(), initpath = httpuv::encodeURIComponent(basename(url))
+      )
+      stop_server = function(...) {
+        if (verbose >= 1) message('Closing local webserver')
+        svr$stop_server()
+      }
+      on.exit(stop_server(), add = TRUE)
+      ws$onClose(stop_server)
+      ws$onError(stop_server)
+      url = svr$url
+    } else url = input  # the input is not a local file; assume it is just a URL
+
+    format = match.arg(format)
+    # remove hash/query parameters in url
+    if (missing(output) && !file.exists(input))
+      output = xfun::with_ext(basename(gsub('[#?].*', '', url)), format)
+    output2 = normalizePath(output, mustWork = FALSE)
+    if (!dir.exists(d <- dirname(output2)) && !dir.create(d, recursive = TRUE)) stop(
+      'Cannot create the directory for the output file: ', d
+    )
 
-  t0 = Sys.time(); token = new.env(parent = emptyenv())
-  on.exit(close_ws())
-  print_page(ws, url, output2, wait, verbose, token, format, options, selector, box_model, scale, res_fun, rej_fun)
+    if ((format == 'pdf') && !all(c(missing(selector), missing(box_model), missing(scale))))
+      warning('For "pdf" format, arguments `selector`, `box_model` and `scale` are ignored.', call. = FALSE)
+
+    box_model = match.arg(box_model)
+
+    pr = NULL
+    res_fun = function(value) {} # default: do nothing
+    rej_fun = function(reason) {} # default: do nothing
+    if (async) {
+      pr_print = promises::promise(function(resolve, reject) {
+        res_fun <<- resolve
+        rej_fun <<- function(reason) reject(paste('Failed to generate output. Reason:', reason))
+      })
+      pr_timeout = promises::promise(function(resolve, reject) {
+        later::later(
+          ~reject(paste('Failed to generate output in', timeout, 'seconds (timeout).')),
+          timeout
+        )
+      })
+      pr = promises::promise_race(pr_print, pr_timeout)
+      promises::finally(pr, close_ws)
+    }
 
-  if (async) {
-    on.exit()
-    return(pr)
-  }
+    t0 = Sys.time(); token = new.env(parent = emptyenv())
+    on.exit(close_ws())
+    print_page(ws, url, output2, wait, verbose, token, format, options, selector, box_model, scale, res_fun, rej_fun)
 
-  while (!isTRUE(token$done)) {
-    if (!is.null(e <- token$error)) stop('Failed to generate output. Reason: ', e)
-    if (as.numeric(difftime(Sys.time(), t0, units = 'secs')) > timeout) stop(
-      'Failed to generate output in ', timeout, ' seconds (timeout).'
-    )
-    later::run_now()
-  }
+    if (async) {
+      on.exit()
+      return(pr)
+    }
 
-  invisible(output)
+    while (!isTRUE(token$done)) {
+      if (!is.null(e <- token$error)) stop('Failed to generate output. Reason: ', e)
+      if (as.numeric(difftime(Sys.time(), t0, units = 'secs')) > timeout) stop(
+        'Failed to generate output in ', timeout, ' seconds (timeout).'
+      )
+      later::run_now()
+    }
+
+    invisible(output)
+  })
 }
 
 #' Find Google Chrome or Chromium in the system