RadicalFx / Radical

Radical is an infrastructure framework whose primary role is to help in the development of composite WPF applications based on the Model View ViewModel pattern.
http://www.radicalframework.com/
MIT License
38 stars 3 forks source link

Incorrect handling of regions de-registration #7

Closed mauroservienti closed 10 years ago

mauroservienti commented 10 years ago

Ho trovato problemi con la UnregisterRegion e le View singleton (Main* e Shell*):

1 - La seconda volta che apro una window istanziata in singleton contenente una region, mi becco questo errore:

image

2 - Se inietto in una region una view singleton che contiene un'altra region, dalla seconda volta che la riapri, non da eccezione, ma non crea una nuova istanza della view da iniettare e neanche la dispose quando viene "chiusa" (la vista nel singleton è iniettata con InjectViewInRegion):

inietto per la PRIMA volta la view:

MainSingletonWithNestedRegionsViewModel() FirstViewModel.OnViewClosing(CancelEventArgs e) FirstViewModel.OnViewClosed() !!! FirstViewModel.Dispose(bool disposing) AmazingNestedViewModel()

ritorno per la PRIMA volta alla view precedente:

FirstViewModel(IMessageBroker broker) MainSingletonWithNestedRegionsViewModel.OnViewClosing(CancelEventArgs e) AmazingNestedViewModel.OnViewClosed() !!! AmazingNestedViewModel.Dispose(bool disposing) MainSingletonWithNestedRegionsViewModel.OnViewClosed()

inietto per la SECONDA volta la view:

FirstViewModel.OnViewClosing(CancelEventArgs e) FirstViewModel.OnViewClosed() !!! FirstViewModel.Dispose(bool disposing)

ritorno per la SECONDA volta alla view precedente:

FirstViewModel(IMessageBroker broker) MainSingletonWithNestedRegionsViewModel.OnViewClosing(CancelEventArgs e) MainSingletonWithNestedRegionsViewModel.OnViewClosed()

3 - Se il punto 2 invece che implementarlo con InjectViewInRegion in AmazingNestedView, lo implemento iniettando la view da codice mi da la stessa eccezione del punto 1

Il concetto di avere una View singleton che può essere visibile o meno, non è proprio uno scenario sensato, perchè le Main e le Shell da quello che ho capito sono state introdotte sostanzialmente per dare la possibilità di creare appunto la main shell dell'applicazione che è sempre presente e sempre visibile, ma alla fine dato che è una cosa che si può fare forse sarebbe il caso comunque di gestire la casistica.

Repro: http://1drv.ms/1oT7X5A

mauroservienti commented 10 years ago

Separando in pezzi:

1) non riesco a riprodurlo; 2) è ovvio... :-\ quando sulla view singletong chiamiamo Release siccome è Singleton Catle semplicemente la ignora e quindi non viene mai chiamato Dispose, mentre rilasciando esplicitamente quella nested il Dispose viene correttamente chiamato, ma questo porta ad un interessante scenario:

Non mi vengono molte idee se non: Se una view è Singleton non facciamo la release, ne di lei ne di tutte quelle nested, in questo modo seguiamo la stessa filosofia dei container se hai un componente singleton che ha delle dipendenze Transient stai sbagliando qualcosa a prescindere;

Commenti?

mauroservienti commented 10 years ago

1) riprodotto adesso.

micdenny commented 10 years ago

Sono d'accordo con la filosofia della risposta al punto 2: "Se una view è Singleton non facciamo la release, ne di lei ne di tutte quelle nested, in questo modo seguiamo la stessa filosofia dei container se hai un componente singleton che ha delle dipendenze Transient stai sbagliando qualcosa a prescindere" +1

mauroservienti commented 10 years ago

Ho committato la fix, abbiamo comunque un interessante problema. Che direi essere indipendente da noi:

Idee? l'unica che mi viene è che ci implementiamo un LifestyleManager custom che sa cosa fare con le "View" che sono Singleton e quindi le rilascia correttamente quando vengono chiuse mentre non le rilascia se vengono nascoste.

Altra soluzione è impedire che ci possano essere più view Singleton oltre quella che fa partire l'applicazione.

mauroservienti commented 10 years ago

http://www.nuget.org/packages/Radical.Windows.Presentation/1.0.7-Alfa1

mauroservienti commented 10 years ago

Per ora lo chiudo, i test fatti fino ad ora sembrano confermare che funziona correttamente, ho pushato tutto in develop e pubblicato su Nuget. Lascio la branch così possiamo eventualmente andare avanti a fare modifiche.

.m

micdenny commented 10 years ago

Si ho fatto qualche prova e senza nuove window tutto ok, funziona correttamente, tra l'altro la seconda volta che visualizzo la vista singleton, la vista iniettata transient precedente viene rilasciata, perchè viene chiamata la release nel momento in cui Region.Content cambia, quindi tutto questo è ok.

Invece se apro la seconda volta una Window definita singleton (Main* Shell*), quando provo a riaprirla adesso ricevo un eccezione che però non possiamo risolvere, dato che giustamente una window chiusa non può essere riaperta, quindi questo sarebbe un errore dello sviluppatore e non credo possiamo mettere alcuna pezza:

System.InvalidOperationException was unhandled by user code
  HResult=-2146233079
  Message=Cannot set Visibility or call Show, ShowDialog, or WindowInteropHelper.EnsureHandle after a Window has closed.
  Source=PresentationFramework
  StackTrace:
       at System.Windows.Window.VerifyCanShow()
       at System.Windows.Window.Show()
       at RadicalViewModelDispose.Messaging.Handlers.OpenViewInSingletonPopupMessageHandler.Handle(Object sender, OpenViewInSingletonPopupMessage message) in c:\Samples\RadicalViewModelDispose\RadicalViewModelDispose\Messaging\Handlers\OpenViewInSingletonPopupMessageHandler.cs:line 36
       at Topics.Radical.Messaging.AbstractMessageHandler`1.Handle(Object sender, Object message) in c:\Development\Projects\Radical\src\net35\Radical\Messaging\AbstractMessageHandler (Generic).cs:line 32
       at Castle.Facilities.SubscribeToMessageFacility.<>c__DisplayClass10.<Subscribe>b__d(Object s, Object msg) in c:\Development\Projects\Radical\trunk\src\net35\Radical.Extensions.Castle\Facilities\SubscribeToMessageFacility.cs:line 131
  InnerException: