Community
Participate
Working Groups
Build Identifier: I20100507 We are trying to have API compatibility between our Eclipse 3.4 code and Eclipse 3.6 code. There is a change in the initialization sequence in GEF org.eclipse.draw2d.Polyline.java. This change results in an invalid access to not-yet-initialized field in subclass our ***Decoration subclass. When a relationship gets decorated, the Java class ***Decoration.java needs to be initialized, before any of its field variables/methods can be invoked. When that happens, its superclass Polyline.java needs to be initialized first, but that "super"-initialization indirectly calls back on the method getBounds() which we override in the subclass ***Decoration.java. Since at this time the subclass ***Decoration.java has not even had a chance to initialize, its fields are all null -- hence we end up with a NPE Reproducible: Always
I am not seeing any changes to the initialization sequence in Polyline. You can going to have to elaborate further which exact method you are having issues with. Polyline was last changed in 2008 for Bug 238874 .
(In reply to comment #1) > I am not seeing any changes to the initialization sequence in Polyline. You can > going to have to elaborate further which exact method you are having issues > with. > > Polyline was last changed in 2008 for Bug 238874 . Hi Anthony, It seems that the regression came from "[238874] gef-head Alex.Shatalin 081017 Create ScalablePolygonShape figure" when Polyline did override repaint and added a bounds = null statement. As the bounds is now null during the init of polyline we will end up figuring out the points though our decorator is not fully initialized. Let me know if this clarifies. ~Loic
(In reply to comment #2) > (In reply to comment #1) > > I am not seeing any changes to the initialization sequence in Polyline. You can > > going to have to elaborate further which exact method you are having issues > > with. > > > > Polyline was last changed in 2008 for Bug 238874 . > > Hi Anthony, > > It seems that the regression came from "[238874] gef-head Alex.Shatalin 081017 > Create ScalablePolygonShape figure" when Polyline did override repaint and > added a bounds = null statement. As the bounds is now null during the init of > polyline we will end up figuring out the points though our decorator is not > fully initialized. > > Let me know if this clarifies. > ~Loic So if we do not throw NPE and allow to continue, does that solve the problem?
Why is this in Polyline?: public void repaint() { bounds = null; super.repaint(); }
Looks like accidental change... i.e. a parameter change method affecting the bounds didn't call bounds = null, but called repaint(). Then bounds = null have been added, but repaint() override wasn't removed.
(In reply to comment #4) > Why is this in Polyline?: > > public void repaint() { > bounds = null; > super.repaint(); > } So we all agree that this method should be removed in 3.6.1? Please confirm and I can change before RC3.
Created attachment 178353 [details] Patch to fix the issue. Can you review this patch for 3.6.1?
Looks good to me.
Committed to HEAD and R3_6_maintenance for 3.5.1 (Helios SR1).
Just to cross reference ... we found the fix here caused a hang in the WSDL editor in WTP (and a smaller regression in XSD editor). See bug 324884. We can probably work around ... but not sure if it'd effect other graphical editors?
Here's the link to the old version of Polyline which was working correctly: http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.gef/plugins/org.eclipse.draw2d/src/org/eclipse/draw2d/Polyline.java?view=markup&revision=1.25&root=Tools_Project Someone found a couple of places where bounds were being set to null, and then repaint() was called, and decided to move the bounds=null into repaint, and also *delete* bounds=null from those callers of repaint(). (Which was wrong, since not every caller of repaint() needs bounds recalculated) To fix this, you also have to add "bounds=null" back everywhere that it used to be: addPoint insertPoint removeAllPoints removePoint setLineWidth setPoint setPoints And possibly new API added since, like: setLineAttributes
This fix regressed caused our WTP WSDL Editor to hang the workbench. See bug 324884 for details. Also, it regressed the drag and drop feedback in our WTP XSD Editor. The feedback does not appear during dragging. I will attach screen caps to show this.
Created attachment 178624 [details] Drag feedback prior to this fix.
Created attachment 178625 [details] Drag feedback with this fix.
AbstractPointListShape doesn't null the bounds for some reason... I thought it does... should have looked first. Yes, looks like we have to override all those methods or move the bounds caluulation logic and nullification of the bounds to AbstractPointListShape.
Hi again, For RC4 and GEF 3.6.1 in Helios SR1 next week, I am going to restore the code to what was delivered in GEF 3.6 for Helios GA. I am not sure what we should consider doing for GEF 3.6.2 in Helios SR2, if anything, but the status is this. 1) A team requested a feature *very* early in GEF 3.5 via a patch and it was delivered in 3.5 M3. 2) Nobody complained for two GEF releases, well over two years. 3) An IBM team complained that the change broke their stuff, escalated the issue requiring the need to look at the issue (IBM pays my salary). 4) We tried to fix and broke other stuff and in addition got more complaints that the original change two years ago may be invalid. I am of the opinion that the change is in, the changes never broke my application or any of the GEF examples I have to work with and we leave the changes as is.
> I am of the opinion that the change is in, the changes never broke my > application or any of the GEF examples I have to work with and we leave the > changes as is. Hi Anthony, I tried most if not all the GEF examples that I could find. As far as I could tell, none use Polygon or Polyline.
(In reply to comment #16) > Hi again, For RC4 and GEF 3.6.1 in Helios SR1 next week, I am going to restore > the code to what was delivered in GEF 3.6 for Helios GA. I have rolled back the change in HEAD and R3_6_maintenance for 3.5.1 (Helios SR1).
(In reply to comment #18) > I have rolled back the change in HEAD and R3_6_maintenance for 3.5.1 (Helios > SR1).
