On 2010/09/27 09:41:59, tbroyer wrote:
http://gwt-code-reviews.appspot.com/925801/diff/1/2
File user/src/com/google/gwt/app/place/AbstractProxyListActivity.java
(right):
http://gwt-code-reviews.appspot.com/925801/diff/1/2#newcode256
user/src/com/google/gwt/app/place/AbstractProxyListActivity.java:256:
if (view
== null) {
Isn't the problem that this activity isn't reusable? I mean, it
initializes the
view in its constructor (lines 87 to 108) and clears everything in
onStop().
Shouldn't it rather initializes in start()? and keep the view around
in onStop
so that it can be re-initialized (re-start()ed)? (and probably clear
the idToRow
and idToProxy maps)
This check would then be "if (display == null)", just like in
AbstractProxyEditActivity.
re-reading the AbstractProxyEditActivity code, it looks like it isn't
reusable either, as the editorDriver is 'null'ed in onStop and not
(re)created in start().
It's no big deal, activities (those activities) are not supposed to be
reused. My concern though is that AbstractProxyListActivity changes the
view before it is started. It probably works very well in the cases that
Spring Roo generates, but I'm concerned that it might not be always the
case. For instance: an ActivityMapper instantiates a new such activity,
which overrides equals() so it compares equal to any other activity of
the same class on the same "place" (same entity proxy class, same page
size, same page number; for instance); if the current activity compares
equal to the new one, ActivityManager keeps it around, but the new one
would have already overwritten the view's delegate and its HasData's
selection model (assuming they share the same singleton view).
But actually, I even wonder if the current code works: when switching
from one AbstractProxyListActivity to another (sharing the same view,
they're the same activities, just 2 distinct instances of them), the
newly created activity sets itself as the view delegate (in the
constructor, i.e. too early IMO), and then the current activity is
stopped... and nulls the view delegate! (a good thing IMO)
...and the new activity is no longer the delegate of its view, and
doesn't "fix" this when it starts.
I.e. in the master/detail app generated by Roo (ScaffoldDesktopApp) the
current code only works because there's a CachingActivityMapper. This
mapper is supposed to be (mainly) an optimization, but remove it and the
app breaks.
http://gwt-code-reviews.appspot.com/925801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
