Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 348215

Summary: Reattaching a detached stack with multiple views will cause part disposal
Product: [Eclipse Project] e4 Reporter: Remy Suen <remy.suen>
Component: UIAssignee: Project Inbox <e4.ui-inbox>
Status: VERIFIED FIXED QA Contact: Eric Moffatt <emoffatt>
Severity: critical    
Priority: P3 CC: pwebster
Version: 1.0Flags: remy.suen: review+
Target Milestone: 4.1 RC4   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
When reparenting a widget in createGui also reparent the contexts
none
Tests patch
none
Revised patch that tests for direct part reparenting as well none

Description Remy Suen CLA 2011-06-03 10:35:21 EDT 
1. Detach a view.
2. Drag another view to that new stack.
3. Use stack dragging to drag the whole thing back inside the workbench window.
4. Disposals happen.

Thread [main] (Suspended (breakpoint at line 322 in CompatibilityPart))	
	CompatibilityView(CompatibilityPart).destroy() line: 322	
	NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]	
	NativeMethodAccessorImpl.invoke(Object, Object[]) line: 48	
	DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 37	
	Method.invoke(Object, Object...) line: 600	
	MethodRequestor.execute() line: 56	
	InjectorImpl.processAnnotated(Class<Annotation>, Object, Class<?>, PrimaryObjectSupplier, PrimaryObjectSupplier, ArrayList<Class<?>>) line: 828	
	InjectorImpl.processAnnotated(Class<Annotation>, Object, Class<?>, PrimaryObjectSupplier, PrimaryObjectSupplier, ArrayList<Class<?>>) line: 808	
	InjectorImpl.disposed(PrimaryObjectSupplier) line: 355	
	FieldRequestor(Requestor).disposed(PrimaryObjectSupplier) line: 112	
	ContextObjectSupplier$ContextInjectionListener.update(IEclipseContext, int, Object[]) line: 63	
	TrackableComputationExt.update(ContextChangeEvent) line: 88	
	TrackableComputationExt.doHandleInvalid(ContextChangeEvent, List<Scheduled>) line: 53	
	TrackableComputationExt(Computation).handleInvalid(ContextChangeEvent, List<Scheduled>) line: 53	
	EclipseContext.dispose() line: 177	
	EclipseContext.dispose() line: 165	
	PartRenderingEngine.clearContext(MContext) line: 824	
	PartRenderingEngine.safeRemoveGui(MUIElement) line: 805	
	PartRenderingEngine.access$3(PartRenderingEngine, MUIElement) line: 728	
	PartRenderingEngine$8.run() line: 723	
	SafeRunner.run(ISafeRunnable) line: 42	
	PartRenderingEngine.removeGui(MUIElement) line: 708	
	PartRenderingEngine$1.handleEvent(Event) line: 137	
	UIEventHandler$1.run() line: 41	
	UISynchronizer(Synchronizer).syncExec(Runnable) line: 180	
	UISynchronizer.syncExec(Runnable) line: 150	
	Display.syncExec(Runnable) line: 4683	
	E4Application$1.syncExec(Runnable) line: 182	
	UIEventHandler.handleEvent(Event) line: 38	
	EventHandlerWrapper.handleEvent(Event, Permission) line: 197	
	EventHandlerTracker.dispatchEvent(EventHandlerWrapper, Permission, int, Event) line: 197	
	EventHandlerTracker.dispatchEvent(Object, Object, int, Object) line: 1	
	EventManager.dispatchEvent(Set<Entry<K,V>>, EventDispatcher<K,V,E>, int, E) line: 230	
	ListenerQueue<K,V,E>.dispatchEventSynchronous(int, E) line: 148	
	EventAdminImpl.dispatchEvent(Event, boolean) line: 135	
	EventAdminImpl.sendEvent(Event) line: 78	
	EventComponent.sendEvent(Event) line: 39	
	EventBroker.send(String, Object) line: 81	
	UIEventPublisher.notifyChanged(Notification) line: 58	
	TrimmedWindowImpl(BasicNotifierImpl).eNotify(Notification) line: 380	
	TrimmedWindowImpl(UIElementImpl).setToBeRendered(boolean) line: 290	
	CleanupAddon$3$1.run() line: 273	
	RunnableLock.run() line: 35	
	UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 135	
	Display.runAsyncMessages(boolean) line: 4140	
	Display.readAndDispatch() line: 3757	
	DnDManager.update() line: 243	
	DnDManager$3.handleEvent(Event) line: 111	
	EventTable.sendEvent(Event) line: 84	
	Display.filterEvent(Event) line: 1262	
	CTabFolder(Widget).sendEvent(Event) line: 1052	
	Display.runDeferredEvents() line: 4165	
	Display.readAndDispatch() line: 3754	
	PartRenderingEngine$9.run() line: 944	
	Realm.runWithDefault(Realm, Runnable) line: 332	
	PartRenderingEngine.run(MApplicationElement, IEclipseContext) line: 860	
	E4Workbench.createAndRunUI(MApplicationElement) line: 87	
	Workbench$3.run() line: 542	
	Realm.runWithDefault(Realm, Runnable) line: 332	
	Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 522	
	PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149	
	IDEApplication.start(IApplicationContext) line: 123
Comment 1 Eric Moffatt CLA 2011-06-03 13:06:56 EDT 
Created attachment 197319 [details]
When reparenting a widget in createGui also reparent the contexts


This also fixes bug 348200 & bug 348209, perhaps we should mark them DUPS of this one...

I chose this bug to own the patch since the others are just manifestations of the context disposal...

The root cause is that parts discreetly dragged into the DW (or opened in the DW) had their 'parentContext' set to the DW's MTrimmedWindow's context which was correctly getting disposed after the stack was dragged out of it...
Comment 2 Remy Suen CLA 2011-06-03 13:33:40 EDT 
(In reply to comment #1)
> Created attachment 197319 [details]
> When reparenting a widget in createGui also reparent the contexts

This patch won't work if you try to move a nested structure that's two or more levels deep (remove a part sash container with a part stack with a part and then add it back to the main window).

The bugs that have been reported do get fixed though because all those moves are only one level deep (a part stack with parts is moved).

Please open a new bug so that that failure case can be fixed in 4.2.
Comment 3 Eric Moffatt CLA 2011-06-03 13:35:26 EDT 
Created attachment 197320 [details]
Tests patch


Both these tests fail without the original patch and succeed with it...
Comment 4 Eric Moffatt CLA 2011-06-03 13:40:56 EDT 
I've added bug 348250 to capture need to revisit this in 4.2.
Comment 5 Remy Suen CLA 2011-06-03 13:42:11 EDT 
(In reply to comment #3)
> Created attachment 197320 [details]
> Tests patch

Confirmed that the tests fail without the patch and pass with the patch applied.
Comment 6 Eric Moffatt CLA 2011-06-03 14:00:45 EDT 
Created attachment 197321 [details]
Revised patch that tests for direct part reparenting as well
Comment 7 Eric Moffatt CLA 2011-06-03 14:42:22 EDT 
Committed in >20110603. Applied both the patches.
Comment 8 Remy Suen CLA 2011-06-06 09:34:13 EDT 
*** Bug 348209 has been marked as a duplicate of this bug. ***
Comment 9 Eric Moffatt CLA 2011-06-06 10:32:31 EDT 
Verified in I20110604-2201