Possibly related: http://code.google.com/p/google-web-toolkit/issues/detail?id=1821
http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/GWT.java File user/src/com/google/gwt/core/client/GWT.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/GWT.java#newcode92 user/src/com/google/gwt/core/client/GWT.java:92: var $moduleName = __gwtModuleFunction.__moduleName; Should this rather use GWT.getModuleName() ? http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/GWT.java#newcode104 user/src/com/google/gwt/core/client/GWT.java:104: var keys = Object.keys(window); Object.keys isn't supported in oldIE, Opera<12 and Safari<5 (I think we can safely ignore Firefox<4 and Chrome<5 nowadays) https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Object/keys http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/GWT.java#newcode105 user/src/com/google/gwt/core/client/GWT.java:105: var frame = window.frameElement; Too bad that window.frameElement isn't more widely supported, but it shouldn't be an issue here (will be undefined if unsupported, which will only prevent the final removal) https://developer.mozilla.org/en-US/docs/DOM/window.frameElement http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/Impl.java File user/src/com/google/gwt/core/client/impl/Impl.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/Impl.java#newcode279 user/src/com/google/gwt/core/client/impl/Impl.java:279: if (unloadSupport.isUnloadSupported() && Impl.isModuleUnloaded()) { "Impl." qualifier is superfluous http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java File user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java#newcode58 user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java:58: * Return true if {@link com.google.gwt.core.client.GWT#unloadModule()} is enabled. Default is false. That comment is a bit confusing http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java#newcode93 user/src/com/google/gwt/core/client/impl/UnloadSupportEnabled.java:93: GWT.UncaughtExceptionHandler uncaughtExceptionHandler = GWT.getUncaughtExceptionHandler(); Is that try/catch really needed? Wouldn't it rather be the responsibility of the caller? (including a try/catch in the disposeAll loop to report a single UmbrellaException in the end) http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/Window.java File user/src/com/google/gwt/user/client/Window.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/Window.java#newcode22 user/src/com/google/gwt/user/client/Window.java:22: import com.google.gwt.event.logical.shared.*; Star imports, really?! http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/DOMImplStandard.java File user/src/com/google/gwt/user/client/impl/DOMImplStandard.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/DOMImplStandard.java#newcode179 user/src/com/google/gwt/user/client/impl/DOMImplStandard.java:179: Impl.scheduleDispose(new Disposable() { Wow that seems a bit overkill to do it for each and every widget. We have RootPanel.hookWindowClosing already that would take care of detaching widgets recursively, we should probably hook into that and provide a new public method on c.g.g.u.c.DOM to explicitly schedule the disposal of an element, or more simply ask people to call sinkEvents(elem, 0) when they're done with an element. Plus, wouldn't it lead to a leak in the app itself then, by keeping a handle to these elements in the list of disposables until the app is unloaded? If scheduling a disposable is needed, then at least it should be attached to the element to avoid scheduling a new disposable when calling sinkEvents for the same element (with different 'bits'), and even better call Impl.dispose when the bits are '0'. Of course the disposable would have to clear the element->disposable reference or we'd create another leak (in IE at least). And widgets should probably be modified to sinkEvents(elem, 0) automatically in onDetach in addition to setEventListener(elem, null). That would probably make them perform a little bit slower when attaching/detaching (having to re-sink events in onLoad) but it would probably be a better trade-off than artificially keeping all those elements around until the app is unloaded (FYI, I keep Google Groups open in a tab all day, and possibly for days if I don't shutdown my computer). Finally, why isn't this done in sinkBitlessEvent too? http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/HistoryImpl.java File user/src/com/google/gwt/user/client/impl/HistoryImpl.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/HistoryImpl.java#newcode107 user/src/com/google/gwt/user/client/impl/HistoryImpl.java:107: oldHandler = $wnd.onhashchange; We'd better add a "private JavaScriptObject oldHandler" field than make oldHandler a "global" (even if HistoryImpl should be a singleton) http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/WindowImpl.java File user/src/com/google/gwt/user/client/impl/WindowImpl.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/impl/WindowImpl.java#newcode25 user/src/com/google/gwt/user/client/impl/WindowImpl.java:25: $wnd.onclose = null; No onbeforeunload? No resetting to "oldOnUnload" like for onhashchange in HistoryImpl? http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/PotentialElement.java File user/src/com/google/gwt/user/client/ui/PotentialElement.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode49 user/src/com/google/gwt/user/client/ui/PotentialElement.java:49: $wnd.GwtPotentialElementShim = null; That'll cause problems if you unload a module from a page where other modules are loaded. Also, I believe this is not needed as I don't think the GwtPotentialElementShim creates a leak. http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/RootPanel.java File user/src/com/google/gwt/user/client/ui/RootPanel.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/RootPanel.java#newcode237 user/src/com/google/gwt/user/client/ui/RootPanel.java:237: public static void detachWidgets() { The comment says "Do not call this method directly", so exposing it as a public method is risky (and the comment should then probably be turned into a JavaDoc). Also, the comment still says "package-protected". I don't think you really wanted to make that method public. http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/impl/FocusImplStandard.java File user/src/com/google/gwt/user/client/ui/impl/FocusImplStandard.java (right): http://gwt-code-reviews.appspot.com/1827804/diff/1/user/src/com/google/gwt/user/client/ui/impl/FocusImplStandard.java#newcode70 user/src/com/google/gwt/user/client/ui/impl/FocusImplStandard.java:70: Impl.scheduleDispose(new Disposable() { Similar issue as in DOM.sinkEvents: will create a leak by artificially keeping a handle on the element until the app is unloaded. We should probably add an explicit disposeFocusable(Element) on FocusImpl and call it where necessary (maybe add a initFocusable(element) so sink the event, called in onAttach of the focusable widgets, and call disposeFocusable in onDetach). http://gwt-code-reviews.appspot.com/1827804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
