Skip to content

Commit

Permalink
Deprecate and avoid unload event handler (#9984)
Browse files Browse the repository at this point in the history
Event handling for `unload` and `beforeunload` now is split, so that
only explicit calls to `WindowImpl.initWindowCloseHandler()`,
`Window.addCloseHandler()`, or `Window.addWindowCloseListener()` can
cause the `unload` event handler to be attached - all of which are now
deprecated. The `beforeunload` event wiring is now the only event
handler set up when `Window.addWindowClosingHandler` etc are called.

RootPanel methods formerly intended to prevent memory leaks during the
`unload` event are deprecated, since modern browsers do not leak these
handlers, and may prevent correct behavior of apps using bfcache.

Documentation changed on using various static wrap(Element) methods to
reflect how to detach, if desired, when the page is closed or no longer
in use.

Fixes #9908
  • Loading branch information
niloc132 authored Oct 9, 2024
1 parent 6307e79 commit 5ecb770
Show file tree
Hide file tree
Showing 29 changed files with 188 additions and 73 deletions.
18 changes: 14 additions & 4 deletions user/src/com/google/gwt/user/client/Window.java
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ public HandlerManager getHandlers() {
// Package protected for testing.
static WindowHandlers handlers;
private static boolean closeHandlersInitialized;
private static boolean beforeCloseHandlersInitialized;
private static boolean scrollHandlersInitialized;
private static boolean resizeHandlersInitialized;
private static int lastResizeWidth;
Expand All @@ -518,7 +519,10 @@ public HandlerManager getHandlers() {
*
* @param handler the handler
* @return returns the handler registration
* @deprecated This method requires the use of the {@code unload} browser event, which is
* deprecated in all browsers.
*/
@Deprecated
public static HandlerRegistration addCloseHandler(CloseHandler<Window> handler) {
maybeInitializeCloseHandlers();
return addHandler(CloseEvent.getType(), handler);
Expand All @@ -531,7 +535,6 @@ public static HandlerRegistration addCloseHandler(CloseHandler<Window> handler)
* @return returns the handler registration
*/
public static HandlerRegistration addResizeHandler(ResizeHandler handler) {
maybeInitializeCloseHandlers();
maybeInitializeResizeHandlers();
return addHandler(ResizeEvent.getType(), handler);
}
Expand All @@ -556,7 +559,7 @@ public static void addWindowCloseListener(WindowCloseListener listener) {
*/
public static HandlerRegistration addWindowClosingHandler(
ClosingHandler handler) {
maybeInitializeCloseHandlers();
maybeInitializeBeforeCloseHandlers();
return addHandler(Window.ClosingEvent.getType(), handler);
}

Expand All @@ -579,7 +582,6 @@ public static void addWindowResizeListener(WindowResizeListener listener) {
*/
public static HandlerRegistration addWindowScrollHandler(
Window.ScrollHandler handler) {
maybeInitializeCloseHandlers();
maybeInitializeScrollHandlers();
return addHandler(Window.ScrollEvent.getType(), handler);
}
Expand Down Expand Up @@ -910,13 +912,21 @@ private static WindowHandlers getHandlers() {
return handlers;
}

@Deprecated
private static void maybeInitializeCloseHandlers() {
if (GWT.isClient() && !closeHandlersInitialized) {
impl.initWindowCloseHandler();
impl.initWindowUnloadHandler();
closeHandlersInitialized = true;
}
}

private static void maybeInitializeBeforeCloseHandlers() {
if (GWT.isClient() && !beforeCloseHandlersInitialized) {
impl.initWindowBeforeUnloadHandler();
beforeCloseHandlersInitialized = true;
}
}

private static void maybeInitializeResizeHandlers() {
if (GWT.isClient() && !resizeHandlersInitialized) {
impl.initWindowResizeHandler();
Expand Down
39 changes: 23 additions & 16 deletions user/src/com/google/gwt/user/client/impl/WindowImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,30 @@ public native String getHash() /*-{
public native String getQueryString() /*-{
return $wnd.location.search;
}-*/;

public native void initWindowCloseHandler() /*-{

@Deprecated
public void initWindowCloseHandler() {
initWindowUnloadHandler();
initWindowBeforeUnloadHandler();
}

@Deprecated
public native void initWindowUnloadHandler() /*-{
var oldOnUnload = $wnd.onunload;
$wnd.onunload = $entry(function(evt) {
try {
@com.google.gwt.user.client.Window::onClosed()();
} finally {
oldOnUnload && oldOnUnload(evt);
$wnd.onunload = null;
}
});
}-*/;

public native void initWindowBeforeUnloadHandler() /*-{
var oldOnBeforeUnload = $wnd.onbeforeunload;
var oldOnUnload = $wnd.onunload;
// Old mozilla doesn't like $entry's explicit return statement and
// will always pop up a confirmation dialog. This is worked around by
// just wrapping the call to onClosing(), which still has the semantics
Expand All @@ -55,18 +74,6 @@ public native void initWindowCloseHandler() /*-{
}
// returns undefined.
};
$wnd.onunload = $entry(function(evt) {
try {
@com.google.gwt.user.client.Window::onClosed()();
} finally {
oldOnUnload && oldOnUnload(evt);
$wnd.onresize = null;
$wnd.onscroll = null;
$wnd.onbeforeunload = null;
$wnd.onunload = null;
}
});
}-*/;

public native void initWindowResizeHandler() /*-{
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Anchor.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public class Anchor extends FocusWidget implements HasHorizontalAlignment,
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Button.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class Button extends ButtonBase {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/FileUpload.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public class FileUpload extends FocusWidget implements HasName, HasChangeHandler
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
4 changes: 2 additions & 2 deletions user/src/com/google/gwt/user/client/ui/FormPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ interface IFrameTemplate extends SafeHtmlTemplates {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* <p>
* The specified form element's target attribute will not be set, and the
Expand All @@ -280,7 +280,7 @@ public static FormPanel wrap(Element element) {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* <p>
* If the createIFrame parameter is set to <code>true</code>, then the wrapped
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Frame.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public class Frame extends Widget implements HasLoadHandlers {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/HTML.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class HTML extends Label
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/HTMLPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static String createUniqueId() {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Hidden.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class Hidden extends Widget implements HasName, TakesValue<String>, IsEdi
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Image.java
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ public static void prefetch(SafeUri url) {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/InlineHTML.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class InlineHTML extends HTML {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/InlineLabel.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class InlineLabel extends Label {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public class Label extends LabelBase<String> implements HasDirectionalText,
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/ListBox.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public class ListBox extends FocusWidget implements SourcesChangeEvents,
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
* @return list box
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class PasswordTextBox extends TextBox {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
2 changes: 1 addition & 1 deletion user/src/com/google/gwt/user/client/ui/ResetButton.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ResetButton extends Button {
*
* This element must already be attached to the document. If the element is
* removed from the document, you must call
* {@link RootPanel#detachNow(Widget)}.
* {@link Widget#removeFromParent()}.
*
* @param element the element to be wrapped
*/
Expand Down
58 changes: 36 additions & 22 deletions user/src/com/google/gwt/user/client/ui/RootPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@
import com.google.gwt.dom.client.BodyElement;
import com.google.gwt.dom.client.Document;
import com.google.gwt.dom.client.Element;
import com.google.gwt.event.logical.shared.CloseEvent;
import com.google.gwt.event.logical.shared.CloseHandler;
import com.google.gwt.i18n.client.BidiUtils;
import com.google.gwt.i18n.client.HasDirection;
import com.google.gwt.i18n.client.LocaleInfo;
import com.google.gwt.user.client.Event;
import com.google.gwt.user.client.Window;

import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -92,11 +89,13 @@ public void execute(Widget w) {
* This method may only be called per widget, and only for widgets that were
* originally passed to {@link #detachOnWindowClose(Widget)}.
* </p>
*
*
* @param widget the widget that no longer needs to be cleaned up when the
* page closes
* @see #detachOnWindowClose(Widget)
* @deprecated Instead, use {@link Widget#removeFromParent()}.
*/
@Deprecated
public static void detachNow(Widget widget) {
assert widgetsToDetach.contains(widget) : "detachNow() called on a widget "
+ "not currently in the detach list";
Expand All @@ -111,25 +110,30 @@ public static void detachNow(Widget widget) {
/**
* Adds a widget to the detach list. This is the list of widgets to be
* detached when the page unloads.
*
*
* <p>
* This method must be called for all widgets that have no parent widgets.
* These are most commonly {@link RootPanel RootPanels}, but can also be any
* widget used to wrap an existing element on the page. Failing to do this may
* cause these widgets to leak memory. This method is called automatically by
* widgets' wrap methods (e.g.
* {@link Button#wrap(com.google.gwt.dom.client.Element)}).
* {@link Button#wrap(Element)}).
* </p>
*
*
* <p>
* This method may <em>not</em> be called on any widget whose element is
* contained in another widget. This is to ensure that the DOM and Widget
* hierarchies cannot get into an inconsistent state.
* </p>
*
*
* @param widget the widget to be cleaned up when the page closes
* @see #detachNow(Widget)
* @deprecated While originally introduced to combat memory leaks in old browsers, this is no
* longer necessary, and the unload event used by this method is being removed from browsers.
* Additionally, it is unreliable as a means to ensure calls to {@link Widget#onUnload()}. See
* <a href="https://github.com/gwtproject/gwt/issues/9908">Issue 9908</a> for more information.
*/
@Deprecated
public static void detachOnWindowClose(Widget widget) {
assert !widgetsToDetach.contains(widget) : "detachOnUnload() called twice "
+ "for the same widget";
Expand Down Expand Up @@ -188,8 +192,6 @@ public static RootPanel get(String id) {
// on the first RootPanel.get(String) or RootPanel.get()
// call.
if (rootPanels.size() == 0) {
hookWindowClosing();

// If we're in a RTL locale, set the RTL directionality
// on the entire document.
if (LocaleInfo.getCurrentLocale().isRTL()) {
Expand Down Expand Up @@ -223,16 +225,37 @@ public static native com.google.gwt.user.client.Element getBodyElement() /*-{

/**
* Determines whether the given widget is in the detach list.
*
* <p>
* Note that modern browsers do not have the memory leaks that originally required use of this
* feature - it is retained only to support application-specific detach events.
* </p>
*
* @param widget the widget to be checked
* @return <code>true</code> if the widget is in the detach list
* @deprecated Use of the detach list requires the unload event, which is planned to be removed
* in future browser updates. See notice on {@link #detachOnWindowClose(Widget)} and
* <a href="https://github.com/gwtproject/gwt/issues/9908">Issue 9908</a> for more information.
*/
@Deprecated
public static boolean isInDetachList(Widget widget) {
return widgetsToDetach.contains(widget);
}

// Package-protected for use by unit tests. Do not call this method directly.
static void detachWidgets() {
/**
* Detaches all widgets that were set to be detached on window close by a call to
* {@link #detachOnWindowClose}. Formerly was package-protected, now can be called by projects
* that required the old behavior and are willing to set up their own onclose handler on the
* window.
* <p>
* Note that modern browsers do not have the memory leaks that originally required use of this
* feature - it is retained only to support application-specific detach events.
* </p>
*
* @deprecated See notice on {@link #detachOnWindowClose(Widget)} and
* <a href="https://github.com/gwtproject/gwt/issues/9908">Issue 9908</a> for more information.
*/
@Deprecated
public static void detachWidgets() {
// When the window is closing, detach all widgets that need to be
// cleaned up. This will cause all of their event listeners
// to be unhooked, which will avoid potential memory leaks.
Expand All @@ -258,15 +281,6 @@ private static native Element getRootElement() /*-{
return $doc;
}-*/;

private static void hookWindowClosing() {
// Catch the window closing event.
Window.addCloseHandler(new CloseHandler<Window>() {
public void onClose(CloseEvent<Window> closeEvent) {
detachWidgets();
}
});
}

/*
* Checks to see whether the given element has any parent element that
* belongs to a widget. This is not terribly efficient, and is thus only used
Expand Down
Loading

0 comments on commit 5ecb770

Please sign in to comment.