398712: [Core] Multiple issues with the PageManager

https://bugs.eclipse.org/bugs/show_bug.cgi?id=398712

401434: [Core] The CoreEditor shall be covered with integration tests
https://bugs.eclipse.org/bugs/show_bug.cgi?id=401434

Improve the SashEditor widget robustness against invalid tabs

4 files changed, 125 insertions, 22 deletions

diff --git a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/contentprovider/IPageManager.java b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/contentprovider/IPageManager.java
index 114407663d1..70634c2be42 100644
--- a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/contentprovider/IPageManager.java
+++ b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/contentprovider/IPageManager.java
@@ -122,4 +122,11 @@ public interface IPageManager extends IPageMngr {
 	 */

 	public void selectPage(Object pageIdentifier);

 

+	/**

+	 * Returns the active page identifier

+	 * 

+	 * @return

+	 */

+	//	public Object getActivePage();

+

 }

diff --git a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ComponentPart.java b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ComponentPart.java
index a3c801b967b..a2ba53a5f77 100644
--- a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ComponentPart.java
+++ b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ComponentPart.java
@@ -63,6 +63,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * 

 	 * @param parent

 	 */

+	@Override

 	public void createPartControl(Composite parent) {

 

 		try {

@@ -85,8 +86,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * @return

 	 * @throws PartInitException

 	 */

-	private Composite createEditorPartControl(Composite parentControl)

-			throws PartInitException {

+	private Composite createEditorPartControl(Composite parentControl) throws PartInitException {

 		Composite editorParent = new Composite(parentControl, SWT.NONE);

 		editorParent.setLayout(new FillLayout());

 		partModel.createPartControl(editorParent);

@@ -95,10 +95,10 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	}

 

 	/**

-	 * Dispose all resources used by this part.

-	 * <br/>

+	 * Dispose all resources used by this part. <br/>

 	 * The Part should not be used after it has been disposed.

 	 */

+	@Override

 	public void dispose() {

 

 		//		detachListeners(editorControl, true);

@@ -110,14 +110,13 @@ public class ComponentPart extends PagePart implements IComponentPage {
 

 	/**

 	 * Dispose this part and all its children.

-	 * The method is called recursively on children of the part.

-	 * <br/> 

+	 * The method is called recursively on children of the part. <br/>

 	 * SWT resources have already been disposed. We don't need to dispose them again.

 	 * 

 	 */

 	@Override

 	public void disposeThisAndChildren() {

-		

+

 		// clean up properties to help GC

 		partModel = null;

 	}

@@ -140,17 +139,16 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * @param toFind

 	 * @return

 	 */

+	@Override

 	public PagePart findPartAt(Point toFind, Class<?> expectedTileType) {

 

-		if(expectedTileType == this.getClass())

+		if(expectedTileType == this.getClass()) {

 			return this;

+		}

 

 		// Not found !!

 		// The tile contains the position, but the type is not found.

-		throw new UnsupportedOperationException("Tile match the expected position '"

-				+ toFind

-				+ "' but there is no Tile of requested type '"

-				+ expectedTileType.getClass().getName() + "'");

+		throw new UnsupportedOperationException("Tile match the expected position '" + toFind + "' but there is no Tile of requested type '" + expectedTileType.getClass().getName() + "'");

 	}

 

 	/**

@@ -158,8 +156,9 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * @return

 	 */

 	public PagePart findPart(Object control) {

-		if(getControl() == control)

+		if(getControl() == control) {

 			return this;

+		}

 

 		// Not found

 		return null;

@@ -170,6 +169,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * 

 	 * @return

 	 */

+	@Override

 	public Composite getControl() {

 		return editorControl;

 	}

@@ -199,6 +199,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * @param compositeParent

 	 *        The composite that should be used as parent.

 	 */

+	@Override

 	public void reparent(TabFolderPart newParent) {

 

 		// Change the tile parent

@@ -207,7 +208,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 		editorControl.setParent(newParent.getControl());

 

 		// Change state

-		if(garbageState == GarbageState.UNVISITED || garbageState == GarbageState.ORPHANED) {

+		if(garbageState == GarbageState.UNVISITED || garbageState == GarbageState.ORPHANED || garbageState == GarbageState.CREATED) {

 			garbageState = GarbageState.REPARENTED;

 		} else {

 			// Bad state, this is an internal error

@@ -221,8 +222,11 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * Asks this part to take focus within the workbench.

 	 * Set the focus on the active nested part if the part is a container.

 	 */

+	@Override

 	public void setFocus() {

-		editorControl.setFocus();

+		if(editorControl != null) {

+			editorControl.setFocus();

+		}

 	}

 

 

@@ -246,6 +250,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * from its parent.

 	 * 

 	 */

+	@Override

 	public void garbage() {

 		dispose();

 		// fire appropriate life cycle event

@@ -260,6 +265,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 	 * @param visitor

 	 * @return

 	 */

+	@Override

 	public boolean visit(IPartVisitor visitor) {

 		return visitor.accept(this);

 	}

@@ -286,8 +292,7 @@ public class ComponentPart extends PagePart implements IComponentPage {
 		//				+ ", '" + editorPart.getTitle()

 		//				+ "', " + this);

 

-		System.out.printf("ComponentPart: disposed=%-5b, visible=%-5b, garbState=%-10s, %s, %s\n"

-				, editorControl.isDisposed(), (editorControl.isDisposed() ? false : editorControl.isVisible()), garbageState, getPageTitle(), this);

+		System.out.printf("ComponentPart: disposed=%-5b, visible=%-5b, garbState=%-10s, %s, %s\n", editorControl.isDisposed(), (editorControl.isDisposed() ? false : editorControl.isVisible()), garbageState, getPageTitle(), this);

 

 	}

 

diff --git a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ErrorComponentPart.java b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ErrorComponentPart.java
new file mode 100644
index 00000000000..6f912300cf5
--- /dev/null
+++ b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/ErrorComponentPart.java
@@ -0,0 +1,80 @@
+/*****************************************************************************

+ * Copyright (c) 2013 CEA LIST.

+ * 

+ * All rights reserved. This program and the accompanying materials

+ * are made available under the terms of the Eclipse Public License v1.0

+ * which accompanies this distribution, and is available at

+ * http://www.eclipse.org/legal/epl-v10.html

+ *

+ * Contributors:

+ *  Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation

+ *****************************************************************************/

+package org.eclipse.papyrus.infra.core.sasheditor.internal;

+

+import org.eclipse.papyrus.infra.core.sasheditor.contentprovider.IComponentModel;

+import org.eclipse.swt.SWT;

+import org.eclipse.swt.graphics.Image;

+import org.eclipse.swt.layout.GridData;

+import org.eclipse.swt.layout.GridLayout;

+import org.eclipse.swt.widgets.Composite;

+import org.eclipse.swt.widgets.Display;

+import org.eclipse.swt.widgets.Label;

+import org.eclipse.ui.ISharedImages;

+import org.eclipse.ui.PlatformUI;

+

+/**

+ * A PagePart which can be used to fill a tab when an error occurs

+ * Avoid manipulating "null" instances, which may lead to widget crashes

+ * 

+ * @author Camille Letavernier

+ * 

+ */

+public class ErrorComponentPart extends ComponentPart {

+

+	public ErrorComponentPart(TabFolderPart parent) {

+		super(parent, createErrorComponentPartModel(), null);

+		createPartControl(parent.getControl());

+	}

+

+	private static IComponentModel createErrorComponentPartModel() {

+		IComponentModel componentModel = new IComponentModel() {

+

+			public String getTabTitle() {

+				return "Invalid tab";

+			}

+

+			public Image getTabIcon() {

+				return PlatformUI.getWorkbench().getSharedImages().getImage(ISharedImages.IMG_OBJS_ERROR_TSK);

+			}

+

+			public Image getComponentIcon() {

+				return Display.getDefault().getSystemImage(SWT.ICON_ERROR);

+			}

+

+			public Object getRawModel() {

+				return null;

+			}

+

+			public Composite createPartControl(Composite parent) {

+				Composite tabComposite = new Composite(parent, SWT.NONE);

+				tabComposite.setLayout(new GridLayout(2, false));

+

+				Image componentIcon = getComponentIcon();

+				if(componentIcon != null) {

+					Label errorImageLabel = new Label(tabComposite, SWT.NONE);

+					errorImageLabel.setLayoutData(new GridData(SWT.BEGINNING, SWT.CENTER, false, false));

+					errorImageLabel.setImage(componentIcon);

+				}

+

+				Label label = new Label(tabComposite, SWT.NONE);

+				label.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false));

+				label.setText("Invalid tab");

+

+				return tabComposite;

+			}

+		};

+

+		return componentModel;

+	}

+

+}

diff --git a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/TabFolderPart.java b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/TabFolderPart.java
index 4578cd8d370..ecdec9d0489 100644
--- a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/TabFolderPart.java
+++ b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor/src/org/eclipse/papyrus/infra/core/sasheditor/internal/TabFolderPart.java
@@ -19,6 +19,7 @@ import java.util.logging.Logger;
 

 import org.eclipse.jface.action.MenuManager;

 import org.eclipse.jface.util.Geometry;

+import org.eclipse.papyrus.infra.core.sasheditor.Activator;

 import org.eclipse.papyrus.infra.core.sasheditor.contentprovider.IPageModel;

 import org.eclipse.papyrus.infra.core.sasheditor.contentprovider.ITabFolderModel;

 import org.eclipse.papyrus.infra.core.sasheditor.internal.eclipsecopy.AbstractTabFolderPart;

@@ -958,9 +959,6 @@ public class TabFolderPart extends AbstractTabFolderPart {
 		} else {

 			// No part found, create one

 			modelPart = createChildPart(newModel);

-			if(modelPart == null) {

-				System.out.println("break");

-			}

 			existingParts.addCreatedPage(modelPart);

 			// Attach it to the tabItem

 			tabItem.resetChild(modelPart);

@@ -993,8 +991,21 @@ public class TabFolderPart extends AbstractTabFolderPart {
 

 			return newPart;

 		}

-		// Use exception ?

-		return null;

+

+		// Use exception?

+		return createErrorPage();

+	}

+

+	/**

+	 * Creates an ErrorComponentPart which displays an error message to the user.

+	 * Avoid returning null, which might later lead to NPEs

+	 * 

+	 * @return

+	 */

+	private PagePart createErrorPage() {

+		//Handle invalid tab case. Improve robustness.

+		Activator.log.warn("Error: the SashWindowContainer returned an invalid tab");

+		return new ErrorComponentPart(this);

 	}

 

 	/**