Bug 404312

Comment 1 Paul Webster 2013-03-25 16:59:05 EDT

We can add an NPE guard, but the real question is what's wrong with the model that a rendered item is not contained within a rendered element with a context?

PW

Comment 2 Marc Gobeil 2013-03-25 17:07:14 EDT

We're messing around with a custom toolbar, and trying to override standard handling of the normal toolbars, so it's likely we've accidentally violated that rule in the process.  An NPE guard with a log message that gave the explanation you just did would be helpful though :)

Comment 3 Elena Laskavaia 2013-05-31 13:35:28 EDT

I think we found a problem. NPE is just side effect of platform removing
random toolbar items ny cool bar manager

This is code:

CoolBarToTrimManager:
	public IContributionItem remove(IContributionItem item) {
		final List<MToolBar> children = modelService.findElements(window, null, MToolBar.class, null);
                /// ^^^ here we getting childen, forming list A
		for (int i = 0; i < children.size(); i++) {
			final MToolBar child = children.get(i);
			final Object obj = child.getTransientData().get(OBJECT);
			if (obj != null && obj.equals(item)) {
                                // we found our child with index i
				...
				workbenchTrimElements.remove(child);

				child.setToBeRendered(false);
                                // we removing now element i from list B
                                // list A != list B!!!!!
                                // random element is removed
				child.getParent().getChildren().remove(i);
				return (IContributionItem) obj;
			}
                        ...
		}
		return null;
	}

Comment 4 Elena Laskavaia 2013-05-31 13:39:59 EDT

Created attachment 231829 [details]
Patch for 4.2 maintenance branch

Comment 5 Marc Gobeil 2013-05-31 13:58:17 EDT

Also:
- the hack applied by Bug 388516 was probably trying to mask this out, and can probably be removed after this is fixed
- even when this went wrong, the exception shouldn't have repeated indefinitely

For the second point, the runnable container that the hack code introduced in 388516 was embedded in, schedules itself every 400ms "until the list goes empty".  As we've seen here, this is dangerous because HandledContributionItem.updateItemEnablement() runs code in a SafeRunner that suppresses any exceptions thrown, but even when operating without throwing an exception, that polling loop can easily become a hidden performance problem.

Comment 6 Paul Webster 2013-05-31 16:04:31 EDT

Once we put this in master and 4.3.1 we can consider patching it back to R4_2_maintenance

PW

Comment 7 Elena Laskavaia 2013-06-05 13:24:06 EDT

Any progress on applying this to master at least?

Comment 8 Paul Webster 2013-06-05 13:35:32 EDT

We're in Kepler RC4, in effect no changes but doc changes.  I'm not sure when Luna and 4.3.1 will be open again, but some time possibly in the next 3 weeks.

PW

Comment 9 Paul Webster 2013-06-17 10:58:22 EDT

*** Bug 410712 has been marked as a duplicate of this bug. ***

Comment 10 Paul Webster 2013-06-18 14:32:52 EDT

Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=31c525b3493d0d53084d12f2edc6d70ea0e9c15d

Thanks Elena.

PW

Comment 11 Elena Laskavaia 2013-06-19 11:04:37 EDT

Do I need to clone it to get into maintanance branch?

Comment 12 Paul Webster 2013-06-19 12:35:13 EDT

(In reply to comment #11)
> Do I need to clone it to get into maintanance branch?

I've already cloned a bug so it can be backported once 4.3.1 opens

PW

Comment 13 Paul Webster 2013-07-29 15:39:13 EDT

(In reply to comment #11)
> Do I need to clone it to get into maintanance branch?

Elena, Could you please sign your CLA - http://wiki.eclipse.org/CLA

PW

Comment 14 Paul Webster 2013-08-01 15:19:38 EDT

(In reply to comment #13)
> 
> Elena, Could you please sign your CLA - http://wiki.eclipse.org/CLA

Hi Elena,

I'd like to get this into 4.3.1 but the window is short.  Did you get a chance to look at the CLA?

PW

Comment 15 Elena Laskavaia 2013-08-01 16:35:06 EDT

(In reply to comment #14)

> > 
> > Elena, Could you please sign your CLA - http://wiki.eclipse.org/CLA
> 
Signed

Comment 16 Paul Elder 2013-08-02 11:43:02 EDT

Release on 4.3 maintenance as:

https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_3_maintenance&id=03ef555d031fbf1c342116f8c281e174ff30da63

Comment 17 Paul Webster 2013-08-07 13:26:33 EDT

In 4.4.0.I20130806-2000

PW

Comment 18 Dani Megert 2013-08-09 06:14:59 EDT

(In reply to comment #16)
> Release on 4.3 maintenance as:
> 
> https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?h=R4_3_maintenance&id=03ef555d031fbf1c342116f8c281e174ff30da63

This is covered by bug 411054.