Skip to content

Commit 0bad213

Browse files
shs96cshs96c
committed
FirefoxDriver: use a XpiDriverService
We effectively had this with the original way of doing Firefox start up with pre-marionette, but that way was created before DriverService was.
1 parent 1a8cdc6 commit 0bad213

File tree

4 files changed

+213
-122
lines changed

4 files changed

+213
-122
lines changed

‎java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java‎

Lines changed: 17 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.openqa.selenium.firefox;
1919

20+
import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.BROWSER_LOGFILE;
2021
import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.DRIVER_USE_MARIONETTE;
2122
import static org.openqa.selenium.firefox.FirefoxOptions.FIREFOX_OPTIONS;
2223
import static org.openqa.selenium.remote.CapabilityType.PROXY;
@@ -25,29 +26,19 @@
2526
import com.google.common.collect.Sets;
2627

2728
import org.openqa.selenium.Capabilities;
28-
import org.openqa.selenium.NoSuchSessionException;
2929
import org.openqa.selenium.Proxy;
3030
import org.openqa.selenium.WebDriverException;
31-
import org.openqa.selenium.firefox.internal.NewProfileExtensionConnection;
32-
import org.openqa.selenium.internal.Killable;
33-
import org.openqa.selenium.internal.Lock;
34-
import org.openqa.selenium.internal.SocketLock;
35-
import org.openqa.selenium.logging.LocalLogs;
36-
import org.openqa.selenium.logging.NeedsLocalLogs;
3731
import org.openqa.selenium.remote.BeanToJsonConverter;
38-
import org.openqa.selenium.remote.Command;
3932
import org.openqa.selenium.remote.CommandExecutor;
4033
import org.openqa.selenium.remote.DesiredCapabilities;
41-
import org.openqa.selenium.remote.DriverCommand;
4234
import org.openqa.selenium.remote.FileDetector;
4335
import org.openqa.selenium.remote.RemoteWebDriver;
44-
import org.openqa.selenium.remote.Response;
4536
import org.openqa.selenium.remote.service.DriverCommandExecutor;
37+
import org.openqa.selenium.remote.service.DriverService;
4638

39+
import java.io.File;
4740
import java.io.IOException;
48-
import java.net.URI;
4941
import java.util.Map;
50-
import java.util.Optional;
5142
import java.util.Set;
5243
import java.util.logging.Logger;
5344

