Comment 1 Remy Suen 2012-10-22 14:08:19 EDT

(In reply to comment #0)
> As part of RC1 for 4.2 we implemented a 'hacky' fix for bug 374444. This
> code is not product quality

Indeed, there are two main problems with this code.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c92d78c6751c2eacb32e7fc9f94914910f40a5d9

1. Every time an MPES is created, a context is created and inserted into the parent context. Creating a context for every MPES instance isn't the issue, but pushing it into the parent context using the special "MultiPageEditorSite" key is.

Given that the context is essentially a map, future pushes of the created contexts will merely override the one that was previously inserted. When I open the PDE editor, I get N MPES instances created based on whether it's just a MANIFEST.MF or if there are plugin.xml or build.properties files around (up to 3 calls).

2. Disposing the MPES does not relay any information to the parent context. The current implementation of MPES's dispose() method currently only disposes the context but does not remove itself from the parent context. This means that the context will be returned in the MenuService code and ultimately reused to create the popup menu's context even though that context is already dead.

Comment 2 Remy Suen 2012-10-23 09:08:39 EDT

Created attachment 222678 [details]
MultiPageEditorPart context menu patch v1

This patch forces PopupMenuExtender to have a defined context that will be used for creating the popup menu under instead of having to run lookups on the part's context to find a more suitable context to use.

Comment 3 Paul Webster 2012-12-06 11:41:21 EST

Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=2510931a531dcb420af22ba5b99a2694feab8ddf


Thanx Remy

PW

Comment 4 Dani Megert 2012-12-10 02:57:52 EST

(In reply to comment #3)
> Released as
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?h=R4_2_maintenance&id=2510931a531dcb420af22ba5b99a2694feab8ddf
> 
> 
> Thanx Remy
> 
> PW

This causes a compile error in the Platform UI perf tests:

1. ERROR in /src/org/eclipse/ui/tests/performance/ObjectContributionTest.java
 (at line 387)
final PopupMenuExtender extender = new PopupMenuExtender(null, fakeMenuManager, selectionProvider, part);
The constructor PopupMenuExtender(null, MenuManager, ISelectionProvider, IWorkbenchPart) is undefined

Comment 5 Dani Megert 2012-12-10 02:58:49 EST

Same in 4.3 M4 warm-up build.

Comment 6 Remy Suen 2012-12-10 08:29:37 EST

(In reply to comment #4)
> This causes a compile error in the Platform UI perf tests

Thanh brought this up to me on Friday and I delivered a fix to R4_2_maintenance for that.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=8ed9606add1da66ea293c33bcc9b348e81be15ff

Can someone perform the merge to master?

Comment 7 Paul Webster 2012-12-10 11:06:04 EST

Sorry about that.  Merged into master with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9258229b1f3e49d7213c86933f07e924f74698ed

PW

Comment 8 Dani Megert 2012-12-11 02:41:17 EST

The error is gone in I20121210-2000.

Comment 9 Remy Suen 2012-12-11 19:12:12 EST

Thank you, Paul and Dani.

Comment 10 Paul Webster 2013-01-17 13:04:57 EST

In M20130116-1800
PW