Skip to content
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
39 changes: 39 additions & 0 deletions core/src/commonMain/kotlin/dev/kdriver/core/browser/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,45 @@ class Config(
return args
}

/**
* Returns an independent copy of this configuration.
*
* [DefaultBrowser.start] resolves and stores runtime state on the config it is given — it writes
* [host] and [port], and appends `--load-extension` / `--lang` arguments. Handing the same
* instance to a second browser would therefore make that browser take the `connectExisting`
* branch: it would never launch a browser process at all, and would instead try to talk to the
* previous one's now-dead port. [DefaultBrowser] copies its config on construction so that
* callers can safely reuse (and retry with) the same [Config] instance.
*
* The copy is deep where it matters: the argument and extension lists are duplicated, so
* mutating one config never affects the other.
*/
fun copy(): Config = Config(
// Pass the already-resolved executable so the copy does not re-run the disk search.
browserExecutablePath = browserExecutablePath,
headless = headless,
userAgent = userAgent,
// _browserArgs, not the browserArgs getter: the getter prepends defaultBrowserArgs, which
// would end up duplicated into the copy's own arg list.
browserArgs = _browserArgs.toList(),
sandbox = sandbox,
lang = lang,
host = host,
port = port,
expert = expert,
browserConnectionTimeout = browserConnectionTimeout,
browserConnectionMaxTries = browserConnectionMaxTries,
commandTimeout = commandTimeout,
autoDiscoverTargets = autoDiscoverTargets,
debugStringLimit = debugStringLimit,
).also { copy ->
// Copy the backing field, never the userDataDir getter: reading it would materialise a
// temporary profile directory on a config that may never be started.
copy._userDataDir = _userDataDir
copy._customDataDir = _customDataDir
copy._extensions.addAll(_extensions)
}

fun addArgument(arg: String) {
val forbiddenArgs = listOf("headless", "data-dir", "data_dir", "no-sandbox", "no_sandbox", "lang")
if (forbiddenArgs.any { arg.contains(it, ignoreCase = true) }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,20 @@ import kotlin.time.Duration.Companion.seconds
*/
open class DefaultBrowser(
val coroutineScope: CoroutineScope,
override val config: Config,
config: Config,
) : Browser {

/**
* This browser's own copy of the configuration it was created with.
*
* [start] resolves runtime state onto it — it assigns [Config.host] / [Config.port] and appends
* launch arguments — so it must not be the caller's instance. Mutating a shared config would
* make any later browser built from it take the `connectExisting` branch of [start]: it would
* skip launching a browser process entirely and try to reach the previous port, which is dead.
* Copying here makes reusing (and retrying with) the same [Config] safe by construction.
*/
override val config: Config = config.copy()

private val logger = KtorSimpleLogger("Browser")
private val updateTargetInfoMutex = Mutex()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import kotlinx.coroutines.runBlocking
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNotSame
import kotlin.test.assertNull
import kotlin.test.assertTrue

class ConfigDslTest {
Expand All @@ -21,7 +23,13 @@ class ConfigDslTest {
val tab = browser.get("https://example.com")

assertTrue(cfg.headless)
assertEquals(cfg, browser.config)
// The browser works on its own copy, so starting it leaves the caller's config untouched
// and the same instance can be reused for another browser.
assertNotSame(cfg, browser.config)
assertEquals(cfg.headless, browser.config.headless)
assertEquals(cfg.sandbox, browser.config.sandbox)
assertNull(cfg.port)
assertNotNull(browser.config.port)
assertNotNull(tab.getContent())

browser.stop()
Expand Down
39 changes: 39 additions & 0 deletions core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,49 @@ import kotlinx.io.files.Path
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertNull
import kotlin.test.assertTrue

class ConfigTest {

@Test
fun testCopyIsIndependent() {
val original = Config(browserExecutablePath = Path("/usr/bin/chromium"))
original.addArgument("--original-only")

val copy = original.copy()
copy.addArgument("--copy-only")
copy.host = "127.0.0.1"
copy.port = 1234

assertTrue(copy.browserArgs.contains("--original-only"))
assertTrue(copy.browserArgs.contains("--copy-only"))
assertFalse(original.browserArgs.contains("--copy-only"))
assertNull(original.host)
assertNull(original.port)
}

@Test
fun testCopyDoesNotDuplicateDefaultArguments() {
val original = Config(browserExecutablePath = Path("/usr/bin/chromium"))

val args = original.copy().copy().browserArgs

assertEquals(args.distinct().size, args.size)
}

@Test
fun testCopyKeepsCustomUserDataDirWithoutMaterialisingATemporaryOne() {
val custom = Config(browserExecutablePath = Path("/usr/bin/chromium"))
custom.userDataDir = Path("/tmp/kdriver-test-profile")
assertEquals(custom.userDataDir, custom.copy().userDataDir)

// A config that was never given one must not have a temp dir created for it by the copy.
val default = Config(browserExecutablePath = Path("/usr/bin/chromium"))
assertFalse(default.copy().usesCustomDataDir)
}

@Test
fun testAddArgument() {
val config = Config(browserExecutablePath = Path("/usr/bin/chromium"))
Expand Down
Loading