Bug 320243

Comment 1 Eric Moffatt 2010-07-21 16:38:18 EDT

Created attachment 174919 [details]
Ongoing work for trim stack handling (not ready for review)

Comment 2 Eric Moffatt 2010-07-22 14:44:14 EDT

Created attachment 175010 [details]
Patch to cover off edge conditions


Another scenario to test that this patch fixes:

Minimize a stack and select one of the icons to open the fast view

*Using the Menu* select Window->Close Perspective
 -or-
Hit Ctrl-3 and execute the close perpective *menu* item (the other one doesn't seem to work).

Comment 3 Boris Bokowski 2010-07-22 16:44:09 EDT

Susan, can you please spend some time testing this, too?

The changes are kind of scary but seem to fix the problems. +1

Comment 4 Susan McCourt 2010-07-22 18:34:40 EDT

*** Bug 320321 has been marked as a duplicate of this bug. ***

Comment 5 Susan McCourt 2010-07-22 18:35:14 EDT

this fix also fixes the "chunk" problem in bug 320321

Comment 6 Remy Suen 2010-07-22 18:43:44 EDT

1. Window > New Window
2. Minimize the 'Package Explorer'.
3. Click the icon on the trim so it shows up.
4. Alt+F4 to close the new window. The key here is that the 'Package Explorer' is still up.
5. Try to do anything in the original window, you can't and are permanently screwed.

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4083)
	at org.eclipse.swt.SWT.error(SWT.java:3998)
	at org.eclipse.swt.SWT.error(SWT.java:3969)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:468)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:340)
	at org.eclipse.swt.widgets.Control.getParent(Control.java:1377)
	at org.eclipse.e4.ui.workbench.addons.minmax.TrimStack$2.handleEvent(TrimStack.java:84)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1253)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1052)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4066)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3657)

Without the patch, you get some errors on workbench window activation but those just popup once and then they leave you alone.

Comment 7 Eric Moffatt 2010-07-22 19:34:39 EDT

Created attachment 175027 [details]
Patch that safes up the pane's disposal


This appears to be a low-level implementation issue; we're lacking some system hooks that will allow us to pick up these events -or- we're not listening to the model's 'widget' att enough (it should always go null on a dispose).

In any case the question is whether or not this patch is better than what we have...

Comment 8 Boris Bokowski 2010-07-22 19:44:33 EDT

Created attachment 175028 [details]
updated patch

I added some more disposed checks to avoid the error case described by Remy.

Comment 9 Boris Bokowski 2010-07-22 19:46:51 EDT

(In reply to comment #7)
> Created an attachment (id=175027) [details]
> Patch that safes up the pane's disposal

Oops, I didn't see your attachment when I uploaded mine, and marked it as obsolete by accident.

I can try to compare my changes and yours later tonight.

Comment 10 Eric Moffatt 2010-07-22 19:59:16 EDT

Committed in >20100722. Applied the patch. 

I'll mark this as FIXED, open a new defect unless this one has a major regression...

Comment 11 Eric Moffatt 2010-07-22 20:00:26 EDT

That's what you get for not being on IRC...you can check my patch if you like but you'll have to commit the changes yourself...

Comment 12 Susan McCourt 2010-07-22 21:08:51 EDT

adding a belated +1 for bookkeeping (is Boris going to run one of those "bugs that got released that weren't reviewed reports?)  ;-)

I was about halfway through testing and had to drop...

Comment 13 Eric Moffatt 2010-07-24 18:01:31 EDT

Marking as fixed, re-open only if it's a regression against the scenarios on this bug.

Comment 14 Eric Moffatt 2010-07-27 14:39:01 EDT

Verified on XP in I20100726-2152.