Skip to content

Commit bc50392

Browse files
authored
chore: throw on context.close() if it was closed externally (#1248)
This is a backport of microsoft/playwright@09ff7ea
1 parent 96d74ef commit bc50392

File tree

3 files changed

+20
-57
lines changed

3 files changed

+20
-57
lines changed

playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext {
4747
private final APIRequestContextImpl request;
4848
final List<PageImpl> pages = new ArrayList<>();
4949
final Router routes = new Router();
50-
private boolean isClosedOrClosing;
50+
private boolean closeWasCalled;
51+
private final WaitableEvent<EventType, ?> closePromise;
5152
final Map<String, BindingCallback> bindings = new HashMap<>();
5253
PageImpl ownerPage;
5354
private static final Map<EventType, String> eventSubscriptions() {
@@ -90,8 +91,9 @@ enum EventType {
9091
} else {
9192
browser = null;
9293
}
93-
this.tracing = connection.getExistingObject(initializer.getAsJsonObject("tracing").get("guid").getAsString());
94-
this.request = connection.getExistingObject(initializer.getAsJsonObject("requestContext").get("guid").getAsString());
94+
tracing = connection.getExistingObject(initializer.getAsJsonObject("tracing").get("guid").getAsString());
95+
request = connection.getExistingObject(initializer.getAsJsonObject("requestContext").get("guid").getAsString());
96+
closePromise = new WaitableEvent<>(listeners, EventType.CLOSE);
9597
}
9698

9799
void setRecordHar(Path path, HarContentPolicy policy) {
@@ -199,11 +201,8 @@ public List<Cookie> cookies(String url) {
199201
}
200202

201203
private void closeImpl() {
202-
if (isClosedOrClosing) {
203-
return;
204-
}
205-
isClosedOrClosing = true;
206-
try {
204+
if (!closeWasCalled) {
205+
closeWasCalled = true;
207206
for (Map.Entry<String, HarRecorder> entry : harRecorders.entrySet()) {
208207
JsonObject params = new JsonObject();
209208
params.addProperty("harId", entry.getKey());
@@ -225,13 +224,9 @@ private void closeImpl() {
225224
}
226225
artifact.delete();
227226
}
228-
229227
sendMessage("close");
230-
} catch (PlaywrightException e) {
231-
if (!isSafeCloseError(e)) {
232-
throw e;
233-
}
234228
}
229+
runUntil(() -> {}, closePromise);
235230
}
236231

237232
@Override
@@ -624,7 +619,6 @@ protected void handleEvent(String event, JsonObject params) {
624619
}
625620

626621
void didClose() {
627-
isClosedOrClosing = true;
628622
if (browser != null) {
629623
browser.contexts.remove(this);
630624
}

playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -511,19 +511,21 @@ private <T> T waitForEventWithTimeout(EventType eventType, Runnable code, Predic
511511

512512
@Override
513513
public void close(CloseOptions options) {
514-
if (isClosed) {
515-
return;
514+
if (options == null) {
515+
options = new CloseOptions();
516516
}
517-
JsonObject params = options == null ? new JsonObject() : gson().toJsonTree(options).getAsJsonObject();
518517
try {
519-
sendMessage("close", params);
518+
if (ownedContext != null) {
519+
ownedContext.close();
520+
} else {
521+
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
522+
sendMessage("close", params);
523+
}
520524
} catch (PlaywrightException exception) {
521-
if (!isSafeCloseError(exception)) {
522-
throw exception;
525+
if (isSafeCloseError(exception) && (options.runBeforeUnload == null || !options.runBeforeUnload)) {
526+
return;
523527
}
524-
}
525-
if (ownedContext != null) {
526-
ownedContext.close();
528+
throw exception;
527529
}
528530
}
529531

playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -349,40 +349,7 @@ void shouldNotThrowOnCloseAfterDisconnect() throws InterruptedException {
349349
}
350350
browser.close();
351351
}
352-
353-
@Test
354-
void shouldNotThrowOnContextCloseAfterDisconnect() throws InterruptedException {
355-
BrowserServer remoteServer = launchBrowserServer(browserType);
356-
Browser browser = browserType.connect(remoteServer.wsEndpoint);
357-
BrowserContext context = browser.newContext();
358-
Page page = context.newPage();
359-
360-
remoteServer.kill();
361-
while (browser.isConnected()) {
362-
try {
363-
page.waitForTimeout(10);
364-
} catch (PlaywrightException e) {
365-
}
366-
}
367-
context.close();
368-
}
369-
370-
@Test
371-
void shouldNotThrowOnPageCloseAfterDisconnect() throws InterruptedException {
372-
BrowserServer remoteServer = launchBrowserServer(browserType);
373-
Browser browser = browserType.connect(remoteServer.wsEndpoint);
374-
Page page = browser.newPage();
375-
376-
remoteServer.kill();
377-
while (browser.isConnected()) {
378-
try {
379-
page.waitForTimeout(10);
380-
} catch (PlaywrightException e) {
381-
}
382-
}
383-
page.close();
384-
}
385-
352+
386353
@Test
387354
void shouldSaveAsVideosFromRemoteBrowser(@TempDir Path tempDir) {
388355
Path videosPath = tempDir.resolve("videosPath");

0 commit comments

Comments
 (0)