Bug 443632 - [Performance] Event Loop Monitor prefs page: UI bugs

Change-Id: I69f98ce5299d606e13da7b2fa5a272960111d582
Signed-off-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>

5 files changed, 131 insertions, 73 deletions

diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterInputDialog.java b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterInputDialog.java
index ac7f2ebf344..fbc98a3f528 100644
--- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterInputDialog.java
+++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterInputDialog.java
@@ -7,14 +7,21 @@
  *
  * Contributors:
  *     Marcus Eng (Google) - initial API and implementation
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.ui.internal.monitoring.preferences;
 
+import java.util.regex.Pattern;
+
+import org.eclipse.jface.dialogs.IDialogConstants;
 import org.eclipse.jface.dialogs.IMessageProvider;
 import org.eclipse.jface.dialogs.TitleAreaDialog;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.ModifyEvent;
+import org.eclipse.swt.events.ModifyListener;
 import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Label;
@@ -25,6 +32,9 @@ import org.eclipse.swt.widgets.Text;
  * A dialog box used to input new stack traces to filter out.
  */
 public class FilterInputDialog extends TitleAreaDialog {
+	private static final Pattern methodNamePattern =
+			Pattern.compile("([\\p{L}_$][\\p{L}\\p{N}_$]*\\.)+[\\p{L}_$][\\p{L}\\p{N}_$]*");
+
 	private Text textFilter;
 	private String filter;
 
@@ -37,10 +47,15 @@ public class FilterInputDialog extends TitleAreaDialog {
 	public void create() {
 		super.create();
 		setTitle(Messages.FilterInputDialog_filter_input_header);
-		setMessage(Messages.FilterInputDialog_filter_input_message,
-				IMessageProvider.INFORMATION);
+		setMessage(Messages.FilterInputDialog_filter_input_message, IMessageProvider.INFORMATION);
 	}
 
+    @Override
+	protected void configureShell(Shell shell) {
+        super.configureShell(shell);
+		shell.setText(Messages.FilterInputDialog_filter_input_title);
+    }
+
 	@Override
 	protected Control createDialogArea(Composite parent) {
 		Composite area = (Composite) super.createDialogArea(parent);
@@ -55,38 +70,51 @@ public class FilterInputDialog extends TitleAreaDialog {
 		return area;
 	}
 
+	@Override
+	protected Control createContents(Composite parent) {
+		Control contents = super.createContents(parent);
+		checkInput();
+		return contents;
+	}
+
 	private void createFilterText(Composite container) {
 		Label filterLabel = new Label(container, SWT.NONE);
 		filterLabel.setText(Messages.FilterInputDialog_filter_input_label);
 
-		GridData dataFilterInput = new GridData();
-		dataFilterInput.grabExcessHorizontalSpace = true;
-		dataFilterInput.horizontalAlignment = GridData.FILL;
-
 		textFilter = new Text(container, SWT.BORDER);
-		textFilter.setLayoutData(dataFilterInput);
+		textFilter.setLayoutData(new GridData(SWT.FILL, SWT.BEGINNING, true, false));
+		textFilter.addModifyListener(new ModifyListener() {
+			@Override
+			public void modifyText(ModifyEvent e) {
+				checkInput();
+			}
+		});
 	}
 
-	@Override
-	protected boolean isResizable() {
-		return false;
-	}
-
-	private void saveInput() {
-		filter = textFilter.getText();
+	private void checkInput() {
+		filter = textFilter.getText().trim();
+		Button okButton = getButton(IDialogConstants.OK_ID);
+		String errorMessage = null;
+		if (!methodNamePattern.matcher(filter).matches()) {
+			if (!filter.isEmpty()) {
+				errorMessage = Messages.FilterInputDialog_invalid_method_name;
+			}
+			okButton.setEnabled(false);
+		} else {
+			okButton.setEnabled(true);
+		}
+		setErrorMessage(errorMessage);
 	}
 
 	@Override
-	protected void okPressed() {
-		saveInput();
-		textFilter.clearSelection();
-		super.okPressed();
+	protected boolean isResizable() {
+		return true;
 	}
 
 	/**
 	 * Returns a {@code String} of the user-defined filter.
 	 */
-	 public String getInput() {
-		 return filter;
-	 }
+	public String getFilter() {
+		return filter;
+	}
 }
diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/ListFieldEditor.java b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterListEditor.java
index 307cc053ec8..7827b58fb57 100644
--- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/ListFieldEditor.java
+++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/FilterListEditor.java
@@ -7,32 +7,41 @@
  *
  * Contributors:
  *     Marcus Eng (Google) - initial API and implementation
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.ui.internal.monitoring.preferences;
 
+import org.eclipse.jface.layout.PixelConverter;
 import org.eclipse.jface.preference.ListEditor;
+import org.eclipse.jface.window.Window;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.widgets.Composite;
-import org.eclipse.ui.internal.monitoring.MonitoringPlugin;
-import org.eclipse.ui.monitoring.PreferenceConstants;
+import org.eclipse.swt.widgets.List;
 
 /**
  * Displays the list of traces to filter out and ignore.
  */
-public class ListFieldEditor extends ListEditor {
-	private FilterInputDialog dialog;
-
-	ListFieldEditor(String name, String labelText, Composite parent) {
+public class FilterListEditor extends ListEditor {
+	FilterListEditor(String name, String labelText, Composite parent) {
 		super(name, labelText, parent);
-		super.getAddButton().setText(Messages.ListFieldEditor_add_filter_button_label);
-		super.getUpButton().setVisible(false);
-		super.getDownButton().setVisible(false);
-		dialog = new FilterInputDialog(super.getShell());
-		this.setEnabled(MonitoringPlugin.getDefault().getPreferenceStore()
-				.getBoolean(PreferenceConstants.MONITORING_ENABLED), parent);
+		getAddButton().setText(Messages.ListFieldEditor_add_filter_button_label);
+		getUpButton().setVisible(false);
+		getDownButton().setVisible(false);
 	}
 
+    @Override
+	protected void doFillIntoGrid(Composite parent, int numColumns) {
+    	super.doFillIntoGrid(parent, numColumns);
+        List list = getListControl(parent);
+        GridData gd = new GridData(SWT.FILL, SWT.FILL, true, true, numColumns - 1, 1);
+    	PixelConverter pixelConverter = new PixelConverter(parent);
+        gd.widthHint = pixelConverter.convertWidthInCharsToPixels(65);
+        list.setLayoutData(gd);
+    }
+
 	/**
-	 * Handles the parsing of defined traces to be filtered.
+	 * Handles parsing of defined traces to be filtered.
 	 */
 	@Override
 	protected String createList(String[] items) {
@@ -51,11 +60,9 @@ public class ListFieldEditor extends ListEditor {
 
 	@Override
 	protected String getNewInputObject() {
-		dialog.open();
-		String input = dialog.getInput();
-		if (input != null && !input.isEmpty() && !input.contains(",")) { //$NON-NLS-1$
-			input.trim();
-			return dialog.getInput();
+		FilterInputDialog dialog = new FilterInputDialog(getShell());
+		if (dialog.open() == Window.OK) {
+			return dialog.getFilter();
 		}
 		return null;
 	}
diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.java b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.java
index 35ff82c42ba..6212c0d8df9 100644
--- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.java
+++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.java
@@ -17,6 +17,8 @@ public final class Messages extends NLS {
 	public static String FilterInputDialog_filter_input_header;
 	public static String FilterInputDialog_filter_input_message;
 	public static String FilterInputDialog_filter_input_label;
+	public static String FilterInputDialog_filter_input_title;
+	public static String FilterInputDialog_invalid_method_name;
 	public static String ListFieldEditor_add_filter_button_label;
 	public static String MonitoringPreferenceListener_preference_error_header;
 	public static String MonitoringPreferenceListener_preference_error;
diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.properties b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.properties
index 84af60fedfc..f23a1fd233c 100644
--- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.properties
+++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/Messages.properties
@@ -10,10 +10,12 @@
 #	  Sergey Prigogin (Google)
 ###############################################################################
 
-FilterInputDialog_filter_input_message=Enter the fully qualified method name of a stack trace to filter out.
-FilterInputDialog_filter_input_label=Stack Frame to Ignore:
 FilterInputDialog_filter_input_header=New Stack Trace Filter
-ListFieldEditor_add_filter_button_label=Add &Filter
+FilterInputDialog_filter_input_label=Stack frame to ignore:
+FilterInputDialog_filter_input_message=Enter the fully qualified method name of a stack frame to filter out.\n Long events containing this stack frame will not be logged.
+FilterInputDialog_filter_input_title=Add Filter
+FilterInputDialog_invalid_method_name=Invalid method name
+ListFieldEditor_add_filter_button_label=Add &Filter...
 MonitoringPreferenceListener_preference_error_header=Invalid Preferences
 MonitoringPreferenceListener_preference_error=The specified preferences could not be updated. See error log for details.
 MonitoringPreferencePage_deadlock_threshold_label=Deadl&ock reporting threshold (ms):
@@ -21,7 +23,7 @@ MonitoringPreferencePage_deadlock_threshold_too_low_error=Deadlock reporting thr
 MonitoringPreferencePage_dump_all_threads_label=D&ump stacks of all threads
 MonitoringPreferencePage_enable_thread_label=&Enable UI responsiveness monitoring
 MonitoringPreferencePage_long_event_threshold=Long event &threshold (ms):
-MonitoringPreferencePage_filter_label=Stack tra&ces to filter out:
+MonitoringPreferencePage_filter_label=&Stack frames to filter out:
 MonitoringPreferencePage_initial_sample_delay_label=I&nitial stack sample delay (ms):
 MonitoringPreferencePage_initial_sample_delay_too_high_error=Initial sample delay must be lower than long event threshold.
 MonitoringPreferencePage_log_freeze_events_label=&Log freeze events to Eclipse error log
diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferencePage.java b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferencePage.java
index b7cc025cd3a..2cbcc8d2c8e 100644
--- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferencePage.java
+++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferencePage.java
@@ -13,6 +13,7 @@ package org.eclipse.ui.internal.monitoring.preferences;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.eclipse.jface.layout.PixelConverter;
 import org.eclipse.jface.preference.BooleanFieldEditor;
 import org.eclipse.jface.preference.FieldEditor;
 import org.eclipse.jface.preference.FieldEditorPreferencePage;
@@ -31,10 +32,12 @@ import org.eclipse.ui.monitoring.PreferenceConstants;
 /**
  * Preference page that allows user to toggle plug in settings from Eclipse preferences.
  */
-public class MonitoringPreferencePage extends FieldEditorPreferencePage implements IWorkbenchPreferencePage {
+public class MonitoringPreferencePage extends FieldEditorPreferencePage
+		implements IWorkbenchPreferencePage {
 	private static final int HOUR_IN_MS = 3600000;
-	private static final IPreferenceStore preferences = MonitoringPlugin.getDefault().getPreferenceStore();
-	private boolean pluginEnabled = preferences.getBoolean(PreferenceConstants.MONITORING_ENABLED);
+	private static final IPreferenceStore preferences =
+			MonitoringPlugin.getDefault().getPreferenceStore();
+	private BooleanFieldEditor monitoringEnabled;
 	private IntegerEditor longEventThreshold;
 	private IntegerEditor sampleInterval;
 	private IntegerEditor initialSampleDelay;
@@ -105,25 +108,27 @@ public class MonitoringPreferencePage extends FieldEditorPreferencePage implemen
 	@Override
 	public void createFieldEditors() {
 		Composite parent = getFieldEditorParent();
-		final GridLayout layout = new GridLayout();
+    	PixelConverter pixelConverter = new PixelConverter(parent);
+		GridLayout layout = new GridLayout(1, false);
 		layout.marginWidth = 0;
 		parent.setLayout(layout);
 
-		Composite groupContainer = new Composite(parent, SWT.NONE);
-		GridLayout groupLayout = new GridLayout(1, false);
-		groupLayout.marginWidth = 0;
-		groupLayout.marginHeight = 0;
-		groupContainer.setLayout(groupLayout);
-		groupContainer.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
-
-		Composite topGroup = new Composite(groupContainer, SWT.NONE);
-		GridLayout innerGroupLayout = new GridLayout(2, false);
-		innerGroupLayout.marginWidth = 0;
-		innerGroupLayout.marginHeight = 0;
-		topGroup.setLayout(innerGroupLayout);
+		Composite container = new Composite(parent, SWT.NONE);
+		layout = new GridLayout(1, false);
+		layout.marginWidth = 0;
+		layout.marginHeight = 0;
+		layout.verticalSpacing = pixelConverter.convertHeightInCharsToPixels(1);
+		container.setLayout(layout);
+		container.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+
+		Composite topGroup = new Composite(container, SWT.NONE);
+		layout = new GridLayout(2, false);
+		layout.marginWidth = 0;
+		layout.marginHeight = 0;
+		topGroup.setLayout(layout);
 		topGroup.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
 
-		createBooleanEditor(PreferenceConstants.MONITORING_ENABLED,
+		monitoringEnabled = createBooleanEditor(PreferenceConstants.MONITORING_ENABLED,
 				Messages.MonitoringPreferencePage_enable_thread_label, topGroup);
 
 		longEventThreshold = createIntegerEditor(
@@ -146,17 +151,20 @@ public class MonitoringPreferencePage extends FieldEditorPreferencePage implemen
 
 		createBooleanEditor(PreferenceConstants.DUMP_ALL_THREADS,
 				Messages.MonitoringPreferencePage_dump_all_threads_label, topGroup);
-		topGroup.setLayout(innerGroupLayout);
+		topGroup.setLayout(layout);
 
 		createBooleanEditor(PreferenceConstants.LOG_TO_ERROR_LOG,
 				Messages.MonitoringPreferencePage_log_freeze_events_label, topGroup);
-		topGroup.setLayout(innerGroupLayout);
+		topGroup.setLayout(layout);
 
-		final Composite bottomGroup = new Composite(groupContainer, SWT.NONE);
-		bottomGroup.setLayout(innerGroupLayout);
-		bottomGroup.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
+		final Composite bottomGroup = new Composite(container, SWT.NONE);
+		layout = new GridLayout(2, false);
+		layout.marginWidth = 0;
+		layout.marginHeight = 0;
+		bottomGroup.setLayout(layout);
+		bottomGroup.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
 
-		addField(new ListFieldEditor(PreferenceConstants.FILTER_TRACES,
+		addField(new FilterListEditor(PreferenceConstants.FILTER_TRACES,
 				Messages.MonitoringPreferencePage_filter_label, bottomGroup), bottomGroup);
 	}
 
@@ -173,18 +181,28 @@ public class MonitoringPreferencePage extends FieldEditorPreferencePage implemen
     			String preferenceName = ((FieldEditor) source).getPreferenceName();
 				if (preferenceName.equals(PreferenceConstants.MONITORING_ENABLED)) {
     				boolean enabled = Boolean.TRUE.equals(event.getNewValue());
-	    			for (Map.Entry<FieldEditor, Composite> entry : editors.entrySet()) {
-						FieldEditor editor = entry.getKey();
-	    				if (!editor.getPreferenceName().equals(PreferenceConstants.MONITORING_ENABLED)) {
-	    					editor.setEnabled(enabled, entry.getValue());
-	    				}
-	    			}
+	    			enableDependentFields(enabled);
     			}
     		}
         }
 		super.propertyChange(event);
 	}
 
+	@Override
+	protected void performDefaults() {
+		super.performDefaults();
+		enableDependentFields(monitoringEnabled.getBooleanValue());
+	}
+
+	private void enableDependentFields(boolean enable) {
+		for (Map.Entry<FieldEditor, Composite> entry : editors.entrySet()) {
+			FieldEditor editor = entry.getKey();
+			if (!editor.getPreferenceName().equals(PreferenceConstants.MONITORING_ENABLED)) {
+				editor.setEnabled(enable, entry.getValue());
+			}
+		}
+	}
+
 	private BooleanFieldEditor createBooleanEditor(String name, String labelText,
 			Composite parent) {
 		BooleanFieldEditor field = new BooleanFieldEditor(name, labelText, parent);
@@ -202,7 +220,8 @@ public class MonitoringPreferencePage extends FieldEditorPreferencePage implemen
 		editor.fillIntoGrid(parent, 2);
 		editors.put(editor, parent);
 		if (!editor.getPreferenceName().equals(PreferenceConstants.MONITORING_ENABLED)) {
-			editor.setEnabled(pluginEnabled, parent);
+			boolean enabled = preferences.getBoolean(PreferenceConstants.MONITORING_ENABLED);
+			editor.setEnabled(enabled, parent);
 		}
 		return editor;
 	}