Community
Participate
Working Groups
Build Identifier: Version: July 2010, Build id: {0} If the attached application starts with fresh workspace then everything works as expected. After terminating and restarting the application following error occurred: !ENTRY org.eclipse.osgi 4 0 2010-07-19 11:33:38.771 !MESSAGE Application error !STACK 1 java.lang.NullPointerException at org.eclipse.e4.ui.workbench.renderers.swt.ToolItemRenderer.generateParameterizedCommand(ToolItemRenderer.java:162) at org.eclipse.e4.ui.workbench.renderers.swt.ToolItemRenderer.getToolTipText(ToolItemRenderer.java:201) at org.eclipse.e4.ui.workbench.renderers.swt.ToolItemRenderer.createWidget(ToolItemRenderer.java:245) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createWidget(PartRenderingEngine.java:568) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:388) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:452) at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.processContents(SWTPartRenderer.java:57) at org.eclipse.e4.ui.workbench.renderers.swt.ToolBarRenderer.processContents(ToolBarRenderer.java:114) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:400) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.getToolbar(StackRenderer.java:546) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.showTab(StackRenderer.java:493) at org.eclipse.e4.ui.workbench.renderers.swt.LazyStackRenderer.postProcess(LazyStackRenderer.java:105) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:404) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:452) at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.processContents(SWTPartRenderer.java:57) at org.eclipse.e4.ui.workbench.renderers.swt.WBWRenderer.processContents(WBWRenderer.java:464) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:400) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.createGui(PartRenderingEngine.java:452) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:630) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:589) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:103) at org.eclipse.e4.ui.internal.workbench.swt.E4Application.start(E4Application.java:124) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574) at org.eclipse.equinox.launcher.Main.run(Main.java:1407) at org.eclipse.equinox.launcher.Main.main(Main.java:1383) Reproducible: Always Steps to Reproduce: 1. Start attached application. 2. File -> ShowPart. 3. Terminate application. 4. Restart application.
Created attachment 174599 [details] test.app and test.plugin
When the MPart is being restored, its toolbar contains the handled item with the correct label, but none of the other fields are correct (specifically, command==null). PW
Created attachment 174661 [details] ModelAssembler patch This patch ensure the ID of all elements is copied when the models are merged but there's still a problem in reconciler. When executing the steps one gets an NPE on second start! !ENTRY org.eclipse.e4.ui.workbench 4 2 2010-07-19 21:07:21.729 !MESSAGE Problems occurred when invoking code from plug-in: "org.eclipse.e4.ui.workbench". !STACK 0 java.lang.NullPointerException at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eDynamicUnset(BasicEObjectImpl.java:1216) at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eUnset(BasicEObjectImpl.java:1194) at org.eclipse.e4.ui.model.application.ui.impl.UILabelImpl.eUnset(UILabelImpl.java:234) at org.eclipse.e4.ui.model.application.descriptor.basic.impl.PartDescriptorImpl.eUnset(PartDescriptorImpl.java:609) at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eUnset(BasicEObjectImpl.java:1164) at org.eclipse.e4.ui.internal.workbench.EMFModelDeltaUnset.apply(EMFModelDeltaUnset.java:31) at org.eclipse.e4.ui.internal.workbench.ModelReconcilingService$1.run(ModelReconcilingService.java:59) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.e4.ui.internal.workbench.ModelReconcilingService.applyDeltas(ModelReconcilingService.java:57)
Created attachment 174662 [details] The delta after 1st start
IMHO the XMLReconciler code is wrong because it resolves the incorrect feature (Part#toolbar) whereas it is (PartDescriptor#toolbar). The problem is found IMHO in: ---------8<--------- private static EStructuralFeature getStructuralFeature(EObject object, String featureName) { } ---------8<--------- which only uses the feature name which is NOT correct because the feature you are searching could be a completely different one (I see in your code that you are already testing things like this for other types)! I still think this is NOT a good idea because our model is extensible and you never know which new feature names are added!
I should add also that for me everything worked as expected since the beginning!
Raising priority because the reconciler stuff could affect more of our code and the change in delta processing is also an important one!
Created attachment 174665 [details] minimal patch This is the minimal patch to fix the reconciler - but IMHO it's not a good solution because it works against our model extensibility. My favored patch follows in a second.
Created attachment 174666 [details] favored patch This what I'd favor. I don't think the performance impact is very high - if we think we could naturally add a cache but for now I'd go with this dynamic lookup
(In reply to comment #3) > Created an attachment (id=174661) [details] > patch > > This patch ensure the ID of all elements is copied when the models are merged > but there's still a problem in reconciler. > this patch fixes the problem described in bug 315855
*** Bug 315855 has been marked as a duplicate of this bug. ***
Oleg, can you please review the "patch" in for the ModelAssembler, Remy please review and comment on "favored patch"/"minimal patch". I'd like to bring this in tomorrow!
Remy please review "minimal patch" and "favored patch"
Oleg please review "ModelAssembler patch" you can use the example attached to 315855 to see that it fixes the problem described there.
(In reply to comment #13) > Remy please review "minimal patch" and "favored patch" +1 for the favoured patch.
(In reply to comment #14) > Oleg please review "ModelAssembler patch" you can use the example attached to > 315855 to see that it fixes the problem described there. From what I see the patch does two changes: 1) it does applicationResource.setInternalId() before fragment.merge(); 2) it adds walking through fragment's tree to set IDs for all elements like we do for top elements The (1) is what fixes the problem in the bug 315855 and the example in this bug. The (2) does not seem to have any visible effect. This is EMF magic that I am not qualified to make a quick decision on. From pure common sense, the (2) is what should be making a difference, but it is (1) that actually seems to fix the problem. - Do you have a good understanding why (1) works? - Can we make an example that would be broken without (2) and fixed by it?
Created attachment 174759 [details] Test Patch JUnit Test to see what the different changes do.
(In reply to comment #16) > (In reply to comment #14) > > Oleg please review "ModelAssembler patch" you can use the example attached to > > 315855 to see that it fixes the problem described there. > > > From what I see the patch does two changes: > > 1) it does applicationResource.setInternalId() before fragment.merge(); > 2) it adds walking through fragment's tree to set IDs for all elements like we > do for top elements > > The (1) is what fixes the problem in the bug 315855 and the example in this > bug. The (2) does not seem to have any visible effect. > > This is EMF magic that I am not qualified to make a quick decision on. From > pure common sense, the (2) is what should be making a difference, but it is (1) > that actually seems to fix the problem. (2) doesn't has an effect in the above examples because none of them has had nested contributions (see the test case which does this). > > - Do you have a good understanding why (1) works? I think I've a good understanding finally what's going on (another patch is to follow the old one is not 100% correct). The reason we need to do the ID-setting before the merge is that at the very moment we move elements from contribute-resource to application-resource EMF tries to look up ids and generates one. Adding our own ID to the internal set later on does NOT fix the problems because the super.getID() already steps in. > - Can we make an example that would be broken without (2) and fixed by it? See the attached JUnit-Patch A patch with the final solution will follow in a minute!
Created attachment 174760 [details] ModelAssembler Patch this use the setID() method which also updates the internal references I think we would have got into trouble when not using feature serialization because EMFs internal map would have been different to ours
(In reply to comment #18) > The reason we need to do the ID-setting before the merge is that at the very > moment we move elements from contribute-resource to application-resource EMF > tries to look up ids and generates one. Adding our own ID to the internal set > later on does NOT fix the problems because the super.getID() already steps in. Thank you, that sounds right. +1 to the "ModelAssembler Patch", nicely done!
Boris it's up to you to look at the patches * "favored patch"/" minimal patch": remy +1ed the favored one * "ModelAssembler Patch" you can play with the JUnit to see what's going on Oleg +1ed it
(In reply to comment #21) > Boris it's up to you to look at the patches > * "favored patch"/" minimal patch": remy +1ed the favored one > * "ModelAssembler Patch" you can play with the JUnit to see what's going on > Oleg +1ed it +1 for "favored patch" +1 for "ModelAssembler Patch"
Thanks for reviewing - changes are released to HEAD
marking as fixed
*** Bug 320789 has been marked as a duplicate of this bug. ***
