Attachment 114933 Details for Bug 247818 – Szymon's proposal

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

[patch] Szymon's proposal

247818_20081013.txt (text/plain), 15.49 KB, created by Szymon Brandys

on 2008-10-13 06:01:43 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Szymon Brandys

Created: 2008-10-13 06:01:43 EDT

Size: 15.49 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.ui.workbench
>Index: Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchStatusDialogManager.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchStatusDialogManager.java,v
>retrieving revision 1.18
>diff -u -r1.18 WorkbenchStatusDialogManager.java
>--- Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchStatusDialogManager.java	3 Oct 2008 19:40:52 -0000	1.18
>+++ Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchStatusDialogManager.java	13 Oct 2008 09:56:45 -0000
>@@ -21,6 +21,7 @@
> 
> import org.eclipse.core.runtime.Assert;
> import org.eclipse.core.runtime.IStatus;
>+import org.eclipse.core.runtime.ListenerList;
> import org.eclipse.core.runtime.jobs.Job;
> import org.eclipse.jface.action.ContributionItem;
> import org.eclipse.jface.action.IAction;
>@@ -39,7 +40,9 @@
> import org.eclipse.jface.resource.JFaceResources;
> import org.eclipse.jface.resource.LocalResourceManager;
> import org.eclipse.jface.resource.ResourceManager;
>+import org.eclipse.jface.util.IPropertyChangeListener;
> import org.eclipse.jface.util.Policy;
>+import org.eclipse.jface.util.PropertyChangeEvent;
> import org.eclipse.jface.viewers.IContentProvider;
> import org.eclipse.jface.viewers.ILabelProviderListener;
> import org.eclipse.jface.viewers.ISelection;
>@@ -221,6 +224,18 @@
> 	 */
> 	private class InternalDialog extends TrayDialog {
> 
>+		/* (non-Javadoc)
>+		 * @see org.eclipse.jface.dialogs.TrayDialog#close()
>+		 */
>+		public boolean close() {
>+			boolean result = super.close();
>+			if (!modalitySwitch) {
>+				firePropertyChangeEvent(new PropertyChangeEvent(this,
>+						DIALOG_OPENED, Boolean.TRUE, Boolean.FALSE));
>+			}
>+			return result;
>+		}
>+
> 		private WorkbenchStatusDialogManager statusDialog;
> 
> 		/**
>@@ -721,6 +736,22 @@
> 	 * The id of the goto action button
> 	 */
> 	private static final int GOTO_ACTION_ID = IDialogConstants.CLIENT_ID + 1;
>+	
>+	/**
>+	 * This property id for the state of the dialog being managed by the
>+	 * manager. newValue equals Boolean.TRUE means the dialog has been opened.
>+	 * newValue equals Boolean.FALSE means the dialog has been closed.
>+	 * 
>+	 * EXPERIMENTAL. This class or interface has been added as
>+	 * part of a work in progress. There is a guarantee neither that this API
>+	 * will work nor that it will remain the same. Please do not use this API
>+	 * without consulting with the Platform/UI team.
>+	 * 
>+	 * 
>+	 * @see IPropertyChangeListener
>+	 * @since 3.5
>+	 */
>+	public static final String DIALOG_OPENED = "dialogOpened"; //$NON-NLS-1$
> 
> 	/**
> 	 * Returns whether the given StatusAdapter object should be displayed.
>@@ -1000,6 +1031,8 @@
> 	 */
> 	private Composite titleArea;
> 
>+	private ListenerList listeners = new ListenerList();
>+
> 	/**
> 	 * Creates workbench status dialog.
> 	 * 
>@@ -1118,6 +1151,8 @@
> 							WorkbenchStatusDialogManager.this, shouldBeModal());
> 					setSelectedStatusAdapter(statusAdapter);
> 					dialog.open();
>+					firePropertyChangeEvent(new PropertyChangeEvent(this,
>+							DIALOG_OPENED, Boolean.FALSE, Boolean.TRUE));
> 					dialog.getShell().addDisposeListener(disposeListener);
> 				}
> 				refresh();
>@@ -1653,7 +1688,7 @@
> 	/**
> 	 * Returns the shell of the dialog.
> 	 */
>-	Shell getShell() {
>+	private Shell getShell() {
> 		if (this.dialog == null) return null;
> 		return this.dialog.getShell();
> 	}
>@@ -2190,4 +2225,61 @@
> 		}
> 		titleArea.layout();
> 	}
>+
>+	/**
>+	 * Adds a property change listener.
>+	 * 
>+	 * This method adds a listener to the dialog. Listener will be notified
>+	 * about dialog state changes. {@link #DIALOG_STATE} is currently the only
>+	 * property that can change.
>+	 * 
>+	 * EXPERIMENTAL. This class or interface has been added as
>+	 * part of a work in progress. There is a guarantee neither that this API
>+	 * will work nor that it will remain the same. Please do not use this API
>+	 * without consulting with the Platform/UI team.
>+	 * 
>+	 * 
>+	 * @see IPropertyChangeListener
>+	 * @param listener
>+	 * the property change listener to add
>+	 * @since 3.5
>+	 */
>+	public void addPropertyChangeListener(IPropertyChangeListener listener) {
>+		listeners.add(listener);
>+	}
>+
>+	/**
>+	 * Removes the property change listener.
>+	 * 
>+	 * 
>+	 * EXPERIMENTAL. This class or interface has been added as
>+	 * part of a work in progress. There is a guarantee neither that this API
>+	 * will work nor that it will remain the same. Please do not use this API
>+	 * without consulting with the Platform/UI team.
>+	 * 
>+	 * 
>+	 * @param listener
>+	 * the property change listener to remove
>+	 * 
>+	 * @since 3.5
>+	 */
>+	public void removePropertyChangeListener(IPropertyChangeListener listener){
>+		listeners.remove(listener);
>+	}
>+	
>+	private void firePropertyChangeEvent(PropertyChangeEvent event) {
>+		if (listeners.size() > 0) {
>+			final Object[] myListeners = listeners.getListeners();
>+			for (int i = 0; i < myListeners.length; ++i) {
>+				try {
>+					((IPropertyChangeListener) myListeners[i])
>+							.propertyChange(event);
>+				} catch (Exception e) {
>+					// no other choice. This may be potentially dangerous
>+					// exception.
>+					e.printStackTrace();
>+				}
>+			}
>+		}
>+	}
> }
>Index: Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java,v
>retrieving revision 1.24
>diff -u -r1.24 WorkbenchErrorHandler.java
>--- Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java	3 Oct 2008 19:15:44 -0000	1.24
>+++ Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java	13 Oct 2008 09:56:45 -0000
>@@ -13,8 +13,9 @@
> 
> import org.eclipse.core.runtime.IStatus;
> import org.eclipse.core.runtime.Status;
>+import org.eclipse.jface.util.IPropertyChangeListener;
>+import org.eclipse.jface.util.PropertyChangeEvent;
> import org.eclipse.swt.widgets.Display;
>-import org.eclipse.swt.widgets.Shell;
> import org.eclipse.ui.PlatformUI;
> import org.eclipse.ui.application.WorkbenchAdvisor;
> import org.eclipse.ui.internal.WorkbenchPlugin;
>@@ -26,6 +27,23 @@
> * @since 3.3
> */
> public class WorkbenchErrorHandler extends AbstractStatusHandler {
>+	
>+	private boolean dialogClosed = false;
>+	private IPropertyChangeListener listener = new IPropertyChangeListener(){
>+
>+		public void propertyChange(PropertyChangeEvent event) {
>+			//check if this is really OPENED -> CLOSED transition
>+			if (WorkbenchStatusDialogManager.DIALOG_OPENED.equals(event
>+					.getProperty())
>+					&& Boolean.TRUE.equals(event
>+							.getOldValue())
>+					&& Boolean.FALSE.equals(event
>+							.getNewValue())) {
>+				dialogClosed = true;
>+			}
>+		}
>+		
>+	};
> 
> 	private WorkbenchStatusDialogManager statusDialogManager;
> 
>@@ -110,10 +128,11 @@
> 		getStatusDialogManager().addStatusAdapter(statusAdapter, block);
> 
> 		if (block) {
>-			Shell shell;
>-			while ((shell = getStatusDialogManager().getShell()) != null
>-					&& !getStatusDialogManager().getShell().isDisposed()) {
>-				if (!shell.getDisplay().readAndDispatch()) {
>+			// this variable will be set to true if and only if user closes the
>+			// dialog
>+			dialogClosed = false;
>+			while (!dialogClosed) {
>+				if (!Display.getDefault().readAndDispatch()) {
> 					Display.getDefault().sleep();
> 				}
> 			}
>@@ -155,5 +174,6 @@
> 	protected void configureStatusDialog(
> 			final WorkbenchStatusDialogManager statusDialog) {
> 		// default configuration does nothing
>+		statusDialog.addPropertyChangeListener(listener);
> 	}
> }
>#P org.eclipse.ui.tests
>Index: Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java,v
>retrieving revision 1.6
>diff -u -r1.6 StatusDialogManagerTest.java
>--- Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java	3 Oct 2008 19:40:52 -0000	1.6
>+++ Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java	13 Oct 2008 09:56:47 -0000
>@@ -20,7 +20,9 @@
> import org.eclipse.jface.action.Action;
> import org.eclipse.jface.dialogs.ErrorDialog;
> import org.eclipse.jface.dialogs.IDialogConstants;
>+import org.eclipse.jface.util.IPropertyChangeListener;
> import org.eclipse.jface.util.Policy;
>+import org.eclipse.jface.util.PropertyChangeEvent;
> import org.eclipse.jface.viewers.ILabelProviderListener;
> import org.eclipse.jface.viewers.ITableLabelProvider;
> import org.eclipse.osgi.util.NLS;
>@@ -31,6 +33,7 @@
> import org.eclipse.swt.widgets.Button;
> import org.eclipse.swt.widgets.Composite;
> import org.eclipse.swt.widgets.Control;
>+import org.eclipse.swt.widgets.Display;
> import org.eclipse.swt.widgets.Event;
> import org.eclipse.swt.widgets.Label;
> import org.eclipse.swt.widgets.Shell;
>@@ -42,6 +45,8 @@
> import org.eclipse.ui.statushandlers.AbstractStatusAreaProvider;
> import org.eclipse.ui.statushandlers.IStatusAdapterConstants;
> import org.eclipse.ui.statushandlers.StatusAdapter;
>+import org.eclipse.ui.statushandlers.StatusManager;
>+import org.eclipse.ui.statushandlers.WorkbenchErrorHandler;
> import org.eclipse.ui.statushandlers.WorkbenchStatusDialogManager;
> 
> public class StatusDialogManagerTest extends TestCase {
>@@ -51,6 +56,7 @@
> 	private static final String THROWABLE = "throwable";
> 	private final static String MESSAGE_1 = "TEST_MESSAGE_1";
> 	private final static String MESSAGE_2 = "TEST_MESSAGE_2";
>+	private final static String MESSAGE_3 = "TEST_MESSAGE_2";
> 	private final static String TITLE = "TEST_TITLE";
> 	private final static NullPointerException NPE = new NullPointerException();
> 	private final static NullPointerException NPE_WITH_MESSAGE = new NullPointerException(
>@@ -73,6 +79,112 @@
> 		assertNotNull(shell);
> 		assertTrue((shell.getStyle() & SWT.APPLICATION_MODAL) == SWT.APPLICATION_MODAL);
> 	}
>+	
>+	/**
>+	 * This test checks if the calling thread is really blocked. It opens a
>+	 * dialog with the BLOCK flag in UI thread, and then monitors in the
>+	 * background thread that the dialog really appears (in 50 tries, 50
>+	 * milliseconds each). After the dialog appears, "OK" selection is emulated,
>+	 * the dialog is closed and the handle method returns. At this point there
>+	 * can be no shell.
>+	 */
>+	public void testBlockingBehavior1() {
>+		WorkbenchErrorHandler weh = new WorkbenchErrorHandler();
>+		Thread thread = new Thread(new Runnable(){
>+			public void run() {
>+				int count = 50;
>+				final boolean opened[] = new boolean[]{false};
>+				while (!opened[0] && count-- != 0) {
>+					try {
>+						Thread.sleep(50);
>+					} catch (InterruptedException e) {
>+						e.printStackTrace();
>+					}
>+					Display.getDefault().syncExec(new Runnable() {
>+						public void run() {
>+							opened[0] = StatusDialogUtil.getStatusShell() != null;
>+							if(opened[0]){
>+								selectWidget(StatusDialogUtil.getOkButton());
>+							}
>+						}
>+					});
>+				}
>+				assertTrue("Dialog should appear!", count > 0);
>+				
>+			}
>+		});
>+		thread.start();
>+		weh.handle(createStatusAdapter(MESSAGE_1), StatusManager.BLOCK);
>+		assertNull("Dialog should block the calling thread!", StatusDialogUtil
>+				.getStatusShell());
>+	}
>+
>+	/**
>+	 * This is more advanced version of testBlockingBehavior1. We have 2
>+	 * background threads that raise statuses. UI thread also raises one. No
>+	 * thread can proceed until "OK" is emulated (and this will happen if the
>+	 * dialog is opened and three statuses were reported).
>+	 */
>+	public void testBlockingBehavior2() {
>+		final WorkbenchErrorHandler weh = new WorkbenchErrorHandler();
>+		Thread thread1 = new Thread(new Runnable() {
>+			public void run() {
>+				weh.handle(createStatusAdapter(MESSAGE_1), StatusManager.BLOCK);
>+				Display.getDefault().syncExec(new Runnable() {
>+					public void run() {
>+						assertNull("Dialog should block the calling thread!",
>+								StatusDialogUtil.getStatusShell());
>+					}
>+				});
>+			}
>+
>+		});
>+		thread1.start();
>+		Thread thread2 = new Thread(new Runnable() {
>+			public void run() {
>+				weh.handle(createStatusAdapter(MESSAGE_2), StatusManager.BLOCK);
>+				Display.getDefault().syncExec(new Runnable() {
>+					public void run() {
>+						assertNull("Dialog should block the calling thread!",
>+								StatusDialogUtil.getStatusShell());
>+					}
>+				});
>+			}
>+		});
>+		thread2.start();
>+		Thread checker = new Thread(new Runnable() {
>+			public void run() {
>+				int count = 50;
>+				final boolean statusesShown[] = new boolean[] { false };
>+				while (!statusesShown[0] && count-- != 0) {
>+					try {
>+						Thread.sleep(50);
>+					} catch (InterruptedException e) {
>+						e.printStackTrace();
>+					}
>+					Display.getDefault().syncExec(new Runnable() {
>+						public void run() {
>+							boolean dialogVisible = StatusDialogUtil
>+									.getStatusShell() != null;
>+							statusesShown[0] = dialogVisible
>+									&& StatusDialogUtil.getTable()
>+											.getItemCount() == 3;
>+							if (statusesShown[0]) {
>+								selectWidget(StatusDialogUtil.getOkButton());
>+							}
>+						}
>+					});
>+				}
>+				assertTrue(
>+						"Dialog should appear and all statuses should be shown",
>+						count > 0);
>+			}
>+		});
>+		checker.start();
>+		weh.handle(createStatusAdapter(MESSAGE_3), StatusManager.BLOCK);
>+		assertNull("Dialog should block the calling thread!", StatusDialogUtil
>+				.getStatusShell());
>+	}
> 
> 	public void testNonBlockingAppearance() {
> 		wsdm.addStatusAdapter(createStatusAdapter(MESSAGE_1), false);
>@@ -80,6 +192,52 @@
> 		assertNotNull(shell);
> 		assertFalse((shell.getStyle() & SWT.APPLICATION_MODAL) == SWT.APPLICATION_MODAL);
> 	}
>+	
>+	/**
>+	 * Check that listener is notified about dialog closure when the dialog is
>+	 * closed by the user.
>+	 */
>+	public void testListener1() {
>+		final PropertyChangeEvent[] events = new PropertyChangeEvent[1];
>+		wsdm.addPropertyChangeListener(new IPropertyChangeListener() {
>+
>+			public void propertyChange(PropertyChangeEvent event) {
>+				events[0] = event;
>+			}
>+
>+		});
>+		wsdm.addStatusAdapter(createStatusAdapter(MESSAGE_1), false);
>+		selectWidget(StatusDialogUtil.getOkButton());
>+		assertNotNull(events[0]);
>+		assertTrue(Boolean.FALSE.equals(events[0]
>+				.getNewValue()));
>+	}
>+	
>+	/**
>+	 * Check that listener is notified about dialog opening.
>+	 * Check that listener is not notified about dialog closure when the
>+	 * modality is switched.
>+	 */
>+	public void testListener2(){
>+		final PropertyChangeEvent[] events = new PropertyChangeEvent[1];
>+		wsdm.addPropertyChangeListener(new IPropertyChangeListener() {
>+
>+			public void propertyChange(PropertyChangeEvent event) {
>+				events[0] = event;
>+			}
>+
>+		});
>+		wsdm.addStatusAdapter(createStatusAdapter(MESSAGE_1), false);
>+		wsdm.addStatusAdapter(createStatusAdapter(MESSAGE_1), true);
>+		assertNotNull(events[0]);
>+		assertTrue(Boolean.TRUE.equals(events[0]
>+				.getNewValue()));
>+		selectWidget(StatusDialogUtil.getOkButton());
>+		// now should be notified
>+		assertNotNull(events[0]);
>+		assertTrue(Boolean.FALSE.equals(events[0]
>+				.getNewValue()));
>+	}
> 
> 	public void testModalitySwitch1() {
> 		wsdm.addStatusAdapter(createStatusAdapter(MESSAGE_1), false);

Actions: View | Diff

Attachments on bug 247818: 113730 | 113856 | 113857 | 113964 | 114310 | 114311 | 114933 | 115700 | 115714