Bug 495567 - properly handle dirty state changes in properties view

This is the first part of the fix, corresponding to the solution #1
described in bug 495567 comment 6): a part exposing
ISaveablePart either via implementing it or via providing adapter to it,
must properly handle and propagate dirty state changes to the UI.

Properties view will properly track now dirty state of the contributing
ISaveablePart part, also if multiple instances of Properties view are
opened for same contributing part. As a result, UI representation of a
Properties view tracking a ISaveablePart will change: it will show
leading asterisk '*' in all cases the contributing part also shows it,
and hide it otherwise.

Change-Id: I7e2084ad8e250fa78c734ecd2cee6786ce7d9138
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

2 files changed, 64 insertions, 4 deletions

diff --git a/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java b/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
index 510eda4ea94..dc979368f24 100644
--- a/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
+++ b/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
@@ -36,6 +36,7 @@ import org.eclipse.swt.widgets.Composite;
 import org.eclipse.ui.IEditorPart;
 import org.eclipse.ui.IMemento;
 import org.eclipse.ui.ISaveablePart;
+import org.eclipse.ui.ISaveablesLifecycleListener;
 import org.eclipse.ui.ISelectionListener;
 import org.eclipse.ui.IViewPart;
 import org.eclipse.ui.IViewReference;
@@ -43,8 +44,13 @@ import org.eclipse.ui.IViewSite;
 import org.eclipse.ui.IWorkbenchActionConstants;
 import org.eclipse.ui.IWorkbenchPage;
 import org.eclipse.ui.IWorkbenchPart;
+import org.eclipse.ui.IWorkbenchPartConstants;
 import org.eclipse.ui.IWorkbenchPartSite;
 import org.eclipse.ui.PartInitException;
+import org.eclipse.ui.Saveable;
+import org.eclipse.ui.SaveablesLifecycleEvent;
+import org.eclipse.ui.internal.DefaultSaveable;
+import org.eclipse.ui.internal.SaveablesList;
 import org.eclipse.ui.internal.views.properties.PropertiesMessages;
 import org.eclipse.ui.part.IContributedContentsView;
 import org.eclipse.ui.part.IPage;
@@ -129,6 +135,48 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
 
 	private boolean wasHidden;
 
+	private final SaveablesTracker saveablesTracker;
+
+	/**
+	 * Propagates state changes of the saveable part tracked by this properties
+	 * view, to properly update the dirty status. See bug 495567 comment 18.
+	 */
+	class SaveablesTracker implements ISaveablesLifecycleListener {
+
+		@Override
+		public void handleLifecycleEvent(SaveablesLifecycleEvent event) {
+			if (currentPart == null || event.getEventType() != SaveablesLifecycleEvent.DIRTY_CHANGED) {
+				return;
+			}
+			// to avoid endless loop we must ignore our own instance which
+			// reports state changes too
+			Saveable[] saveables = event.getSaveables();
+			if (saveables == null) {
+				return;
+			}
+			for (Saveable saveable : saveables) {
+				// check if the saveable is for the current part
+				if (new DefaultSaveable(PropertySheet.this).equals(saveable)) {
+					return;
+				}
+			}
+
+			if (event.getSource() instanceof SaveablesList) {
+				SaveablesList saveablesList = (SaveablesList) event.getSource();
+				for (Saveable saveable : saveables) {
+					IWorkbenchPart[] parts = saveablesList.getPartsForSaveable(saveable);
+					for (IWorkbenchPart part : parts) {
+						if (PropertySheet.this.currentPart == part) {
+							firePropertyChange(IWorkbenchPartConstants.PROP_DIRTY);
+							return;
+						}
+					}
+				}
+			}
+		}
+
+	}
+
     /**
      * Creates a property sheet view.
      */
@@ -136,6 +184,7 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
         super();
         pinPropertySheetAction = new PinPropertySheetAction();
         RegistryFactory.getRegistry().addListener(this, EXT_POINT);
+		saveablesTracker = new SaveablesTracker();
     }
 
     @Override
@@ -174,7 +223,10 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
 				.getToolBarManager();
 		menuManager.add(new Separator(IWorkbenchActionConstants.MB_ADDITIONS));
 		toolBarManager.add(pinPropertySheetAction);
-
+		ISaveablesLifecycleListener saveables = getSite().getService(ISaveablesLifecycleListener.class);
+		if (saveables instanceof SaveablesList) {
+			((SaveablesList) saveables).addModelLifecycleListener(saveablesTracker);
+		}
         getSite().getPage().getWorkbenchWindow().getWorkbench().getHelpSystem()
 				.setHelp(getPageBook(),
 						IPropertiesHelpContextIds.PROPERTY_SHEET_VIEW);
@@ -188,7 +240,10 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
         // remove ourselves as a selection and registry listener
         getSite().getPage().removePostSelectionListener(this);
         RegistryFactory.getRegistry().removeListener(this);
-
+		ISaveablesLifecycleListener saveables = getSite().getService(ISaveablesLifecycleListener.class);
+		if (saveables instanceof SaveablesList) {
+			((SaveablesList) saveables).removeModelLifecycleListener(saveablesTracker);
+		}
         currentPart = null;
         currentSelection = null;
         pinPropertySheetAction = null;
@@ -414,6 +469,8 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
 		} else {
 			setContentDescription(""); //$NON-NLS-1$
 		}
+		// since our selection changes, our dirty state might change too
+		firePropertyChange(IWorkbenchPartConstants.PROP_DIRTY);
 	}
 
     /**
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
index e62c9ef114f..a43bbf9159d 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
@@ -26,7 +26,6 @@ import org.eclipse.e4.core.services.log.Logger;
 import org.eclipse.e4.ui.di.Focus;
 import org.eclipse.e4.ui.di.Persist;
 import org.eclipse.e4.ui.di.PersistState;
-import org.eclipse.e4.ui.model.application.ui.MDirtyable;
 import org.eclipse.e4.ui.model.application.ui.basic.MPart;
 import org.eclipse.e4.ui.model.application.ui.menu.MMenu;
 import org.eclipse.e4.ui.workbench.IPresentationEngine;
@@ -376,7 +375,11 @@ public abstract class CompatibilityPart implements ISelectionChangedListener {
 				case IWorkbenchPartConstants.PROP_DIRTY:
 					ISaveablePart saveable = SaveableHelper.getSaveable(wrapped);
 					if (saveable != null) {
-						((MDirtyable) part).setDirty(saveable.isDirty());
+						part.setDirty(saveable.isDirty());
+					} else if (part.isDirty()) {
+						// reset if the wrapped legacy part do not exposes
+						// saveable adapter anymore, see bug 495567 comment 6
+						part.setDirty(false);
 					}
 					break;
 				case IWorkbenchPartConstants.PROP_INPUT: