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

[JS] kill chromium service on quit #10796

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 8 additions & 33 deletions javascript/node/selenium-webdriver/chrome.js
Expand Up @@ -140,7 +140,6 @@ const CHROMEDRIVER_EXE =
process.platform === 'win32' ? 'chromedriver.exe' : 'chromedriver'

/** @type {remote.DriverService} */
let defaultService = null

/**
* Creates {@link selenium-webdriver/remote.DriverService} instances that manage
Expand Down Expand Up @@ -240,6 +239,14 @@ class Driver extends chromium.Driver {
super.createSession(caps, opt_serviceExecutor)
)
}

/**
* returns new instance chrome driver service
* @returns {remote.DriverService}
*/
static getDefaultService() {
return new ServiceBuilder().build()
}
}

/**
Expand All @@ -252,46 +259,14 @@ function locateSynchronously() {
return io.findInPath(CHROMEDRIVER_EXE, true)
}

/**
* Sets the default service to use for new ChromeDriver instances.
* @param {!remote.DriverService} service The service to use.
* @throws {Error} If the default service is currently running.
*/
function setDefaultService(service) {
if (defaultService && defaultService.isRunning()) {
throw Error(
`The previously configured ChromeDriver service is still running. ` +
`You must shut it down before you may adjust its configuration.`
)
}
defaultService = service
}

/**
* Returns the default ChromeDriver service. If such a service has not been
* configured, one will be constructed using the default configuration for
* a ChromeDriver executable found on the system PATH.
* @return {!remote.DriverService} The default ChromeDriver service.
*/
function getDefaultService() {
if (!defaultService) {
defaultService = new ServiceBuilder().build()
}
return defaultService
}

Options.prototype.CAPABILITY_KEY = 'goog:chromeOptions'
Options.prototype.BROWSER_NAME_VALUE = Browser.CHROME
Driver.getDefaultService = getDefaultService
Driver.prototype.VENDOR_COMMAND_PREFIX = 'goog'

// PUBLIC API

module.exports = {
Driver: Driver,
Options,
ServiceBuilder,
getDefaultService,
setDefaultService,
locateSynchronously,
}
4 changes: 3 additions & 1 deletion javascript/node/selenium-webdriver/chromium.js
Expand Up @@ -661,11 +661,13 @@ class Driver extends webdriver.WebDriver {
*/
static createSession(caps, opt_serviceExecutor) {
let executor
let onQuit
if (opt_serviceExecutor instanceof http.Executor) {
executor = opt_serviceExecutor
configureExecutor(executor, this.VENDOR_COMMAND_PREFIX)
} else {
let service = opt_serviceExecutor || this.getDefaultService()
onQuit = () => service.kill()
executor = createExecutor(service.start(), this.VENDOR_COMMAND_PREFIX)
}

Expand All @@ -679,7 +681,7 @@ class Driver extends webdriver.WebDriver {
}
}

return /** @type {!Driver} */ (super.createSession(executor, caps))
return /** @type {!Driver} */ (super.createSession(executor, caps, onQuit))
}

/**
Expand Down
40 changes: 8 additions & 32 deletions javascript/node/selenium-webdriver/edge.js
Expand Up @@ -90,7 +90,6 @@ const EDGEDRIVER_CHROMIUM_EXE =
process.platform === 'win32' ? 'msedgedriver.exe' : 'msedgedriver'

/** @type {remote.DriverService} */
let defaultService = null

/**
* Creates {@link selenium-webdriver/remote.DriverService} instances that manage
Expand Down Expand Up @@ -157,6 +156,14 @@ class Driver extends chromium.Driver {
)
}

/**
* returns new instance of edge driver service
* @returns {remote.DriverService}
*/
static getDefaultService() {
return new ServiceBuilder().build()
}

/**
* This function is a no-op as file detectors are not supported by this
* implementation.
Expand All @@ -165,34 +172,6 @@ class Driver extends chromium.Driver {
setFileDetector() {}
}

/**
* Sets the default service to use for new Edge instances.
* @param {!remote.DriverService} service The service to use.
* @throws {Error} If the default service is currently running.
*/
function setDefaultService(service) {
if (defaultService && defaultService.isRunning()) {
throw Error(
'The previously configured EdgeDriver service is still running. ' +
'You must shut it down before you may adjust its configuration.'
)
}
defaultService = service
}

/**
* Returns the default Microsoft Edge driver service. If such a service has
* not been configured, one will be constructed using the default configuration
* for a MicrosoftWebDriver executable found on the system PATH.
* @return {!remote.DriverService} The default Microsoft Edge driver service.
*/
function getDefaultService() {
if (!defaultService) {
defaultService = new ServiceBuilder().build()
}
return defaultService
}

/**
* _Synchronously_ attempts to locate the chromedriver executable on the current
* system.
Expand All @@ -206,15 +185,12 @@ function locateSynchronously() {
Options.prototype.BROWSER_NAME_VALUE = Browser.EDGE
Options.prototype.CAPABILITY_KEY = 'ms:edgeOptions'
Driver.prototype.VENDOR_CAPABILITY_PREFIX = 'ms'
Driver.getDefaultService = getDefaultService

// PUBLIC API

module.exports = {
Driver,
Options,
ServiceBuilder,
getDefaultService,
setDefaultService,
locateSynchronously,
}