diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Config.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Config.kt index 0b24b3929..62190705f 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Config.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Config.kt @@ -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) }) { diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index a922652b4..64a3fff25 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -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() diff --git a/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigDslTest.kt b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigDslTest.kt index bbb06e237..748fa9fe6 100644 --- a/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigDslTest.kt +++ b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigDslTest.kt @@ -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 { @@ -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() diff --git a/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigTest.kt b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigTest.kt index dee8604b3..87b7a4a19 100644 --- a/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigTest.kt +++ b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/ConfigTest.kt @@ -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"))