@@ -63,7 +54,7 @@
6354
*WebDriver driver = new FirefoxDriver(options);
6455
* </pre>
6556
*/
66-
public class FirefoxDriver extends RemoteWebDriver implements Killable {
57+
public class FirefoxDriver extends RemoteWebDriver {
6758

6859
public static final class SystemProperty {
6960

@@ -233,17 +224,22 @@ private FirefoxDriver(
233224
}
234225

235226
private static CommandExecutor toExecutor(FirefoxOptions options) {
236-
if (options.isLegacy()) {
237-
return new FirefoxDriver.LazyCommandExecutor(options.getBinary(), options.getProfile());
227+
DriverService.Builder builder;
238228

229+
if (options.isLegacy()) {
230+
builder = XpiDriverService.builder()
231+
.withBinary(options.getBinaryOrNull().orElseGet(FirefoxBinary::new))
232+
.withProfile(options.getProfile());
239233
} else {
240-
GeckoDriverService.Builder builder = new GeckoDriverService.Builder().usingPort(0);
241-
Optional<FirefoxBinary> binary = options.getBinaryOrNull();
242-
if (binary.isPresent()) {
243-
builder.usingFirefoxBinary(binary.get());
244-
}
245-
return new DriverCommandExecutor(builder.build());
234+
builder = new GeckoDriverService.Builder()
235+
.usingFirefoxBinary(options.getBinaryOrNull().orElseGet(FirefoxBinary::new));
246236
}
237+
238+
if (System.getProperty(BROWSER_LOGFILE) != null) {
239+
builder.withLogFile(new File(System.getProperty(System.getProperty(BROWSER_LOGFILE))));
240+
}
241+
242+
return new DriverCommandExecutor(builder.build());
247243
}
248244

249245
private static FirefoxOptions getFirefoxOptions(Capabilities capabilities) {
@@ -295,16 +291,6 @@ public void setFileDetector(FileDetector detector) {
295291
"via RemoteWebDriver");
296292
}
297293

298-
/**
299-
* Attempt to forcibly kill this Killable at the OS level. Useful where the extension has
300-
* stopped responding, and you don't want to leak resources. Should not ordinarily be called.
301-
*/
302-
public void kill() {
303-
if (this.getCommandExecutor() instanceof LazyCommandExecutor) {
304-
((LazyCommandExecutor) this.getCommandExecutor()).binary.quit();
305-
}
306-
}
307-
308294
private static boolean isLegacy(Capabilities desiredCapabilities) {
309295
Boolean forceMarionette = forceMarionetteFromSystemProperty();
310296
if (forceMarionette != null) {
@@ -322,46 +308,6 @@ private static Boolean forceMarionetteFromSystemProperty() {
322308
return Boolean.valueOf(useMarionette);
323309
}
324310

325-
@Override
326-
protected void startClient(Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
327-
if (isLegacy(desiredCapabilities)) {
328-
LazyCommandExecutor exe = (LazyCommandExecutor) getCommandExecutor();
329-
330-
// TODO(simon): Make this not sinfully ugly
331-
ExtensionConnection connection = connectTo(exe.binary, exe.profile, "localhost");
332-
exe.setConnection(connection);
333-
334-
try {
335-
connection.start();
336-
} catch (IOException e) {
337-
throw new WebDriverException("An error occurred while connecting to Firefox", e);
338-
}
339-
340-
}
341-
}
342-
343-
protected ExtensionConnection connectTo(FirefoxBinary binary, FirefoxProfile profile,
344-
String host) {
345-
Lock lock = obtainLock(profile);
346-
try {
347-
FirefoxBinary bin = binary == null ? new FirefoxBinary() : binary;
348-
return new NewProfileExtensionConnection(lock, bin, profile, host);
349-
} catch (Exception e) {
350-
throw new WebDriverException(e);
351-
}
352-
}
353-
354-
protected Lock obtainLock(FirefoxProfile profile) {
355-
return new SocketLock();
356-
}
357-
358-
@Override
359-
protected void stopClient() {
360-
if (this.getCommandExecutor() instanceof LazyCommandExecutor) {
361-
((LazyCommandExecutor) this.getCommandExecutor()).quit();
362-
}
363-
}
364-
365311
/**
366312
* Drops capabilities that we shouldn't send over the wire.
367313
*
@@ -391,53 +337,4 @@ private static Capabilities dropCapabilities(Capabilities capabilities) {
391337

392338
return caps;
393339
}
394-
395-
public static class LazyCommandExecutor implements CommandExecutor, NeedsLocalLogs {
396-
private ExtensionConnection connection;
397-
private final FirefoxBinary binary;
398-
private final FirefoxProfile profile;
399-
private LocalLogs logs = LocalLogs.getNullLogger();
400-
401-
LazyCommandExecutor(FirefoxBinary binary, FirefoxProfile profile) {
402-
this.binary = binary;
403-
this.profile = profile;
404-
}
405-
406-
public void setConnection(ExtensionConnection connection) {
407-
this.connection = connection;
408-
connection.setLocalLogs(logs);
409-
}
410-
411-
public void quit() {
412-
if (connection != null) {
413-
connection.quit();
414-
connection = null;
415-
}
416-
if (profile != null) {
417-
profile.cleanTemporaryModel();
418-
}
419-
}
420-
421-
public Response execute(Command command) throws IOException {
422-
if (connection == null) {
423-
if (command.getName().equals(DriverCommand.QUIT)) {
424-
return new Response();
425-
}
426-
throw new NoSuchSessionException(
427-
"The FirefoxDriver cannot be used after quit() was called.");
428-
}
429-
return connection.execute(command);
430-
}
431-
432-
public void setLocalLogs(LocalLogs logs) {
433-
this.logs = logs;
434-
if (connection != null) {
435-
connection.setLocalLogs(logs);
436-
}
437-
}
438-
439-
public URI getAddressOfRemoteServer() {
440-
return connection.getAddressOfRemoteServer();
441-
}
442-
}
443340
}

‎java/client/src/org/openqa/selenium/firefox/XpiDriverService.java‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.firefox;
19+
20+
21+
import static org.openqa.selenium.firefox.FirefoxProfile.PORT_PREFERENCE;
22+
23+
import com.google.common.base.Preconditions;
24+
import com.google.common.collect.ImmutableList;
25+
import com.google.common.collect.ImmutableMap;
26+
27+
import org.openqa.selenium.WebDriverException;
28+
import org.openqa.selenium.firefox.internal.ClasspathExtension;
29+
import org.openqa.selenium.firefox.internal.Extension;
30+
import org.openqa.selenium.firefox.internal.FileExtension;
31+
import org.openqa.selenium.remote.service.DriverService;
32+
33+
import java.io.File;
34+
import java.io.IOException;
35+
import java.net.URL;
36+
import java.util.Optional;
37+
import java.util.concurrent.locks.Lock;
38+
import java.util.concurrent.locks.ReentrantLock;
39+
40+
public class XpiDriverService extends DriverService {
41+
42+
private final Lock lock = new ReentrantLock();
43+
44+
private final int port;
45+
private final FirefoxBinary binary;
46+
private final FirefoxProfile profile;
47+
private File profileDir;
48+
49+
private XpiDriverService(
50+
File executable,
51+
int port,
52+
ImmutableList<String> args,
53+
ImmutableMap<String, String> environment,
54+
FirefoxBinary binary,
55+
FirefoxProfile profile)
56+
throws IOException {
57+
super(executable, port, args, environment);
58+
59+
Preconditions.checkState(port > 0, "Port must be set");
60+
61+
this.port = port;
62+
this.binary = binary;
63+
this.profile = profile;
64+
}
65+
66+
@Override
67+
protected URL getUrl(int port) throws IOException {
68+
return new URL("http", "localhost", port, "/hub");
69+
}
70+
71+
@Override
72+
public void start() throws IOException {
73+
lock.lock();
74+
try {
75+
profile.setPreference(PORT_PREFERENCE, port);
76+
addWebDriverExtension(profile);
77+
78+
profileDir = profile.layoutOnDisk();
79+
binary.startProfile(profile, profileDir, "-foreground");
80+
81+
waitUntilAvailable();
82+
} finally {
83+
lock.unlock();
84+
}
85+
}
86+
87+
@Override
88+
public void stop() {
89+
lock.lock();
90+
try {
91+
binary.quit();
92+
profile.cleanTemporaryModel();
93+
profile.clean(profileDir);
94+
} finally {
95+
lock.unlock();
96+
}
97+
}
98+
99+
private void addWebDriverExtension(FirefoxProfile profile) {
100+
if (profile.containsWebDriverExtension()) {
101+
return;
102+
}
103+
profile.addExtension("webdriver", loadCustomExtension().orElse(loadDefaultExtension()));
104+
}
105+
106+
private Optional<Extension> loadCustomExtension() {
107+
String xpiProperty = System.getProperty(FirefoxDriver.SystemProperty.DRIVER_XPI_PROPERTY);
108+
if (xpiProperty != null) {
109+
File xpi = new File(xpiProperty);
110+
return Optional.of(new FileExtension(xpi));
111+
}
112+
return Optional.empty();
113+
}
114+
115+
private static Extension loadDefaultExtension() {
116+
return new ClasspathExtension(
117+
FirefoxProfile.class,
118+
"/" + FirefoxProfile.class.getPackage().getName().replace(".", "/") + "/webdriver.xpi");
119+
}
120+
121+
/**
122+
* Configures and returns a new {@link XpiDriverService} using the default configuration. In
123+
* this configuration, the service will use the firefox executable identified by the
124+
* {@link FirefoxDriver.SystemProperty#BROWSER_BINARY} system property on a free port.
125+
*
126+
* @return A new XpiDriverService using the default configuration.
127+
*/
128+
public static XpiDriverService createDefaultService() {
129+
try {
130+
return new XpiDriverService.Builder().usingAnyFreePort().build();
131+
} catch (WebDriverException e) {
132+
throw new IllegalStateException(e.getMessage(), e.getCause());
133+
}
134+
}
135+
136+
public static Builder builder() {
137+
return new Builder();
138+
}
139+
140+
public static class Builder extends DriverService.Builder<XpiDriverService, XpiDriverService.Builder> {
141+
142+
private FirefoxBinary binary = null;
143+
private FirefoxProfile profile = null;
144+
145+
private Builder() {
146+
// Only available through the static factory method in the XpiDriverService
147+
}
148+
149+
public Builder withBinary(FirefoxBinary binary) {
150+
this.binary = Preconditions.checkNotNull(binary);
151+
return this;
152+
}
153+
154+
public Builder withProfile(FirefoxProfile profile) {
155+
this.profile = Preconditions.checkNotNull(profile);
156+
return this;
157+
}
158+
159+
@Override
160+
protected File findDefaultExecutable() {
161+
return new FirefoxBinary().getFile();
162+
}
163+
164+
@Override
165+
protected ImmutableList<String> createArgs() {
166+
return ImmutableList.of("-foreground");
167+
}
168+
169+
@Override
170+
protected XpiDriverService createDriverService(
171+
File exe,
172+
int port,
173+
ImmutableList<String> args,
174+
ImmutableMap<String, String> environment) {
175+
try {
176+
return new XpiDriverService(
177+
exe,
178+
port,
179+
args,
180+
environment,
181+
binary == null ? new FirefoxBinary() : binary,
182+
profile == null ? new FirefoxProfile() : profile);
183+
} catch (IOException e) {
184+
throw new WebDriverException(e);
185+
}
186+
}
187+
}
188+
}

0 commit comments

Comments
 (0)