Skip to content

Commit

Permalink
feat: More eager cleanup for UIs using Beacon API (#16657) (#18128)
Browse files Browse the repository at this point in the history
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

Co-authored-by: Matti Tahvonen <matti@vaadin.com>
  • Loading branch information
tltv and mstahv authored Nov 27, 2023
1 parent dd98b96 commit b2bdaba
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ public void start(ValueMap initialUidl) {
registry.getRequestResponseTracker().startRequest();
registry.getMessageHandler().handleMessage(initialUidl);
}

Browser.getWindow().addEventListener("pagehide", e -> {
registry.getMessageSender().sendUnloadBeacon();
});

Browser.getWindow().addEventListener("pageshow", e -> {
// Currently only Safari gets here, sometimes when going back/foward
// with browser buttons
// Chrome discards our state as beforeunload is used
// As state is most likely cleared on the server already (especially
// now with Beacon API request, it is probably
// better resyncronize the state (would happen on first server
// visit)
Browser.getWindow().getLocation().reload();
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@
*/
public class MessageSender {

public void sendUnloadBeacon() {
JsonArray dummyEmptyJson = Json.createArray();
JsonObject extraJson = Json.createObject();
extraJson.put(ApplicationConstants.UNLOAD_BEACON, true);
JsonObject payload = preparePayload(dummyEmptyJson, extraJson);
sendBeacon(registry.getXhrConnection().getUri(), payload.toJson());
}

public static native void sendBeacon(String url, String payload) /*-{
$wnd.navigator.sendBeacon(url, payload);
}-*/;

public enum ResynchronizationState {
NOT_ACTIVE, SEND_TO_SERVER, WAITING_FOR_RESPONSE
}
Expand Down Expand Up @@ -135,7 +147,12 @@ private void doSendInvocationsToServer() {
protected void send(final JsonArray reqInvocations,
final JsonObject extraJson) {
registry.getRequestResponseTracker().startRequest();
send(preparePayload(reqInvocations, extraJson));

}

private JsonObject preparePayload(final JsonArray reqInvocations,
final JsonObject extraJson) {
JsonObject payload = Json.createObject();
String csrfToken = registry.getMessageHandler().getCsrfToken();
if (!csrfToken.equals(ApplicationConstants.CSRF_TOKEN_DEFAULT_VALUE)) {
Expand All @@ -146,16 +163,13 @@ protected void send(final JsonArray reqInvocations,
registry.getMessageHandler().getLastSeenServerSyncId());
payload.put(ApplicationConstants.CLIENT_TO_SERVER_ID,
clientToServerMessageId++);

if (extraJson != null) {
for (String key : extraJson.keys()) {
JsonValue value = extraJson.get(key);
payload.put(key, value);
}
}

send(payload);

return payload;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,14 @@ public void disconnect() {
try {
outgoingMessage.get(1000, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
getLogger().info(
"Timeout waiting for messages to be sent to client before disconnect",
e);
if (ui.isClosing()) {
getLogger().debug(
"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.");
} else {
getLogger().info(
"Timeout waiting for messages to be sent to client before disconnect",
e);
}
} catch (Exception e) {
getLogger().info(
"Error waiting for messages to be sent to client before disconnect",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.vaadin.flow.server.communication;

import java.io.IOException;
Expand All @@ -35,6 +34,7 @@

import com.vaadin.flow.component.UI;
import com.vaadin.flow.internal.MessageDigestUtil;
import com.vaadin.flow.router.PreserveOnRefresh;
import com.vaadin.flow.server.ErrorEvent;
import com.vaadin.flow.server.VaadinRequest;
import com.vaadin.flow.server.VaadinService;
Expand Down Expand Up @@ -189,6 +189,10 @@ public JsonObject getRawJson() {
return json;
}

private boolean isUnloadBeaconRequest() {
return json.hasKey(ApplicationConstants.UNLOAD_BEACON);
}

}

private static final int MAX_BUFFER_SIZE = 64 * 1024;
Expand Down Expand Up @@ -280,7 +284,6 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request)
// did not reach the client. When the client re-sends the message,
// it would only get an empty response (because the dirty flags have
// been cleared on the server) and would be out of sync

if (requestId == expectedId - 1 && Arrays.equals(messageHash,
ui.getInternals().getLastProcessedMessageHash())) {
/*
Expand Down Expand Up @@ -340,6 +343,24 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request)
// signature for source and binary compatibility
throw new ResynchronizationRequiredException();
}
if (rpcRequest.isUnloadBeaconRequest()) {
if (isPreserveOnRefreshTarget(ui)) {
getLogger().debug(
"Eager UI close ignored for @PreserveOnRefresh view");
} else {
ui.close();
getLogger().debug("UI closed with a beacon request");
}
}

}

// Kind of same as in AbstractNavigationStateRenderer, but gets
// "routeLayoutTypes" & class from UI instance.
private static boolean isPreserveOnRefreshTarget(UI ui) {
return ui.getInternals().getActiveRouterTargetsChain().stream()
.anyMatch(rt -> rt.getClass()
.isAnnotationPresent(PreserveOnRefresh.class));
}

private String getMessageDetails(RpcRequest rpcRequest) {
Expand Down Expand Up @@ -474,6 +495,7 @@ private static RpcInvocationHandler resolveHandlerConflicts(
}

private static class LazyInvocationHandlers {

private static final Map<String, RpcInvocationHandler> HANDLERS = loadHandlers()
.stream()
.collect(Collectors.toMap(RpcInvocationHandler::getRpcType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,10 @@ public class ApplicationConstants implements Serializable {
*/
public static final String DEV_TOOLS_ENABLED = "devToolsEnabled";

/**
* The name of the parameter used for notifying the server that user closed
* the tab/window or navigated away.
*/
public static final String UNLOAD_BEACON = "UNLOAD";

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2000-2023 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package com.vaadin.flow.uitest.ui;

import com.vaadin.flow.component.DetachEvent;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.router.Route;

@Route(value = "com.vaadin.flow.uitest.ui.UIsCollectedWithBeaconAPIView")
public class UIsCollectedWithBeaconAPIView extends Div {

static int viewcount = 0;

Div count = new Div();

public UIsCollectedWithBeaconAPIView() {
viewcount++;
add(count);
count.setId("uis");
NativeButton showUisNumber = new NativeButton("Update",
event -> updateCount());
add(showUisNumber);
updateCount();
}

private void updateCount() {
count.setText("" + viewcount);
}

@Override
protected void onDetach(DetachEvent detachEvent) {
super.onDetach(detachEvent);
viewcount--;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@
import static com.vaadin.flow.uitest.ui.LocaleChangeView.CHANGE_LOCALE_BUTTON_ID;
import static com.vaadin.flow.uitest.ui.LocaleChangeView.SAME_UI_RESULT_ID;
import static com.vaadin.flow.uitest.ui.LocaleChangeView.SHOW_RESULTS_BUTTON_ID;
import org.openqa.selenium.WindowType;

public class LocaleChangeIT extends ChromeBrowserTest {

@Test
public void setSessionLocale_currentUIInstanceUpdatedUponEachLocaleUpdate() {
final int openedUI = 3;

IntStream.range(0, openedUI).forEach(i -> open());
IntStream.range(0, openedUI).forEach(i -> {
driver.switchTo().newWindow(WindowType.TAB);
open();
});

waitForElementPresent(By.id(CHANGE_LOCALE_BUTTON_ID));
findElement(By.id(CHANGE_LOCALE_BUTTON_ID)).click();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright 2000-2023 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package com.vaadin.flow.uitest.ui;

import org.junit.Assert;
import org.junit.Test;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;

import com.vaadin.flow.testutil.ChromeBrowserTest;

public class UIsCollectedWithBeaconAPIIT extends ChromeBrowserTest {

@Test
public void beaconHandling_navigateAwayFromApplication_uiClosedEarly() {
openUiAndExpect1();
goToGoogle();
// If previous UI is not properly detached, following will fail
openUiAndExpect1();
}

private void openUiAndExpect1() throws NumberFormatException {
open();
WebElement uisCount = findElement(By.id("uis"));
int count = Integer.parseInt(uisCount.getText());
Assert.assertEquals(1, count);
}

private void goToGoogle() {
getDriver().get("https://google.com/");
Assert.assertTrue(getDriver().getPageSource().contains("Google"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void routeScopedBeanIsPreservedAfterViewRefresh() {
String beanCall = findElement(By.id("preserve-on-refresh")).getText();

// refresh
open();
getDriver().navigate().refresh();

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

0 comments on commit b2bdaba

Please sign in to comment.