Skip to content

Commit d08549a

Browse files
authored
feat: More eager cleanup for UIs using Beacon API (#16657)
Notifies server about closed UIs using a beacon request on pagehide event. At the same time unifies behaviour in certain edge cases, where Safari maintained the state when brought back from page cache. Previously Safari in some situations kept the state. Tested manually with Safari, Chrome, Firefox and validated results using VisualVM. Closes #6293
1 parent 4e20867 commit d08549a

File tree

9 files changed

+174
-11
lines changed

9 files changed

+174
-11
lines changed

flow-client/src/main/java/com/vaadin/client/ApplicationConnection.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,21 @@ public void start(ValueMap initialUidl) {
131131
registry.getRequestResponseTracker().startRequest();
132132
registry.getMessageHandler().handleMessage(initialUidl);
133133
}
134+
135+
Browser.getWindow().addEventListener("pagehide", e -> {
136+
registry.getMessageSender().sendUnloadBeacon();
137+
});
138+
139+
Browser.getWindow().addEventListener("pageshow", e -> {
140+
// Currently only Safari gets here, sometimes when going back/foward
141+
// with browser buttons
142+
// Chrome discards our state as beforeunload is used
143+
// As state is most likely cleared on the server already (especially
144+
// now with Beacon API request, it is probably
145+
// better resyncronize the state (would happen on first server
146+
// visit)
147+
Browser.getWindow().getLocation().reload();
148+
});
134149
}
135150

