Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 324228 - Part's context not reparented by renderer even if it has other valid placeholders pointing at it
Summary: Part's context not reparented by renderer even if it has other valid placehol...
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 1.0   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 4.1 M2   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-01 13:10 EDT by Remy Suen CLA
Modified: 2010-09-14 14:06 EDT (History)
0 users

See Also:


Attachments
PartRenderingEngineTests tests patch v1 (5.25 KB, patch)
2010-09-07 08:41 EDT, Remy Suen CLA
no flags Details | Diff
PartRenderingEngineTests tests patch v2 (7.38 KB, patch)
2010-09-10 13:18 EDT, Remy Suen CLA
no flags Details | Diff
Reparent the part's context as well as its widget (1.60 KB, patch)
2010-09-14 10:55 EDT, Eric Moffatt CLA
no flags Details | Diff
PartRenderingEngineTests tests patch v3 (11.81 KB, patch)
2010-09-14 12:37 EDT, Remy Suen CLA
no flags Details | Diff
Revised patch to handle sharing of non-context elements (3.25 KB, patch)
2010-09-14 13:48 EDT, Eric Moffatt CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Suen CLA 2010-09-01 13:10:42 EDT
1. Window > Open Perspective > Debug
2. Window > Show View > Other... > Java > Package Explorer
3. Window > Reset Perspective

You cannot go back to the 'Java' perspective. Well, you can, it'll just show the same stuff as what's in the 'Debug' perspective. Attempting to reset it will not get you anywhere.

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:466)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:403)
	at org.eclipse.swt.widgets.Menu.getItems(Menu.java:477)
	at org.eclipse.swt.widgets.Menu.fixMenus(Menu.java:349)
	at org.eclipse.swt.widgets.Decorations.fixDecorations(Decorations.java:276)
	at org.eclipse.swt.widgets.Control.fixChildren(Control.java:2117)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:452)
	at org.eclipse.swt.widgets.Tree.fixChildren(Tree.java:1150)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:455)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:455)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:455)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:455)
	at org.eclipse.swt.widgets.Composite.fixChildren(Composite.java:455)
	at org.eclipse.swt.widgets.Control.setParent(Control.java:3953)
	at org.eclipse.e4.ui.workbench.renderers.swt.PerspectiveStackRenderer.showTab(PerspectiveStackRenderer.java:106)
	at org.eclipse.e4.ui.workbench.renderers.swt.LazyStackRenderer$1.handleEvent(LazyStackRenderer.java:75)
	at org.eclipse.e4.ui.services.internal.events.UIEventHandler.handleEvent(UIEventHandler.java:41)
	at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:188)
	at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:198)
	at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:227)
	at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:149)
	at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:139)
	at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:78)
	at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:39)
	at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:73)
	at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:58)
	at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:380)
	at org.eclipse.e4.ui.model.application.ui.advanced.impl.PerspectiveStackImpl.setSelectedElement(PerspectiveStackImpl.java:134)
	at org.eclipse.e4.ui.model.application.ui.advanced.impl.PerspectiveStackImpl.setSelectedElement(PerspectiveStackImpl.java:1)
	at org.eclipse.e4.ui.internal.workbench.ModelServiceImpl.removePerspectiveModel(ModelServiceImpl.java:624)
	at org.eclipse.ui.internal.WorkbenchPage.closePerspective(WorkbenchPage.java:1052)
	at org.eclipse.ui.internal.WorkbenchPage.resetPerspective(WorkbenchPage.java:2150)
	at org.eclipse.ui.internal.ResetPerspectiveAction.run(ResetPerspectiveAction.java:59)
	at org.eclipse.ui.internal.PerspectiveAction.run(PerspectiveAction.java:70)
Comment 1 Remy Suen CLA 2010-09-07 08:41:17 EDT
Created attachment 178313 [details]
PartRenderingEngineTests tests patch v1

While a shared part has its widget reparented if it still has valid placeholders pointing at it, its context is not. Hence, when the perspective is disposed, any shared parts it's currently showing also gets disposed (in the example, the 'Package Explorer' gets destroyed).
Comment 2 Remy Suen CLA 2010-09-10 13:18:55 EDT
Created attachment 178638 [details]
PartRenderingEngineTests tests patch v2
Comment 3 Remy Suen CLA 2010-09-13 08:51:26 EDT
(In reply to comment #2)
> Created an attachment (id=178638) [details]
> PartRenderingEngineTests tests patch v2

The testRemoveGuiBug324228_B could be deemed incorrect.

The question we need to ask is as follows:
Consider a shared part that is in two _rendered_ perspectives, say, the 'Outline' view in both the 'Java' and 'Debug' perspectives. The context of the part is parented to the perspectives based on which perspective is showing, so far so good. So suppose the 'Debug' perspective is currently the active perspective and the user closes the 'Outline' view. Should the part's context now be parented off of the 'Java' perspective or should it stick with the 'Debug' perspective?
Comment 4 Eric Moffatt CLA 2010-09-14 10:55:11 EDT
Created attachment 178840 [details]
Reparent the part's context as well as its widget
Comment 5 Remy Suen CLA 2010-09-14 11:24:34 EDT
(In reply to comment #4)
> Created an attachment (id=178840) [details]
> Reparent the part's context as well as its widget

The tests are all green for me. Since we perform a null/disposal check in that block of code anyway, I think we might as well stick the context reparenting code within that if block for safety.
Comment 6 Remy Suen CLA 2010-09-14 12:37:10 EDT
Created attachment 178852 [details]
PartRenderingEngineTests tests patch v3

Two new tests that will fail.
Comment 7 Eric Moffatt CLA 2010-09-14 13:48:55 EDT
Created attachment 178857 [details]
Revised patch to handle sharing of non-context elements


This patch also removes an erroneous test in the LSR's 'processContents'...
Comment 8 Eric Moffatt CLA 2010-09-14 13:50:55 EDT
Committed in >20100914. Applied the patch.
Comment 9 Eric Moffatt CLA 2010-09-14 14:06:00 EDT
Committed in >20100914. Checked in the tests.