RLesur / crrri

A Chrome Remote Interface written in R
https://rlesur.github.io/crrri/
Other
157 stars 12 forks source link

Allow children to close parent connection #93

Open ColinFay opened 4 years ago

ColinFay commented 4 years ago

Let's say I have this piece of code:

library(crrri)
client <- Chrome$new(bin = pagedown::find_chrome(), debug_port = 8888)$connect()
#> Running '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome' \
#>   --no-first-run --headless \
#>   '--user-data-dir=/Users/colin/Library/Application Support/r-crrri/chrome-data-dir-qibjymyz' \
#>   '--remote-debugging-port=8888'
Page <- client$Page
Page$navigate(url = "http://localhost:3000")
#> Error in eval(expr, envir, enclos): attempt to apply non-function

client$close()
#> Error in eval(expr, envir, enclos): attempt to apply non-function

Created on 2020-04-10 by the reprex package (v0.3.0)

AFAIK, here I'll never have access to the close() method from the Chrome generator. Would be nice to have access to the close method even if we never create the chrome object itself.

cderv commented 4 years ago

Just to be sure... In your example, you did not use a callback so client is a promise.

Page <- client$Page won't work. And there will not be any close element even if we add this feature.

client$close() won't work.

client <- Chrome$new(bin = pagedown::find_chrome(), debug_port = 8888)$connect()

Is this on purpose ? The expected usage is

  1. Open chrome process -> chrome object
  2. Connect to a tab -> client object

Mixing both in one step will indeed hide all the methods of the chrome object.

However, I see the advantage of adding a function that allow to close the opened process if this situation happens. I'll think of something... 🤔

cderv commented 4 years ago

Let's note also that the chrome process is supervised and there is a finalizer, so it should be closed when the object is garbage collected.

library(crrri)
chrome <- Chrome$new(bin = pagedown::find_chrome(), debug_port = httpuv::randomPort())
rm(chrome)
gc(TRUE)

Without creating the chrome object it does not work.

client <- Chrome$new(bin = pagedown::find_chrome(),
                     debug_port = httpuv::randomPort())$connect()
rm(client)
gc(TRUE)

Currently the processx object is only in the Chrome R6 class because this is the one corresponding to the chrome browser execution. It is not passed through other class because it does not seem related for me.

By not creating the chrome object your missing the methods made to control the opened browser.

What is your real need here ? Is it just being able to close the opened process in case of error because you don't have the chrome object ?