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

Bug 366605

Summary:	[Compatibility] Failures in LeakTestSuite
Product:	[Eclipse Project] Platform	Reporter:	Remy Suen <remy.suen>
Component:	UI	Assignee:	Remy Suen <remy.suen>
Status:	RESOLVED FIXED	QA Contact:	Remy Suen <remy.suen>
Severity:	normal
Priority:	P3	CC:	tom.schindl
Version:	4.2
Target Milestone:	4.2 M5
Hardware:	All
OS:	All
Whiteboard:
Bug Depends on:
Bug Blocks:	366451, 381873

Description Remy Suen

2011-12-13 14:10:08 EST

We have three failures here.

-testSimpleViewLeak
-testBug2555005SiteLeak
-testBug265449PropertiesLeak

Comment 1 Remy Suen

2011-12-13 15:16:43 EST

(In reply to comment #0)
> -testSimpleViewLeak
> -testBug265449PropertiesLeak

Fixed.

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2f36b46905cbc7fefebe121ad272532e226064e5

Comment 2 Remy Suen

2011-12-13 15:36:00 EST

(In reply to comment #1)
> (In reply to comment #0)
> > -testSimpleViewLeak
> > -testBug265449PropertiesLeak

Correction. testBug2555005SiteLeak was fixed.

testBug265449PropertiesLeak is now fixed also.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=fbf0e39912eed245b339765f927f4c00de16170d

Comment 3 Thomas Schindl

2011-12-13 18:07:31 EST

(In reply to comment #1)
> (In reply to comment #0)
> > -testSimpleViewLeak
> > -testBug265449PropertiesLeak
> 
> Fixed.
> 
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2f36b46905cbc7fefebe121ad272532e226064e5

Remy I'm not sure but the fix looks strange because I think you could leave out items because of the remove not. 

Take this structure:
MToolBar
MOpaqueToolItem
MOpaqueToolItem

If i read the code appropriately you would not remove the second MOpaqueToolItem or am I mistaken

Wouldn't it be better to use an Iterator and call iterator.remove()?

Comment 4 Thomas Schindl

2011-12-13 18:17:27 EST

(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > -testSimpleViewLeak
> > > -testBug265449PropertiesLeak
> 
> Correction. testBug2555005SiteLeak was fixed.
> 
> testBug265449PropertiesLeak is now fixed also.
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=fbf0e39912eed245b339765f927f4c00de16170d

Same comment as the other one. I'm reopening this please close if you don't agree that this could is bogus and an iterator is the better and correct solution.

Comment 5 Remy Suen

2011-12-13 18:34:24 EST

(In reply to comment #3)
> Remy I'm not sure but the fix looks strange because I think you could leave out
> items because of the remove not. 
> 
> Take this structure:
> MToolBar
> MOpaqueToolItem
> MOpaqueToolItem
> 
> If i read the code appropriately you would not remove the second
> MOpaqueToolItem or am I mistaken
> 
> Wouldn't it be better to use an Iterator and call iterator.remove()?

I'm calling remove on the iterating 'i' local variable and also decrementing it in the next line. It seems right to me but I could be wrong.

Comment 6 Thomas Schindl

2011-12-13 18:39:24 EST

Ah missed the decrement but I think using an iterator is better not?

Comment 7 Remy Suen

2011-12-13 18:41:06 EST

(In reply to comment #6)
> Ah missed the decrement but I think using an iterator is better not?

The code is certainly easier to read that way. I will rewrite the code.

Comment 8 Remy Suen

2012-01-19 10:59:31 EST

(In reply to comment #6)
> Ah missed the decrement but I think using an iterator is better not?

Done.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=aad73d8ba1f576f62764828a8d53f37a467e191d