feat: More eager cleanup for UIs using Beacon API #16657
Conversation
mshabarov
added
the
Contribution
Safari sometimes causes exception that is reported to the log, although behavior is not critical.
|
Included IT only runs in Chrome 🤔 Might be good to get it run also on Chrome and Safari + permutation with Push on, but some team member can probably configure that about 10k time faster than me 👼 |
|
Looks like Push is on by default for these tests, I think that is enough as the xhr mode has much less moving parts. Also heard that in "platform tests" these tests are run on other browsers, so all should be covered. |
addin some git ignores might help 🤓
mstahv
force-pushed
the
feature/eager-ui-cleanup-with-beacon-api
branch
from
a0920c8 to
3781c28
Compare
|
Gave another look to failing preserveonrefresh tests in spring-test modules. Looks like my separate test project didn't have its client side updated and it was sending its beacon request in a different event that I used in early prototypes. Or something else weird that actually made preserveonrefresh work. Now disabled the check for those views in 3361b18 An option would be to put those views into some lazy close queue, where they would be cleaned after it is clear that it was a not a reload. I don't it is very relevant though. Maybe it would even be undesired (can't come back to the previous state with back button e.g. after x minutes like now). I don't have an opinion on that feature because I never used it 🤷♂️ |
There was a problem hiding this comment.
Some typo and nits.
I would also like @heruan to have a brief look, to be sure this change may not break beacon handling in Collaboration Kit
flow-server/src/main/java/com/vaadin/flow/server/communication/AtmospherePushConnection.java
Outdated
Show resolved
Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java
Outdated
Show resolved
Hide resolved
...s/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/UIsCollectedWithBeaconAPIIT.java
Outdated
Show resolved
Hide resolved
...s/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/UIsCollectedWithBeaconAPIIT.java
Outdated
Show resolved
Hide resolved
…/AtmospherePushConnection.java Co-authored-by: Marco Collovati <marco@vaadin.com>
…/ServerRpcHandler.java Co-authored-by: Marco Collovati <marco@vaadin.com>
|
I briefly looked at what @heruan et co have done in collaboration engine with beacon api, and my hunch is that no issues with that. Currently you seem to be hooking on unload event, which is kind of deprecated but works most often. Depending on how you use it, you might in some cases now (after this change) use normal detach listeners, but I'd expect that you should at least in upcoming versions also detect visibility changes as well (e.g. to gray out a user who has switched to another tab) 🤷♂️ |
flow-client/src/main/java/com/vaadin/client/ApplicationConnection.java
Outdated
Show resolved
Hide resolved
flow-client/src/main/java/com/vaadin/client/ApplicationConnection.java
Outdated
Show resolved
Hide resolved
…ion.java Co-authored-by: Peter Czuczor <61667986+czp13@users.noreply.github.com>
…ion.java Co-authored-by: Peter Czuczor <61667986+czp13@users.noreply.github.com>
|
Kudos, SonarCloud Quality Gate passed!
|
mstahv
changed the title
Since Flow 24.1.0.alpha4, the UIs are closed eagerly when navigating away from the page. To test memory used by multiple UIs, they should be opened in separated tabs. See vaadin/flow#16657
Since Flow 24.1.0.alpha4, the UIs are closed eagerly when navigating away from the page. To test memory used by multiple UIs, they should be opened in separated tabs. See vaadin/flow#16657
|
This ticket/PR has been released with Vaadin 24.1.0. |
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
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
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>
* 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 * chore: updated test to run with older selenium API * chore: clean up * chore: fixed window index in IT --------- Co-authored-by: Matti Tahvonen <matti@vaadin.com>
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
Type of change
Checklist
Additional for
Featuretype of change