shoes4 icon indicating copy to clipboard operation
shoes4 copied to clipboard

RedrawingAspect ownership and cleanup

Open jasonrclark opened this issue 11 years ago • 1 comments

Ran into a problem with some order dependent test failures (https://travis-ci.org/shoes/shoes4/jobs/29270977) and tracked them down to https://github.com/shoes/shoes4/blob/master/spec/swt_shoes/integration_spec.rb leaving around a live RedrawingAspect that the later styling tests were failing because of.

As I looked at this further, my first instinct was to have Shoes::Swt::App#quit call to simply remove callbacks from its @redrawing_aspect, but then I realized that isn't right currently because the after_do callbacks are global but RedrawingAspect instances are scoped to individual "apps".

Which leads to the real issue here, which is that spawning multiple windows using Shoes.app, you end up getting RedrawingAspect events for other windows' element that don't belong to you :grimacing:

In a practical sense, this probably won't break things but is definitely a perf issue if you're using multiple windows, and also makes things like clearing state with the tests hard. It also "forced" the hack to clean up after our integration spec in ce050722432e1a7649d8f15b158f16fe07392b54.

I don't know what solution would be best. Maybe there's just one RedrawingAspect but it's aware of the relationship between apps and elements to dispatch to? Or maybe there's some way for an individual app instance to only remove it's hooks (ref-counting at some level so we don't remove them entirely from after_do until everyone's done)?

jasonrclark avatar Jul 06 '14 23:07 jasonrclark

A similar issue is already filed at #385 - e.g. to just redraw for the own app. That is definitely possible (or well I believe so) if the aspect checks which element this redrawn originated from. Or well takes the app of the element that threw this... some guard statements whatever. We'd need a unified way to access the current app from all cases where the aspect is invoked though.

Removing the callbacks should not be hard but there should probably be just one callback that then decides itself wich app to redraw. If the last app quits then I'd guess the process quits as well cleaning stuff up... but we can not assume that you are right - good point about App#quit.

I hope that I'm making sense. Don't feel like I do right now (too much writing + heat).

PragTob avatar Jul 09 '14 10:07 PragTob