Merge remote-tracking branch 'origin/GP-1-dragonmacher-focus-fix--SQUASHED'

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/programtree/ProgramTreePlugin.java

@@ -83,7 +83,7 @@ public class ProgramTreePlugin extends ProgramPlugin
 
 	private final static Icon NAVIGATION_ICON = Icons.NAVIGATE_ON_INCOMING_EVENT_ICON;
 
-	private HashMap<String, TreeViewProvider> providerMap;// map of view providers, key is the name
+	private Map<String, TreeViewProvider> providerMap;// map of view providers, key is the name
 	private GoToService goToService;
 	private ViewManagerService viewManagerService;
 	private ProgramTreeActionManager actionManager;
@@ -202,9 +202,7 @@ public class ProgramTreePlugin extends ProgramPlugin
 	 */
 	@Override
 	public void dispose() {
-		Iterator<String> iter = providerMap.keySet().iterator();
-		while (iter.hasNext()) {
-			String treeName = iter.next();
+		for (String treeName : providerMap.keySet()) {
 			TreeViewProvider provider = providerMap.get(treeName);
 			deregisterService(ViewProviderService.class, provider);
 			provider.dispose();
@@ -251,10 +249,8 @@ public class ProgramTreePlugin extends ProgramPlugin
 		viewProvider.writeDataState(saveState);
 
 		saveState.putInt(NUMBER_OF_VIEWS, providerMap.size());
-		Iterator<String> iter = providerMap.keySet().iterator();
 		int idx = 0;
-		while (iter.hasNext()) {
-			String treeName = iter.next();
+		for (String treeName : providerMap.keySet()) {
 			saveState.putString(TREE_NAME + "-" + idx, treeName);
 			TreeViewProvider provider = providerMap.get(treeName);
 			provider.writeDataState(saveState);
@@ -269,7 +265,6 @@ public class ProgramTreePlugin extends ProgramPlugin
 	 */
 	@Override
 	public void readDataState(SaveState saveState) {
-		viewProvider.readDataState(saveState);
 
 		int numberOfViews = saveState.getInt(NUMBER_OF_VIEWS, 0);
 
@@ -313,6 +308,23 @@ public class ProgramTreePlugin extends ProgramPlugin
 		}
 
 		selectionToggleAction.setSelected(saveState.getBoolean(TOGGLE_STATE, true));
+
+		//
+		// At this point, all tree views have been restored.  The low level components have cache
+		// that needs to get updated.  We want to maintain the order of the tree views so that the
+		// UI does not move around on the user.  Use the view names as they are stored in the 
+		// program to provide a consistent order.
+		//		
+		List<TreeViewProvider> list = new ArrayList<>();
+		String[] orderedTreeNames = currentProgram.getListing().getTreeNames();
+		for (String treeName : orderedTreeNames) {
+			TreeViewProvider provider = providerMap.get(treeName);
+			list.add(provider);
+		}
+
+		viewProvider.treeViewsRestored(list);
+		viewProvider.readDataState(saveState);
+
 	}
 
 	@Override
@@ -355,10 +367,7 @@ public class ProgramTreePlugin extends ProgramPlugin
 	private void removeStaleProviders(ArrayList<TreeViewProvider> providerList) {
 		HashMap<String, TreeViewProvider> map = new HashMap<>(providerMap);
 
-		// remove views from the map that are not in the providerList
-		Iterator<String> iter = map.keySet().iterator();
-		while (iter.hasNext()) {
-			String treeName = iter.next();
+		for (String treeName : map.keySet()) {
 			TreeViewProvider provider = map.get(treeName);
 			if (!providerList.contains(provider)) {
 				deregisterService(ViewProviderService.class, provider);
@@ -610,9 +619,7 @@ public class ProgramTreePlugin extends ProgramPlugin
 	 * fragment was moved; update all the view maps.
 	 */
 	void fragmentMoved() {
-		Iterator<String> iter = providerMap.keySet().iterator();
-		while (iter.hasNext()) {
-			String treeName = iter.next();
+		for (String treeName : providerMap.keySet()) {
 			TreeViewProvider provider = providerMap.get(treeName);
 			provider.notifyListeners();
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/programtree/TreeViewProvider.java

@@ -70,6 +70,11 @@ class TreeViewProvider implements ViewProviderService {
 		});
 	}
 
+	@Override
+	public String toString() {
+		return treePanel.getTreeName();
+	}
+
 	@Override
 	public JComponent getViewComponent() {
 		return treePanel;

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/programtree/ViewManagerComponentProvider.java

@@ -17,13 +17,13 @@ package ghidra.app.plugin.core.programtree;
 
 import java.awt.event.MouseEvent;
 import java.util.ArrayList;
+import java.util.Collection;
 
 import javax.swing.JComponent;
 
 import docking.*;
 import ghidra.app.context.ProgramActionContext;
 import ghidra.app.services.ViewManagerService;
-import ghidra.framework.model.DomainObject;
 import ghidra.framework.options.SaveState;
 import ghidra.framework.plugintool.ComponentProviderAdapter;
 import ghidra.framework.plugintool.PluginTool;
@@ -35,7 +35,6 @@ import ghidra.util.HelpLocation;
 public class ViewManagerComponentProvider extends ComponentProviderAdapter
 		implements ViewManagerService, ViewChangeListener {
 
-	private static final String OLD_NAME = "ProgramTreePlugin";
 	private static final String NAME = "Program Tree";
 
 	public static final String CURRENT_VIEW = "Current Viewname";
@@ -79,14 +78,12 @@ public class ViewManagerComponentProvider extends ComponentProviderAdapter
 		viewPanel.addView(service);
 		String viewName = service.getViewName();
 		if (viewName.equals(restoredViewName)) {
-			// state is being restored, so set the current view now
+			viewPanel.setCurrentView(restoredViewName);
 			restoredViewName = null;
-			viewPanel.setCurrentView(viewName);
 		}
 		else if (viewPanel.getNumberOfViews() == 1) {
-			viewName = viewPanel.getCurrentViewName();
-
 			// we only have one view, so force view map events to go out
+			viewName = viewPanel.getCurrentViewName();
 			viewPanel.setCurrentView(viewName);
 		}
 	}
@@ -117,8 +114,7 @@ public class ViewManagerComponentProvider extends ComponentProviderAdapter
 
 	@Override
 	public void viewChanged(AddressSetView addrSet) {
-		for (int i = 0; i < listeners.size(); i++) {
-			ViewChangeListener l = listeners.get(i);
+		for (ViewChangeListener l : listeners) {
 			l.viewChanged(addrSet);
 		}
 	}
@@ -145,23 +141,15 @@ public class ViewManagerComponentProvider extends ComponentProviderAdapter
 	}
 
 	void readDataState(SaveState saveState) {
-		if (saveState != null) {
-			restoredViewName = saveState.getString(CURRENT_VIEW, null);
-			if (viewPanel.setCurrentView(restoredViewName)) {
-				restoredViewName = null; // have the view
-			}
-			// else wait for serviceAdded to restore the view...
+		String savedCurrentView = saveState.getString(CURRENT_VIEW, null);
+		if (!viewPanel.setCurrentView(savedCurrentView)) {
+			// the view to has not yet been added from a call to serviceAdded(); save for later
+			restoredViewName = savedCurrentView;
 		}
 	}
 
-	Object getUndoRedoState(DomainObject domainObject) {
-		SaveState saveState = new SaveState();
-		writeDataState(saveState);
-		return saveState;
-	}
-
-	void restoreUndoRedoState(DomainObject domainObject, Object state) {
-		readDataState((SaveState) state);
+	void treeViewsRestored(Collection<TreeViewProvider> treeViews) {
+		viewPanel.treeViewsRestored(treeViews);
 	}
 
 	/**
@@ -222,5 +210,4 @@ public class ViewManagerComponentProvider extends ComponentProviderAdapter
 	public void setCurrentProgram(Program program) {
 		currentProgram = program;
 	}
-
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/programtree/ViewPanel.java

@@ -17,21 +17,23 @@ package ghidra.app.plugin.core.programtree;
 
 import java.awt.*;
 import java.awt.event.MouseEvent;
+import java.util.Collection;
 import java.util.HashMap;
 
 import javax.swing.*;
 import javax.swing.event.ChangeEvent;
 import javax.swing.event.ChangeListener;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.ActionContext;
-import docking.EditListener;
 import docking.action.DockingAction;
 import docking.action.MenuData;
+import docking.widgets.OptionDialog;
 import docking.widgets.tabbedpane.DockingTabRenderer;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.address.AddressSetView;
 import ghidra.program.util.ProgramLocation;
-import ghidra.util.Msg;
 import ghidra.util.exception.AssertException;
 
 /**
@@ -349,6 +351,15 @@ class ViewPanel extends JPanel implements ChangeListener {
 		setPreferredSize(new Dimension(200, 300));
 	}
 
+	void treeViewsRestored(Collection<TreeViewProvider> treeViews) {
+
+		map.clear();
+
+		for (TreeViewProvider treeProvider : treeViews) {
+			addView(treeProvider);
+		}
+	}
+
 	/**
 	 * If the panel is active, then set the current view to be active and all
 	 * others to be inactive.
@@ -448,52 +459,24 @@ class ViewPanel extends JPanel implements ChangeListener {
 	 */
 	private void renameView() {
 		ViewProviderService vps = getCurrentViewProvider();
-		int tabIndex = tabbedPane.getSelectedIndex();
 		String oldName = vps.getViewName();
-		Rectangle rect = tabbedPane.getBoundsAt(tabIndex);
-		tool.showEditWindow(oldName, tabbedPane, rect, new RenameListener(vps, tabIndex));
-	}
+		String newName =
+			OptionDialog.showInputSingleLineDialog(tabbedPane, "Rename Tab", "New name:", oldName);
 
-//==================================================================================================
-// Inner Classes
-//==================================================================================================	
-
-	private class RenameListener implements EditListener {
-
-		private ViewProviderService vps;
-		private int tabIndex;
-
-		RenameListener(ViewProviderService vps, int tabIndex) {
-			this.vps = vps;
-			this.tabIndex = tabIndex;
+		if (StringUtils.isBlank(newName)) {
+			return;
 		}
 
-		@Override
-		public void editCompleted(String newName) {
-
-			if (newName.length() == 0) {
-
-				Msg.showError(getClass(), null, "Invalid Name", "Please enter a valid name.");
-
-				String oldName = vps.getViewName();
-				Rectangle rect = tabbedPane.getBoundsAt(tabIndex);
-				tool.showEditWindow(oldName, tabbedPane, rect, this);
-				return;
+		if (!newName.equals(oldName)) {
+			if (vps.viewRenamed(newName)) {
+				int selectedIndex = tabbedPane.getSelectedIndex();
+				tabbedPane.setTitleAt(selectedIndex, newName);
+				DockingTabRenderer renderer =
+					(DockingTabRenderer) tabbedPane.getTabComponentAt(selectedIndex);
+				renderer.setTitle(newName, newName);
+				map.remove(oldName);
+				map.put(newName, vps);
 			}
-
-			String oldName = vps.getViewName();
-			if (!newName.equals(oldName)) {
-				if (vps.viewRenamed(newName)) {
-					int selectedIndex = tabbedPane.getSelectedIndex();
-					tabbedPane.setTitleAt(selectedIndex, newName);
-					DockingTabRenderer renderer =
-						(DockingTabRenderer) tabbedPane.getTabComponentAt(selectedIndex);
-					renderer.setTitle(newName, newName);
-					map.remove(oldName);
-					map.put(newName, vps);
-				}
-			}
-
 		}
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/programtree/ViewManagerPluginTest.java

@@ -17,8 +17,8 @@ package ghidra.app.plugin.core.programtree;
 
 import static org.junit.Assert.*;
 
-import java.awt.*;
-import java.awt.event.ActionListener;
+import java.awt.Component;
+import java.awt.Container;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.swing.*;
@@ -26,8 +26,8 @@ import javax.swing.*;
 import org.junit.*;
 
 import docking.DefaultActionContext;
-import docking.EditWindow;
 import docking.action.DockingActionIf;
+import docking.widgets.dialogs.InputDialog;
 import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin;
 import ghidra.app.services.ProgramManager;
 import ghidra.app.services.ViewManagerService;
@@ -254,6 +254,9 @@ public class ViewManagerPluginTest extends AbstractGhidraHeadedIntegrationTest {
 
 	@Test
 	public void testDeleteView() throws Exception {
+
+		env.showTool();
+
 		// delete the "Tree Two" view
 		setCurrentViewProvider("Tree Two");
 
@@ -359,47 +362,35 @@ public class ViewManagerPluginTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(provider.getCurrentView().hasSameAddresses(vps.getCurrentView()));
 	}
 
-	// NOTE: this test has been commented out because it fails consistently due to timing errors.
-	// However, this test will almost always run successfully after the first time it is run.  So,
-	// this test can be uncommented and run to test the functionality of view renaming when 
-	// changes are made.
-	public void dontTestRenameView() throws Exception {
+	@Test
+	public void testRenameView() throws Exception {
 		env.showTool();
-
-		final DockingActionIf renameAction = getAction(plugin, "Rename Tree View");
-
 		waitForTasks();
-		waitForSwing();
 
 		setCurrentViewProvider(DEFAULT_TREE_NAME);
-		SwingUtilities
-				.invokeAndWait(() -> renameAction.actionPerformed(new DefaultActionContext()));
 
-		EditWindow editWindow = findEditWindow(tool.getToolFrame());
-		assertNotNull(editWindow);
+		DockingActionIf renameAction = getAction(plugin, "Rename Tree View");
+		performAction(renameAction, false);
 
-		final JTextField textField = (JTextField) getInstanceField("textField", editWindow);
-		SwingUtilities.invokeAndWait(() -> {
-			textField.setText("My Tree");
-			ActionListener[] listeners = textField.getActionListeners();
-			listeners[0].actionPerformed(null);
-		});
+		InputDialog dialog = waitForDialogComponent(InputDialog.class);
+		dialog.setValue("My Tree");
+		pressButtonByText(dialog, "OK");
+		waitForProgram(program);
 
-		program.flushEvents();
-
-		ViewProviderService vps = provider.getCurrentViewProvider();
+		ViewProviderService vps = runSwing(() -> provider.getCurrentViewProvider());
 		assertEquals("My Tree", vps.getViewName());
 		assertNull(program.getListing().getRootModule(DEFAULT_TREE_NAME));
 		assertTrue(provider.getCurrentView().hasSameAddresses(cb.getView()));
 		assertTrue(provider.getCurrentView().hasSameAddresses(vps.getCurrentView()));
 
 		undo(program);
-		vps = provider.getCurrentViewProvider();
+		vps = runSwing(() -> provider.getCurrentViewProvider());
 		assertEquals(DEFAULT_TREE_NAME, vps.getViewName());
 		assertNotNull(program.getListing().getRootModule(DEFAULT_TREE_NAME));
 
 		redo(program);
-		vps = provider.getCurrentViewProvider();
+		provider.getCurrentViewProvider();
+		vps = runSwing(() -> provider.getCurrentViewProvider());
 		assertEquals("My Tree", vps.getViewName());
 		assertNull(program.getListing().getRootModule(DEFAULT_TREE_NAME));
 	}
@@ -409,25 +400,19 @@ public class ViewManagerPluginTest extends AbstractGhidraHeadedIntegrationTest {
 
 		env.showTool();
 
-		final DockingActionIf renameAction = getAction(plugin, "Rename Tree View");
-
 		waitForTasks();
 		waitForSwing();
 
 		setCurrentViewProvider(DEFAULT_TREE_NAME);
-		SwingUtilities
-				.invokeAndWait(() -> renameAction.actionPerformed(new DefaultActionContext()));
 
-		EditWindow editWindow = findEditWindow(tool.getToolFrame());
-		assertNotNull(editWindow);
-		final JTextField textField = (JTextField) getInstanceField("textField", editWindow);
-		SwingUtilities.invokeAndWait(() -> {
-			textField.requestFocus();
-			textField.setText("Main Tree");
-			ActionListener[] listeners = textField.getActionListeners();
-			listeners[0].actionPerformed(null);
-		});
-		program.flushEvents();
+		DockingActionIf renameAction = getAction(plugin, "Rename Tree View");
+		performAction(renameAction, false);
+
+		InputDialog dialog = waitForDialogComponent(InputDialog.class);
+		dialog.setValue("Main Tree");
+		pressButtonByText(dialog, "OK");
+		waitForProgram(program);
+
 		ViewProviderService vps = provider.getCurrentViewProvider();
 		assertEquals(DEFAULT_TREE_NAME, vps.getViewName());
 		assertTrue(provider.getCurrentView().hasSameAddresses(cb.getView()));
@@ -466,16 +451,6 @@ public class ViewManagerPluginTest extends AbstractGhidraHeadedIntegrationTest {
 		return null;
 	}
 
-	private EditWindow findEditWindow(Window window) {
-		Window[] w = window.getOwnedWindows();
-		for (Window element : w) {
-			if (element instanceof EditWindow) {
-				return (EditWindow) element;
-			}
-		}
-		return null;
-	}
-
 	private void findTabbedPane() {
 		Component[] comp = viewPanel.getComponents();
 		for (Component element : comp) {