gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.53k stars 378 forks source link

Elements wrapping architecture flawed ? #3119

Closed dankurka closed 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 3113

Hi GWT team,

Unless I'm totally mistaken, based on the current architecture, wrapping an
element leads to several issues :

1) Memory leaks until the window is closed.

When an element gets wrapped, the widget calls
RootPanel.detachOnWindowClose(). Which means that the root panel will keep
a reference to it until the window gets closed, even if the widget has been
long gone from whatever method created it. This leads to huge memory leaks.

The only way to instruct the RootPanel to “forget” about it is detachNow().
But it cannot be called if the element is still attached (obviously). So no
component can “claim” the responsibility of this widget’s handling.

2) An Exception raised during "cleanup" (window close) which prevents it to
complete successfully if the element has disappeared from the Document.

We cannot be notified if the element disappears. So the RootPanel’s
detachWidgets() method will fail and raise an exception.
We could handle the widget’s detachment and subsequently call detachNow()
if it was properly tied to its parent, but that’s also impossible (see below).

3) No proper way to add the created widget to any panel (at least, declare
it as a children so that it gets proper onAttach/onDetach “events”).

There is no way for a panel to “acquire” the created widget. If we want to
set the widget’s parent from an already attached panel, it will try to
onAttach() the widget which will not work as it’s already attached. And if
we bluntly add the widget to an unattached panel’s list of children, we
will encounter the same issue at onAttach() time.

4) Finally, we cannot generically handle “wrappable” widgets. I’d love all
of them to implement an interface such as HasWrap() or IsWrappable().

Why would we want to make a huge use of wrap() ?
We have decided to split the pages design from the web logic. To achieve
this, we designed a set of components using an RPC service that gets files
from the server (containing HTML + the related CSS code, with a nice
runtime cleanup/compress/cache process) and dynamically injects it inside
the DOM.

Then, by wrapping the proper elements, we very easily add the web logic.
This is basically pretty similar to an MVC model and completely takes the
web design out of the Java developers hands.

What I suggest (but I’m no architect, so, don’t hesitate to prove me wrong)
is that widgets created using a wrap call would need a slightly different
handling in the Widget.setParent() call :

        if (parent == null) {
        ...
          if (isWrapped) RootPanel.detachOnWindowClose(this) ;
        } else {
        ...
          if (isWrapped) RootPanel.removeFromDetachList(this) ;
          if (parent.isAttached() && !isWrapped) {
            onAttach();
            ...
          }
        ...
        }

And, of course, the RootPanel would need the proper removeFromDetachList()
method.

Thanks a lot in advance for your replies.

Reported by frederic.olivie on 2008-11-19 12:34:21

dankurka commented 9 years ago
We are having the same issue with wrap, using a probably similar architecture.
The first major problem we've found is that after we call RootPanel.get(id) with a
given id, no other element with the same id can be wrapped, even if the previous
element is already removed from the DOM.

It appears to be related to the rootPanels map from id to RootPanel's, used inside
the RootPanel.get(id) method. Apparently the wrap method is trying to reference the
previous element, as if it had not yet been released.

Would be great for us to have a better implementation of wrap(), because it really
helps to separate desginer and programmer tasks.

We shall report further advances here. Also, looking forward to your replies.

Reported by inhodpr on 2008-12-29 21:51:01

dankurka commented 9 years ago
Closing as a result of issue triage. More information can be found here:

http://groups.google.com/group/google-web-
toolkit/browse_thread/thread/3a7bf1c6a3d35431

Thanks

Reported by cramsdale@google.com on 2009-12-21 16:17:19

dankurka commented 9 years ago

Reported by cramsdale@google.com on 2009-12-21 19:38:28

dankurka commented 9 years ago
I'm facing the similar problem. The issue was closed as stale but there is no comment

if there is no intent to improve wrapping of existing elements or this is considered
by 
design and there won't be any effort. Any comment?

Thanks a lot for your replies.

Reported by george.djabarov on 2010-01-21 00:35:37

dankurka commented 9 years ago
any workaround you folks use when using wrap at the moment to prevent memory leak?

Reported by second.comet on 2011-01-17 04:21:55