Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate and avoid unload event handler #9984

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
jnehlmeier marked this conversation as resolved.
Show resolved Hide resolved
}
});
}-*/;

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
jnehlmeier marked this conversation as resolved.
Show resolved Hide resolved
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>
*
jnehlmeier marked this conversation as resolved.
Show resolved Hide resolved
*
* @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.
*
niloc132 marked this conversation as resolved.
Show resolved Hide resolved
* <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() {
jnehlmeier marked this conversation as resolved.
Show resolved Hide resolved
// 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