136151
/**

flow-client/src/main/java/com/vaadin/client/communication/MessageSender.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@
3737
*/
3838
public class MessageSender {
3939

40+
public void sendUnloadBeacon() {
41+
JsonArray dummyEmptyJson = Json.createArray();
42+
JsonObject extraJson = Json.createObject();
43+
extraJson.put(ApplicationConstants.UNLOAD_BEACON, true);
44+
JsonObject payload = preparePayload(dummyEmptyJson, extraJson);
45+
sendBeacon(registry.getXhrConnection().getUri(), payload.toJson());
46+
}
47+
48+
public static native void sendBeacon(String url, String payload) /*-{
49+
$wnd.navigator.sendBeacon(url, payload);
50+
}-*/;
51+
4052
public enum ResynchronizationState {
4153
NOT_ACTIVE, SEND_TO_SERVER, WAITING_FOR_RESPONSE
4254
}
@@ -135,7 +147,12 @@ private void doSendInvocationsToServer() {
135147
protected void send(final JsonArray reqInvocations,
136148
final JsonObject extraJson) {
137149
registry.getRequestResponseTracker().startRequest();
150+
send(preparePayload(reqInvocations, extraJson));
151+
152+
}
138153

154+
private JsonObject preparePayload(final JsonArray reqInvocations,
155+
final JsonObject extraJson) {
139156
JsonObject payload = Json.createObject();
140157
String csrfToken = registry.getMessageHandler().getCsrfToken();
141158
if (!csrfToken.equals(ApplicationConstants.CSRF_TOKEN_DEFAULT_VALUE)) {
@@ -146,16 +163,13 @@ protected void send(final JsonArray reqInvocations,
146163
registry.getMessageHandler().getLastSeenServerSyncId());
147164
payload.put(ApplicationConstants.CLIENT_TO_SERVER_ID,
148165
clientToServerMessageId++);
149-
150166
if (extraJson != null) {
151167
for (String key : extraJson.keys()) {
152168
JsonValue value = extraJson.get(key);
153169
payload.put(key, value);
154170
}
155171
}
156-
157-
send(payload);
158-
172+
return payload;
159173
}
160174

161175
/**

flow-server/src/main/java/com/vaadin/flow/server/communication/AtmospherePushConnection.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,14 @@ public void disconnect() {
349349
try {
350350
outgoingMessage.get(1000, TimeUnit.MILLISECONDS);
351351
} catch (TimeoutException e) {
352-
getLogger().info(
353-
"Timeout waiting for messages to be sent to client before disconnect",
354-
e);
352+
if (ui.isClosing()) {
353+
getLogger().debug(
354+
"Something was not sent to client on an UI that was already closed by beacon request or similar. This seems to happen with Safari occassionally when navigating away from a UI.");
355+
} else {
356+
getLogger().info(
357+
"Timeout waiting for messages to be sent to client before disconnect",
358+
e);
359+
}
355360
} catch (Exception e) {
356361
getLogger().info(
357362
"Error waiting for messages to be sent to client before disconnect",

flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* License for the specific language governing permissions and limitations under
1414
* the License.
1515
*/
16-
1716
package com.vaadin.flow.server.communication;
1817

1918
import java.io.IOException;
@@ -35,6 +34,7 @@
3534

3635
import com.vaadin.flow.component.UI;
3736
import com.vaadin.flow.internal.MessageDigestUtil;
37+
import com.vaadin.flow.router.PreserveOnRefresh;
3838
import com.vaadin.flow.server.ErrorEvent;
3939
import com.vaadin.flow.server.VaadinRequest;
4040
import com.vaadin.flow.server.VaadinService;
@@ -189,6 +189,10 @@ public JsonObject getRawJson() {
189189
return json;
190190
}
191191

192+
private boolean isUnloadBeaconRequest() {
193+
return json.hasKey(ApplicationConstants.UNLOAD_BEACON);
194+
}
195+
192196
}
193197

194198
private static final int MAX_BUFFER_SIZE = 64 * 1024;
@@ -280,7 +284,6 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request)
280284
// did not reach the client. When the client re-sends the message,
281285
// it would only get an empty response (because the dirty flags have
282286
// been cleared on the server) and would be out of sync
283-
284287
if (requestId == expectedId - 1 && Arrays.equals(messageHash,
285288
ui.getInternals().getLastProcessedMessageHash())) {
286289
/*
@@ -340,6 +343,24 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request)
340343
// signature for source and binary compatibility
341344
throw new ResynchronizationRequiredException();
342345
}
346+
if (rpcRequest.isUnloadBeaconRequest()) {
347+
if (isPreserveOnRefreshTarget(ui)) {
348+
getLogger().debug(
349+
"Eager UI close ignored for @PreserveOnRefresh view");
350+
} else {
351+
ui.close();
352+
getLogger().debug("UI closed with a beacon request");
353+
}
354+
}
355+
356+
}
357+
358+
// Kind of same as in AbstractNavigationStateRenderer, but gets
359+
// "routeLayoutTypes" & class from UI instance.
360+
private static boolean isPreserveOnRefreshTarget(UI ui) {
361+
return ui.getInternals().getActiveRouterTargetsChain().stream()
362+
.anyMatch(rt -> rt.getClass()
363+
.isAnnotationPresent(PreserveOnRefresh.class));
343364
}
344365

345366
private String getMessageDetails(RpcRequest rpcRequest) {
@@ -474,6 +495,7 @@ private static RpcInvocationHandler resolveHandlerConflicts(
474495
}
475496

476497
private static class LazyInvocationHandlers {
498+
477499
private static final Map<String, RpcInvocationHandler> HANDLERS = loadHandlers()
478500
.stream()
479501
.collect(Collectors.toMap(RpcInvocationHandler::getRpcType,

flow-server/src/main/java/com/vaadin/flow/shared/ApplicationConstants.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,4 +214,10 @@ public class ApplicationConstants implements Serializable {
214214
*/
215215
public static final String DEV_TOOLS_ENABLED = "devToolsEnabled";
216216

217+
/**
218+
* The name of the parameter used for notifying the server that user closed
219+
* the tab/window or navigated away.
220+
*/
221+
public static final String UNLOAD_BEACON = "UNLOAD";
222+
217223
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2000-2023 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
package com.vaadin.flow.uitest.ui;
17+
18+
import com.vaadin.flow.component.DetachEvent;
19+
import com.vaadin.flow.component.html.Div;
20+
import com.vaadin.flow.component.html.NativeButton;
21+
import com.vaadin.flow.router.Route;
22+
23+
@Route(value = "com.vaadin.flow.uitest.ui.UIsCollectedWithBeaconAPIView")
24+
public class UIsCollectedWithBeaconAPIView extends Div {
25+
26+
static int viewcount = 0;
27+
28+
Div count = new Div();
29+
30+
public UIsCollectedWithBeaconAPIView() {
31+
viewcount++;
32+
add(count);
33+
count.setId("uis");
34+
NativeButton showUisNumber = new NativeButton("Update",
35+
event -> updateCount());
36+
add(showUisNumber);
37+
updateCount();
38+
}
39+
40+
private void updateCount() {
41+
count.setText("" + viewcount);
42+
}
43+
44+
@Override
45+
protected void onDetach(DetachEvent detachEvent) {
46+
super.onDetach(detachEvent);
47+
viewcount--;
48+
}
49+
50+
}

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/LocaleChangeIT.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,18 @@
2727
import static com.vaadin.flow.uitest.ui.LocaleChangeView.CHANGE_LOCALE_BUTTON_ID;
2828
import static com.vaadin.flow.uitest.ui.LocaleChangeView.SAME_UI_RESULT_ID;
2929
import static com.vaadin.flow.uitest.ui.LocaleChangeView.SHOW_RESULTS_BUTTON_ID;
30+
import org.openqa.selenium.WindowType;
3031

3132
public class LocaleChangeIT extends ChromeBrowserTest {
3233

3334
@Test
3435
public void setSessionLocale_currentUIInstanceUpdatedUponEachLocaleUpdate() {
3536
final int openedUI = 3;
3637

37-
IntStream.range(0, openedUI).forEach(i -> open());
38+
IntStream.range(0, openedUI).forEach(i -> {
39+
driver.switchTo().newWindow(WindowType.TAB);
40+
open();
41+
});
3842

3943
waitForElementPresent(By.id(CHANGE_LOCALE_BUTTON_ID));
4044
findElement(By.id(CHANGE_LOCALE_BUTTON_ID)).click();
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2000-2023 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
package com.vaadin.flow.uitest.ui;
17+
18+
import org.junit.Assert;
19+
import org.junit.Test;
20+
import org.openqa.selenium.By;
21+
import org.openqa.selenium.WebElement;
22+
23+
import com.vaadin.flow.testutil.ChromeBrowserTest;
24+
25+
public class UIsCollectedWithBeaconAPIIT extends ChromeBrowserTest {
26+
27+
@Test
28+
public void beaconHandling_navigateAwayFromApplication_uiClosedEarly() {
29+
openUiAndExpect1();
30+
goToGoogle();
31+
// If previous UI is not properly detached, following will fail
32+
openUiAndExpect1();
33+
}
34+
35+
private void openUiAndExpect1() throws NumberFormatException {
36+
open();
37+
WebElement uisCount = findElement(By.id("uis"));
38+
int count = Integer.parseInt(uisCount.getText());
39+
Assert.assertEquals(1, count);
40+
}
41+
42+
private void goToGoogle() {
43+
getDriver().get("https://google.com/");
44+
Assert.assertTrue(getDriver().getPageSource().contains("Google"));
45+
}
46+
47+
}

flow-tests/vaadin-spring-tests/test-spring-common/src/test/java/com/vaadin/flow/spring/test/routescope/PreserveOnRefreshIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public void routeScopedBeanIsPreservedAfterViewRefresh() {
3535
String beanCall = findElement(By.id("preserve-on-refresh")).getText();
3636

3737
// refresh
38-
open();
38+
getDriver().navigate().refresh();
3939

4040
Assert.assertEquals("Bean is not preserved after refresh", beanCall,
4141
findElement(By.id("preserve-on-refresh")).getText());

0 commit comments

Comments
 (0)