Skip to content

Commit d7445b7

Browse files
committed
Fix issue with cleaning user data dir
1 parent 494f46b commit d7445b7

File tree

6 files changed

+210
-27
lines changed

6 files changed

+210
-27
lines changed

cdtp-definition-builder/pom.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
</dependencies>
118118

119119
<build>
120-
<finalName>${artifactId}</finalName>
120+
<finalName>${project.artifactId}</finalName>
121121
<plugins>
122122
<plugin>
123123
<groupId>com.coveo</groupId>
@@ -168,6 +168,7 @@
168168
<plugin>
169169
<groupId>org.jacoco</groupId>
170170
<artifactId>jacoco-maven-plugin</artifactId>
171+
<version>${jacoco.version}</version>
171172
<executions>
172173
<execution>
173174
<id>pre-unit-test</id>

cdtp-java-client/src/main/java/com/github/kklisura/cdtp/launch/ChromeArguments.java

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
*/
2222

2323
import com.github.kklisura.cdtp.launch.support.annotations.ChromeArgument;
24-
import java.io.IOException;
25-
import java.nio.file.Files;
2624
import java.util.HashMap;
2725
import java.util.Map;
2826

@@ -32,6 +30,8 @@
3230
* @author Kenan Klisura
3331
*/
3432
public class ChromeArguments {
33+
public static final String USER_DATA_DIR_ARGUMENT = "user-data-dir";
34+
3535
@ChromeArgument("headless")
3636
private Boolean headless;
3737

@@ -44,7 +44,7 @@ public class ChromeArguments {
4444
@ChromeArgument("no-first-run")
4545
private Boolean noFirstRun;
4646

47-
@ChromeArgument("user-data-dir")
47+
@ChromeArgument(USER_DATA_DIR_ARGUMENT)
4848
private String userDataDir;
4949

5050
@ChromeArgument("incognito")
@@ -331,8 +331,7 @@ public static Builder defaults(boolean headless) {
331331
.disableSync()
332332
.disableTranslate()
333333
.metricsRecordingOnly()
334-
.safebrowsingDisableAutoUpdate()
335-
.userDataDir(randomUserDataDir());
334+
.safebrowsingDisableAutoUpdate();
336335

337336
if (headless) {
338337
builder.headless().disableGpu().hideScrollbars().muteAudio();
@@ -341,20 +340,6 @@ public static Builder defaults(boolean headless) {
341340
return builder;
342341
}
343342

344-
/**
345-
* Returns a random user data dir.
346-
*
347-
* @return Random user data dir absolute path.
348-
* @throws RuntimeException If it fails to create temp directory.
349-
*/
350-
public static String randomUserDataDir() {
351-
try {
352-
return Files.createTempDirectory("cdtp-user-data-dir").toAbsolutePath().toString();
353-
} catch (IOException e) {
354-
throw new RuntimeException("Failed creating temp directory for user data dir.", e);
355-
}
356-
}
357-
358343
/** The type Builder. */
359344
public static class Builder {
360345
private ChromeArguments arguments;

cdtp-java-client/src/main/java/com/github/kklisura/cdtp/launch/ChromeLauncher.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
* #L%
2121
*/
2222

23+
import static com.github.kklisura.cdtp.launch.ChromeArguments.USER_DATA_DIR_ARGUMENT;
2324
import static com.github.kklisura.cdtp.utils.ChromeDevToolsUtils.closeQuietly;
25+
import static com.github.kklisura.cdtp.utils.FilesUtils.deleteQuietly;
26+
import static com.github.kklisura.cdtp.utils.FilesUtils.randomTempDir;
2427

2528
import com.github.kklisura.cdtp.launch.config.ChromeLauncherConfiguration;
2629
import com.github.kklisura.cdtp.launch.exceptions.ChromeProcessException;
@@ -60,6 +63,8 @@ public class ChromeLauncher implements AutoCloseable {
6063

6164
private static final Logger LOGGER = LoggerFactory.getLogger(ChromeLauncher.class);
6265

66+
private static final String TEMP_PREFIX = "cdtp-user-data-dir";
67+
6368
private static final Pattern DEVTOOLS_LISTENING_LINE_PATTERN =
6469
Pattern.compile("^DevTools listening on ws:\\/\\/.+?:(\\d+)\\/");
6570

@@ -86,6 +91,8 @@ public class ChromeLauncher implements AutoCloseable {
8691

8792
private ChromeLauncherConfiguration configuration;
8893

94+
private Path userDataDirPath;
95+
8996
/** Instantiates a new Chrome launcher. */
9097
public ChromeLauncher() {
9198
this(new ChromeLauncherConfiguration());
@@ -224,6 +231,8 @@ public void close() {
224231

225232
chromeProcess.destroyForcibly();
226233
chromeProcess = null;
234+
} finally {
235+
deleteQuietly(userDataDirPath);
227236
}
228237

229238
try {
@@ -254,13 +263,21 @@ private int launchChromeProcess(Path chromeBinary, ChromeArguments chromeArgumen
254263

255264
Map<String, Object> argumentsMap = getArguments(chromeArguments);
256265

266+
// Special case for user data directory.
267+
if (chromeArguments.getUserDataDir() == null) {
268+
String userDatDir = randomTempDir(TEMP_PREFIX);
269+
userDataDirPath = Paths.get(userDatDir);
270+
argumentsMap.put(USER_DATA_DIR_ARGUMENT, userDatDir);
271+
}
272+
257273
List<String> arguments = argsMapToArgsList(argumentsMap);
258274

259275
LOGGER.info(
260276
"Launching chrome process {} with arguments {}", chromeBinary.toString(), argumentsMap);
261277

262278
try {
263279
chromeProcess = processLauncher.launch(chromeBinary.toString(), arguments);
280+
264281
return waitForDevToolsServer(chromeProcess);
265282
} catch (IOException e) {
266283
// Unsubscribe from registry on exceptions.
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package com.github.kklisura.cdtp.utils;
2+
3+
/*-
4+
* #%L
5+
* cdpt-java-client
6+
* %%
7+
* Copyright (C) 2018 Kenan Klisura
8+
* %%
9+
* Licensed under the Apache License, Version 2.0 (the "License");
10+
* you may not use this file except in compliance with the License.
11+
* You may obtain a copy of the License at
12+
*
13+
* http://www.apache.org/licenses/LICENSE-2.0
14+
*
15+
* Unless required by applicable law or agreed to in writing, software
16+
* distributed under the License is distributed on an "AS IS" BASIS,
17+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
* See the License for the specific language governing permissions and
19+
* limitations under the License.
20+
* #L%
21+
*/
22+
23+
import java.io.File;
24+
import java.io.IOException;
25+
import java.nio.file.Files;
26+
import java.nio.file.Path;
27+
28+
/**
29+
* Files utils.
30+
*
31+
* @author Kenan Klisura
32+
*/
33+
public final class FilesUtils {
34+
/**
35+
* Deletes the files/directories on a given path.
36+
*
37+
* @param path Path to delete.
38+
*/
39+
public static void deleteQuietly(Path path) {
40+
if (path != null) {
41+
try {
42+
File file = path.toFile();
43+
if (file.isDirectory()) {
44+
File[] directoryFiles = file.listFiles();
45+
46+
if (directoryFiles != null) {
47+
for (File directoryFile : directoryFiles) {
48+
deleteQuietly(directoryFile.toPath());
49+
}
50+
}
51+
}
52+
53+
file.delete();
54+
} catch (Exception e) {
55+
// Ignore this exception.
56+
}
57+
}
58+
}
59+
60+
/**
61+
* Returns a random temp dir.
62+
*
63+
* @return Random temp data dir absolute path.
64+
* @throws RuntimeException If it fails to create temp directory.
65+
*/
66+
public static String randomTempDir(String prefix) {
67+
try {
68+
return Files.createTempDirectory(prefix).toAbsolutePath().toString();
69+
} catch (IOException e) {
70+
throw new RuntimeException("Failed creating temp directory for " + prefix, e);
71+
}
72+
}
73+
}

cdtp-java-client/src/test/java/com/github/kklisura/cdtp/launch/ChromeArgumentsTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private void assertDefaults(ChromeArguments args) {
117117
assertTrue(args.getSafebrowsingDisableAutoUpdate());
118118
assertTrue(args.getDisablePopupBlocking());
119119

120-
assertNotNull(args.getUserDataDir());
120+
assertNull(args.getUserDataDir());
121121

122122
assertEquals(0, args.getRemoteDebuggingPort().intValue());
123123
}

0 commit comments

Comments
 (0)