Clarifies the order in which onLoad() and onUnload() will be called. onLoad() will always be
called *after* a Widget becomes attached, and onUnload() will be called just *before* a
Widget is detached.
Also adds template methods doAttachChildren() and doDetachChildren() making it easier for
Widgets to maintain this contract correctly.
Patch by: jgw
Review by: knorton, sandymac
Issue: 1121
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1229 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/CheckBox.java b/user/src/com/google/gwt/user/client/ui/CheckBox.java
index 357f1e6..e939231 100644
--- a/user/src/com/google/gwt/user/client/ui/CheckBox.java
+++ b/user/src/com/google/gwt/user/client/ui/CheckBox.java
@@ -163,11 +163,10 @@
* onAttach needs special handling for the CheckBox case. Must still call
* {@link Widget#onAttach()} to preserve the <code>onAttach</code> contract.
*/
- protected void onAttach() {
+ protected void onLoad() {
// Sets the event listener on the inputElem, as in this case that's the
// element we want so input on.
DOM.setEventListener(inputElem, this);
- super.onAttach();
}
/**
@@ -175,11 +174,10 @@
* document. Overridden because of IE bug that throws away checked state and
* in order to clear the event listener off of the <code>inputElem</code>.
*/
- protected void onDetach() {
+ protected void onUnload() {
// Clear out the inputElem's event listener (breaking the circular
// reference between it and the widget).
DOM.setEventListener(inputElem, null);
setChecked(isChecked());
- super.onDetach();
}
}
diff --git a/user/src/com/google/gwt/user/client/ui/Composite.java b/user/src/com/google/gwt/user/client/ui/Composite.java
index 2e63f11..491fea1 100644
--- a/user/src/com/google/gwt/user/client/ui/Composite.java
+++ b/user/src/com/google/gwt/user/client/ui/Composite.java
@@ -48,6 +48,13 @@
return super.getElement();
}
+ public boolean isAttached() {
+ if (widget != null) {
+ return widget.isAttached();
+ }
+ return false;
+ }
+
/**
* Provides subclasses access to the topmost widget that defines this
* composite.
@@ -84,17 +91,19 @@
}
protected void onAttach() {
- super.onAttach();
-
- // Call onAttach() on behalf of the contained widget.
widget.onAttach();
+ onLoad();
}
protected void onDetach() {
- super.onDetach();
-
- // Call onDetach() on behalf of the contained widget.
- widget.onDetach();
+ try {
+ onUnload();
+ } finally {
+ // We don't want an exception in user code to keep us from calling the
+ // super implementation (or event listeners won't get cleaned up and
+ // the attached flag will be wrong).
+ widget.onDetach();
+ }
}
/**
diff --git a/user/src/com/google/gwt/user/client/ui/Panel.java b/user/src/com/google/gwt/user/client/ui/Panel.java
index 2b0b37b..8c755cc 100644
--- a/user/src/com/google/gwt/user/client/ui/Panel.java
+++ b/user/src/com/google/gwt/user/client/ui/Panel.java
@@ -71,15 +71,14 @@
throw new IllegalArgumentException("w is not a child of this panel");
}
- // Remove it at the DOM and GWT levels.
+ // setParent() must be called before removeChild() to ensure that the
+ // element is still attached when onDetach()/onUnload() are called.
Element elem = w.getElement();
w.setParent(null);
DOM.removeChild(DOM.getParent(elem), elem);
}
- protected void onAttach() {
- super.onAttach();
-
+ protected void doAttachChildren() {
// Ensure that all child widgets are attached.
for (Iterator it = iterator(); it.hasNext();) {
Widget child = (Widget) it.next();
@@ -87,13 +86,29 @@
}
}
- protected void onDetach() {
- super.onDetach();
-
+ protected void doDetachChildren() {
// Ensure that all child widgets are detached.
for (Iterator it = iterator(); it.hasNext();) {
Widget child = (Widget) it.next();
child.onDetach();
}
}
+
+ /**
+ * A Panel's onLoad method will be called after all of its children are
+ * attached.
+ *
+ * @see Widget#onLoad()
+ */
+ protected void onLoad() {
+ }
+
+ /**
+ * A Panel's onUnload method will be called before its children become
+ * detached themselves.
+ *
+ * @see Widget#onLoad()
+ */
+ protected void onUnload() {
+ }
}
diff --git a/user/src/com/google/gwt/user/client/ui/Tree.java b/user/src/com/google/gwt/user/client/ui/Tree.java
index 74acaa6..103c02d 100644
--- a/user/src/com/google/gwt/user/client/ui/Tree.java
+++ b/user/src/com/google/gwt/user/client/ui/Tree.java
@@ -597,6 +597,24 @@
return accum.iterator();
}
+ protected void doAttachChildren() {
+ // Ensure that all child widgets are attached.
+ for (Iterator it = iterator(); it.hasNext();) {
+ Widget child = (Widget) it.next();
+ child.onAttach();
+ }
+ DOM.setEventListener(focusable, this);
+ }
+
+ protected void doDetachChildren() {
+ // Ensure that all child widgets are detached.
+ for (Iterator it = iterator(); it.hasNext();) {
+ Widget child = (Widget) it.next();
+ child.onDetach();
+ }
+ DOM.setEventListener(focusable, null);
+ }
+
/**
* Indicates if keyboard navigation is enabled for the Tree and for a given
* TreeItem. Subclasses of Tree can override this function to selectively
@@ -610,28 +628,6 @@
return true;
}
- protected void onAttach() {
- super.onAttach();
-
- // Ensure that all child widgets are attached.
- for (Iterator it = iterator(); it.hasNext();) {
- Widget child = (Widget) it.next();
- child.onAttach();
- }
- DOM.setEventListener(focusable, this);
- }
-
- protected void onDetach() {
- super.onDetach();
-
- // Ensure that all child widgets are detached.
- for (Iterator it = iterator(); it.hasNext();) {
- Widget child = (Widget) it.next();
- child.onDetach();
- }
- DOM.setEventListener(focusable, null);
- }
-
protected void onLoad() {
root.updateStateRecursive();
}
diff --git a/user/src/com/google/gwt/user/client/ui/Widget.java b/user/src/com/google/gwt/user/client/ui/Widget.java
index d80321d..caa6855 100644
--- a/user/src/com/google/gwt/user/client/ui/Widget.java
+++ b/user/src/com/google/gwt/user/client/ui/Widget.java
@@ -72,32 +72,49 @@
}
/**
+ * If a widget implements HasWidgets, it must override this method and
+ * call onAttach() for each of its child widgets.
+ *
+ * @see Panel#onAttach()
+ */
+ protected void doAttachChildren() {
+ }
+
+ /**
+ * If a widget implements HasWidgets, it must override this method and
+ * call onDetach() for each of its child widgets.
+ *
+ * @see Panel#onDetach()
+ */
+ protected void doDetachChildren() {
+ }
+
+ /**
* This method is called when a widget is attached to the browser's document.
- * To receive notification after a Widget has been added from the
+ * To receive notification after a Widget has been added to the
* document, override the {@link #onLoad} method.
*
* <p>
* Subclasses that override this method must call
* <code>super.onAttach()</code> to ensure that the Widget has been
- * attached to the underlying Element.
+ * attached to its underlying Element.
* </p>
*
* @throws IllegalStateException if this widget is already attached
*/
protected void onAttach() {
- if (attached) {
+ if (isAttached()) {
throw new IllegalStateException(
"Should only call onAttach when the widget is detached from the browser's document");
}
attached = true;
-
- // Set the main element's event listener. This should only be set
- // while the widget is attached, because it creates a circular
- // reference between JavaScript and the DOM.
DOM.setEventListener(getElement(), this);
+ doAttachChildren();
- // Now that the widget is attached, call onLoad().
+ // onLoad() gets called only *after* all of the children are attached and
+ // the attached flag is set. This allows widgets to be notified when they
+ // are fully attached, and panels when all of their children are attached.
onLoad();
}
@@ -110,27 +127,27 @@
* Subclasses that override this method must call
* <code>super.onDetach()</code> to ensure that the Widget has been
* detached from the underlying Element. Failure to do so will result
- * in application memeroy leaks due to circular references between DOM
+ * in application memory leaks due to circular references between DOM
* Elements and JavaScript objects.
* </p>
*
* @throws IllegalStateException if this widget is already detached
*/
protected void onDetach() {
- if (!attached) {
+ if (!isAttached()) {
throw new IllegalStateException(
"Should only call onDetach when the widget is attached to the browser's document");
}
-
- // Give the user a chance to clean up, but don't trust the code to not throw
+
try {
+ // onUnload() gets called *before* everything else (the opposite of
+ // onLoad()).
onUnload();
} finally {
- attached = false;
-
- // Clear out the element's event listener (breaking the circular
- // reference between it and the widget).
+ // Put this in a finally, just in case onUnload throws an exception.
+ doDetachChildren();
DOM.setEventListener(getElement(), null);
+ attached = false;
}
}
@@ -140,7 +157,7 @@
*/
protected void onLoad() {
}
-
+
/**
* This method is called immediately before a widget will be detached from the
* browser's document.
@@ -159,7 +176,7 @@
* @param elem the object's new element
*/
protected void setElement(Element elem) {
- if (attached) {
+ if (isAttached()) {
// Remove old event listener to avoid leaking. onDetach will not do this
// for us, because it is only called when the widget itself is detached from
// the document.
@@ -167,7 +184,7 @@
}
super.setElement(elem);
- if (attached) {
+ if (isAttached()) {
// Hook the event listener back up on the new element. onAttach will not
// do this for us, because it is only called when the widget itself is
// attached to the document.
diff --git a/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java b/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
new file mode 100644
index 0000000..430deda
--- /dev/null
+++ b/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
@@ -0,0 +1,133 @@
+/*
+ * Copyright 2007 Google Inc.
+ *
+ * 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.google.gwt.user.client.ui;
+
+import com.google.gwt.junit.client.GWTTestCase;
+import com.google.gwt.user.client.DOM;
+import com.google.gwt.user.client.Element;
+
+/**
+ * Tests the semantics and ordering of onAttach/onDetach/onLoad/onUnload.
+ */
+public class WidgetOnLoadTest extends GWTTestCase {
+
+ public String getModuleName() {
+ return "com.google.gwt.user.User";
+ }
+
+ static int orderIndex;
+
+ static boolean isElementAttached(Element elem) {
+ return DOM.isOrHasChild(RootPanel.getBodyElement(), elem);
+ }
+
+ static class TestPanel extends FlowPanel {
+ int onAttachOrder;
+ int onLoadOrder;
+ int onDetachOrder;
+ int onUnloadOrder;
+ boolean domAttachedOnLoad;
+ boolean domAttachedOnUnload;
+
+ protected void onAttach() {
+ onAttachOrder = ++orderIndex;
+ super.onAttach();
+ }
+
+ protected void onLoad() {
+ onLoadOrder = ++orderIndex;
+ domAttachedOnLoad = isElementAttached(getElement());
+ super.onLoad();
+ }
+
+ protected void onDetach() {
+ onDetachOrder = ++orderIndex;
+ super.onDetach();
+ }
+
+ protected void onUnload() {
+ onUnloadOrder = ++orderIndex;
+ domAttachedOnUnload = isElementAttached(getElement());
+ super.onUnload();
+ }
+ }
+
+ static class TestWidget extends Label {
+ int onAttachOrder;
+ int onLoadOrder;
+ int onDetachOrder;
+ int onUnloadOrder;
+ boolean domAttachedOnLoad;
+ boolean domAttachedOnUnload;
+
+ protected void onAttach() {
+ onAttachOrder = ++orderIndex;
+ super.onAttach();
+ }
+
+ protected void onLoad() {
+ domAttachedOnLoad = isElementAttached(getElement());
+ onLoadOrder = ++orderIndex;
+ super.onLoad();
+ }
+
+ protected void onDetach() {
+ onDetachOrder = ++orderIndex;
+ super.onDetach();
+ }
+
+ protected void onUnload() {
+ onUnloadOrder = ++orderIndex;
+ domAttachedOnUnload = isElementAttached(getElement());
+ super.onUnload();
+ }
+ }
+
+ public void testOnLoadAndUnloadOrder() {
+ TestPanel tp = new TestPanel();
+ TestWidget tw = new TestWidget();
+
+ tp.add(tw);
+ RootPanel.get().add(tp);
+ RootPanel.get().remove(tp);
+
+ // Trivial tests. Ensure that each panel/widget's onAttach/onDetach are
+ // called before their associated onLoad/onUnload.
+ assertTrue(tp.onAttachOrder < tp.onLoadOrder);
+ assertTrue(tp.onDetachOrder < tp.onUnloadOrder);
+ assertTrue(tw.onAttachOrder < tw.onLoadOrder);
+ assertTrue(tw.onDetachOrder < tw.onUnloadOrder);
+
+ // Also trivial. Ensure that the panel's onAttach/onDetach is called before
+ // its child's onAttach/onDetach.
+ assertTrue(tp.onAttachOrder < tw.onAttachOrder);
+
+ // Ensure that the panel's onLoad is only called after its widgets are
+ // attached/loaded.
+ assertTrue(tp.onLoadOrder > tw.onLoadOrder);
+
+ // Ensure that the panel's onUnload is called before its widgets are
+ // detached/unloaded.
+ assertTrue(tp.onUnloadOrder < tw.onUnloadOrder);
+
+ // Make sure each widget/panel's elements are actually still attached to the
+ // DOM during onLoad/onUnload.
+ assertTrue(tp.domAttachedOnLoad);
+ assertTrue(tp.domAttachedOnUnload);
+ assertTrue(tw.domAttachedOnLoad);
+ assertTrue(tw.domAttachedOnUnload);
+ }
+}
