#windowIsClosing not sent to model of WindowModel
PharoLauncher is currently leaking AnnouncementSubscriptions when its window closes. I'm not sure with Spec how to ensure actions are performed when the window is closed, but I have been able to achieve this with the following modification [1] which is... ----- WindowModel>>windowIsClosing isClosedHolder value: true self model windowIsClosing "<----proposed modification" ----- I investigated with a halt in SpecWindow>>close to trace through and discover that it calls... SpecWindow(StandardWindow)>>delete, which in my non-fullscreen-case calls... SpecWindow(SystemWindow)>>delete, which "model windowIsClosing" calls... MorphicWindowAdaptor>>windowIsClosing, which "self model windowIsClosing" calls... WindowModel>>windowIsClosing, which does only "isClosedHolder value: true" That last line happens to hold aPharoLauncher in its 'model' instance variable, so with the above proposal I can implement PharoLauncher>>windowIsClosing to unregister the announcer. Any problem expected with that modification? [1] https://pharo.fogbugz.com/f/cases/12677 cheers -ben
I think i would be more elegant that the model register to the isClosedHolder changes :) Ben On 18 Jan 2014, at 12:09, btc@openinworld.com wrote:
PharoLauncher is currently leaking AnnouncementSubscriptions when its window closes. I'm not sure with Spec how to ensure actions are performed when the window is closed, but I have been able to achieve this with the following modification [1] which is... ----- WindowModel>>windowIsClosing isClosedHolder value: true self model windowIsClosing "<----proposed modification" -----
I investigated with a halt in SpecWindow>>close to trace through and discover that it calls... SpecWindow(StandardWindow)>>delete, which in my non-fullscreen-case calls... SpecWindow(SystemWindow)>>delete, which "model windowIsClosing" calls... MorphicWindowAdaptor>>windowIsClosing, which "self model windowIsClosing" calls... WindowModel>>windowIsClosing, which does only "isClosedHolder value: true"
That last line happens to hold aPharoLauncher in its 'model' instance variable, so with the above proposal I can implement PharoLauncher>>windowIsClosing to unregister the announcer. Any problem expected with that modification?
[1] https://pharo.fogbugz.com/f/cases/12677
cheers -ben
Hi Ben,
I've gave that a go and had some success, but it raised an architectural question I ask near the end. I've documented my investigation here in case it is of use to others new to Spec or the Spec documentation being developed.
Spec documentation is actually under heavy construction :) May I keep this content for further quotation?
So the question this leads up to is... Should child ComposableModels all reference the same WindowModel as their owner?
The thing is, the isnât var window doesnât point to the same, but the method `window` does :) Could you try something like: PhLTitledTreeModel>>repository: aRepository repositoryHolder value: aRepository. self repository whenChangedSend: #refresh to: self. "<--subscription-of-concern" self refresh. self whenBuiltDo: [ self window whenClosed: [ self repository unsubscribe: self ] ] Thanks for your investigation :) Ben
On 20 Jan 2014, at 13:51, btc@openinworld.com wrote:
Sure thing. That was part of my original intention.
Cool :) Thanks
When I try... self whenBuiltDo: [ self halt. self window whenClosedDo: [ self halt. self repository unsubscribe: self ] ]. none of the halts occur.
However I got what I needed with the followingâ¦
did you try to register in the initialize method ? Because if you register once the application is already open, it may not work :)
---------- ComposableModel>>topWindowHolder ^ owner ifNil: [ window ] ifNotNil: [ :o | o topWindowHolder ]. ---------- ComposableModel>>whenWindowClosed: aBlock self topWindowHolder value ifNotNil: [ :w | w whenClosedDo: aBlock ] ifNil: [ self topWindowHolder whenChangedDo: [ :w | w whenClosedDo: aBlock ] ]. ---------- PhLTitledTreeModel>>repository: aRepository self assert: self repository isNil description: 'Changing the repository is not allowed because we would have to change the context as well'. repositoryHolder value: aRepository. self repository whenChangedSend: #refresh to: self. self whenWindowClosed: [ self repository unsubscribe: self. self inform: 'Works for composed sub-items'. ] . self refresh ---------- PharoLauncher>>initialize super initialize. self whenWindowClosed: [ self inform: 'Works for top level owner' ]. ---------- And optionally you could now have... ComposableModel>>window ^ self topWindowHolder value âââââ
topWindowHolder sounds a bit hackish :) I think this can be achieved with the whenBuiltDo:, I should investigate :)
If those ComposableModel additions are okay, I'll submit a slice on Case 12677 (renamed to "ComposableModel subcomponents need to act on window close") https://pharo.fogbugz.com/f/cases/12677/
You can also submitted here: https://github.com/SpecForPharo/spec if you want :) (and become a Spec contributor :P) Ben
On 20 Jan 2014, at 14:37, btc@openinworld.com wrote:
I don't want to unsubscribe #refresh in PhLTitledTreeModel>>initialize. I _really_ want to unsubscribe #refresh immediately after the subscribe #refresh is set up.
But it does not prevent you to register to the event whenever you want :) Could you give a try and tell me?
I tried to avoid calling in #windowHolder but would that preferred? Do you have another suggestion?
I was not talking about the name, more the implementation :P (but not more than #window :) )
btw, the 'window' i-var of ComposableModel seems like it should be 'windowHolder'
I think all the other variable should be name without holder at the end :) Ben
On 23 Jan 2014, at 12:53, btc@openinworld.com wrote:
I gave this a try and indeed #whenBuiltDo: works from #initialize. However in PharoLauncher <Create Image> stopped updating the list. After a bit of a muddling around I found that I am prevented from registering the event "wherever I wanted", since the subscription does not go to theValueHolder but to the contents of theValueHolder.
That sounds strange :(
That is as far as I can push this for now. I have a working solution with previously proposed ComposableModel>>topWindowHolder and ComposableModel>>whenWindowClosed: so I will proceed with that. Any objections to pushing these into Pharo 3 at this late stage :) and I'll just make them extensions from PharoLauncher.
If you submit a slice, I will review it :) Note that you can also do it on top of the bleeding edge Spec[1] Ben [1]https://github.com/SpecForPharo/spec
cheers -ben
I tried to avoid calling in #windowHolder but would that preferred? Do you have another suggestion?
I was not talking about the name, more the implementation :P (but not more than #window :) )
btw, the 'window' i-var of ComposableModel seems like it should be 'windowHolder'
I think all the other variable should be name without holder at the end :)
Ben
participants (2)
-
Benjamin -
btc＠openinworld.com