Merge remote-tracking branch

'origin/GT-3233-dragonmacher-fg-navigation-history' (closes #1115)

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/DisassociateAction.java

@@ -35,7 +35,7 @@ import ghidra.util.exception.CancelledException;
 import ghidra.util.task.*;
 
 public class DisassociateAction extends DockingAction {
-	public static final String MENU_NAME = "Disassociate Datatypes from";
+	public static final String MENU_NAME = "Disassociate Datatypes From";
 
 	private final SourceArchive sourceArchive;
 	private final DataTypeManager dtm;

Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph.html

@@ -786,9 +786,31 @@
         graph groups changes (during a group or ungroup operation)</LI>
 
         <LI><B>Never</B> - never perform a relayout of the graph automatically</LI>
-      </UL><BR>
+      </UL>
       <BR>
-       
+      <BR>      
+      
+      
+      <P>The <B>Navigation History</B> option determines how the navigation history 
+      will be updated when using the Function Graph.  The values are:</P>
+      
+      <UL>
+        <LI><B>Navigation Events</B> - save a history entry when a navigation takes place
+	    	(e.g., double-click or Go To event).  This setting will record less history.  Further, 
+	    	this setting works the same as the Tool's general history saving mechanism.  This 
+	    	setting should be preferred if you wish to use the navigation actions to go back
+	    	to previously visited <b>functions</b> more than individual addresses.
+	    </LI>
+
+        <LI><B>Vertex Changes</B> - save a history entry each time a new vertex is selected.  This
+        setting allows users to move throughout the graph view while using the navigation actions
+        to go back to previously visited vertices.  This setting will create a much larger
+        and more detailed history list.
+        </LI>
+      </UL>
+      <BR>
+      <BR>
+      
 
       <P>The <B>Scroll Wheel Pans</B> option signals to move the graph vertical when scrolling the
       mouse scroll wheel. Disabling this option restores the original function graph scroll wheel

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGController.java

@@ -311,22 +311,31 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi
 		FunctionGraph graph = functionGraphData.getFunctionGraph();
 		FGVertex newFocusedVertex = graph.getFocusedVertex();
 		boolean vertexChanged = lastUserNavigatedVertex != newFocusedVertex;
+		boolean updateHistory = false;
 		if (vertexChanged) {
-			// put the navigation on the history stack if we've changed nodes (this is the
+			if (shouldSaveVertexChanges()) {
-			// location we are leaving)
+				// put the navigation on the history stack if we've changed nodes (this is the
-			provider.saveLocationToHistory();
+				// location we are leaving)
+				provider.saveLocationToHistory();
+				updateHistory = true;
+			}
 			lastUserNavigatedVertex = newFocusedVertex;
 		}
 
 		viewSettings.setLocation(loc);
 		provider.graphLocationChanged(loc);
 
-		if (vertexChanged) {
+		if (updateHistory) {
 			// put the new location on the history stack now that we've updated the provider
 			provider.saveLocationToHistory();
 		}
 	}
 
+	private boolean shouldSaveVertexChanges() {
+		return functionGraphOptions
+				.getNavigationHistoryChoice() == NavigationHistoryChoices.VERTEX_CHANGES;
+	}
+
 	@Override
 	public void programSelectionChanged(ProgramSelection selection) {
 		// We need to translate the given selection (which is from a single vertex) to the current

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphOptions.java

@@ -41,8 +41,21 @@ public class FunctionGraphOptions extends VisualGraphOptions {
 		"Edge Color - Unconditional Jump ";
 	private static final String EDGE_COLOR_CONDITIONAL_JUMP_KEY = "Edge Color - Conditional Jump ";
 
+	//@formatter:off
+	private static final String NAVIGATION_HISTORY_KEY = "Navigation History";	
+	private static final String NAVIGATION_HISTORY_DESCRIPTION =
+		"Determines how the navigation history will be updated when using the Function Graph. " +
+		"The basic options are:" +
+		"<ul>" +
+	    "<li><b>Navigation Events</b> - save a history entry when a navigation takes place " +
+	    	"(e.g., double-click or Go To event)</li>" +
+	    "<li><b>Vertex Changes</b> - save a history entry each time a new vertex is selected</li>" +
+	    "</ul>" +
+	    "<b><i>See help for more</i></b>";
+	//@formatter:on
+
 	private static final String USE_FULL_SIZE_TOOLTIP_KEY = "Use Full-size Tooltip";
-	private static final String USE_FULL_SIZE_TOOLTIP_DESCRIPTION = "Signals to use the " + "" +
+	private static final String USE_FULL_SIZE_TOOLTIP_DESCRIPTION = "Signals to use the " +
 		"full-size vertex inside of the tooltip popup.  When enabled the tooltip vertex will " +
 		"use the same format size as the Listing.  When disabled, the vertex will use the " +
 		"same format size as in the Function Graph.";
@@ -85,7 +98,9 @@ public class FunctionGraphOptions extends VisualGraphOptions {
 
 	private boolean useFullSizeTooltip = false;
 
-	private RelayoutOption relayoutOption = RelayoutOption.NEVER;
+	private RelayoutOption relayoutOption = RelayoutOption.VERTEX_GROUPING_CHANGES;
+	private NavigationHistoryChoices navigationHistoryChoice =
+		NavigationHistoryChoices.VERTEX_CHANGES;
 
 	private Map<String, FGLayoutOptions> layoutOptionsByName = new HashMap<>();
 
@@ -125,6 +140,10 @@ public class FunctionGraphOptions extends VisualGraphOptions {
 		return relayoutOption;
 	}
 
+	public NavigationHistoryChoices getNavigationHistoryChoice() {
+		return navigationHistoryChoice;
+	}
+
 	public boolean useFullSizeTooltip() {
 		return useFullSizeTooltip;
 	}
@@ -134,9 +153,12 @@ public class FunctionGraphOptions extends VisualGraphOptions {
 		HelpLocation help = new HelpLocation(OWNER, "Options");
 		options.setOptionsHelpLocation(help);
 
-		options.registerOption(RELAYOUT_OPTIONS_KEY, RelayoutOption.VERTEX_GROUPING_CHANGES, help,
+		options.registerOption(RELAYOUT_OPTIONS_KEY, relayoutOption, help,
 			RELAYOUT_OPTIONS_DESCRIPTION);
 
+		options.registerOption(NAVIGATION_HISTORY_KEY, navigationHistoryChoice, help,
+			NAVIGATION_HISTORY_DESCRIPTION);
+
 		options.registerOption(SHOW_ANIMATION_OPTIONS_KEY, useAnimation(), help,
 			SHOW_ANIMATION_DESCRIPTION);
 
@@ -201,8 +223,10 @@ public class FunctionGraphOptions extends VisualGraphOptions {
 		fallthroughEdgeHighlightColor =
 			options.getColor(EDGE_FALLTHROUGH_HIGHLIGHT_COLOR_KEY, fallthroughEdgeHighlightColor);
 
-		relayoutOption =
+		relayoutOption = options.getEnum(RELAYOUT_OPTIONS_KEY, relayoutOption);
-			options.getEnum(RELAYOUT_OPTIONS_KEY, RelayoutOption.VERTEX_GROUPING_CHANGES);
+
+		navigationHistoryChoice =
+			options.getEnum(NAVIGATION_HISTORY_KEY, NavigationHistoryChoices.VERTEX_CHANGES);
 
 		useAnimation = options.getBoolean(SHOW_ANIMATION_OPTIONS_KEY, useAnimation);
 

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/NavigationHistoryChoices.java

@@ -0,0 +1,39 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.app.plugin.core.functiongraph.mvc;
+
+/**
+ * An enum to be used with Function Graph options
+ */
+public enum NavigationHistoryChoices {
+
+	/** A navigation event is a double-click or Go To operation */
+	NAVIGATION_EVENTS("Navigation Events"),
+
+	/** When a new vertex is focused */
+	VERTEX_CHANGES("Vertex Changes");
+
+	private String displayName;
+
+	NavigationHistoryChoices(String displayName) {
+		this.displayName = displayName;
+	}
+
+	@Override
+	public String toString() {
+		return displayName;
+	}
+}

Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java

@@ -1865,13 +1865,21 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte
 		return groupVertex;
 	}
 
-	private boolean isUncollapsed(final FGVertex vertex) {
+	protected boolean isUncollapsed(final FGVertex vertex) {
 		final AtomicReference<Boolean> reference = new AtomicReference<>();
 		runSwing(() -> reference.set(vertex.isUncollapsedGroupMember()));
 		return reference.get();
 	}
 
-	private void pickVertices(final Set<FGVertex> vertices) {
+	protected void pickVertex(FGVertex v) {
+		runSwing(() -> {
+			PickedState<FGVertex> pickedState = getPickedState();
+			pickedState.clear();
+			pickedState.pick(v, true);
+		});
+	}
+
+	protected void pickVertices(final Set<FGVertex> vertices) {
 		runSwing(() -> {
 			PickedState<FGVertex> pickedState = getPickedState();
 			pickedState.clear();
@@ -2264,9 +2272,12 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte
 
 	protected void goTo(String address) {
 		Address addr = getAddress(address);
-		GoToService goToService = tool.getService(GoToService.class);
+		goTo(addr);
-		goToService.goTo(addr);
+	}
 
+	protected void goTo(Address address) {
+		GoToService goToService = tool.getService(GoToService.class);
+		goToService.goTo(address);
 		waitForBusyGraph();
 	}
 

Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java

@@ -23,6 +23,8 @@ import java.awt.datatransfer.Clipboard;
 import java.awt.datatransfer.Transferable;
 import java.awt.geom.Point2D;
 import java.util.*;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import org.junit.*;
 
@@ -36,6 +38,8 @@ import edu.uci.ics.jung.visualization.util.Caching;
 import generic.test.TestUtils;
 import ghidra.app.cmd.label.AddLabelCmd;
 import ghidra.app.events.ProgramSelectionPluginEvent;
+import ghidra.app.nav.LocationMemento;
+import ghidra.app.nav.Navigatable;
 import ghidra.app.plugin.core.colorizer.ColorizingPlugin;
 import ghidra.app.plugin.core.colorizer.ColorizingService;
 import ghidra.app.plugin.core.functiongraph.graph.*;
@@ -43,8 +47,7 @@ import ghidra.app.plugin.core.functiongraph.graph.vertex.FGVertex;
 import ghidra.app.plugin.core.functiongraph.mvc.*;
 import ghidra.app.plugin.core.navigation.GoToAddressLabelPlugin;
 import ghidra.app.plugin.core.navigation.NextPrevAddressPlugin;
-import ghidra.app.services.BlockModelService;
+import ghidra.app.services.*;
-import ghidra.app.services.ProgramManager;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSetView;
 import ghidra.program.model.block.*;
@@ -701,7 +704,136 @@ public class FunctionGraphPlugin1Test extends AbstractFunctionGraphTest {
 		assertZoomedIn();
 	}
 
-	protected void doTestLabelChangeAtVertexEntryUpdatesTitle() {
+	@Test
+	public void testNavigationHistory_VertexChangesOption() throws Exception {
+
+		setNavigationHistoryOption(NavigationHistoryChoices.VERTEX_CHANGES);
+
+		FGData graphData = getFunctionGraphData();
+		FunctionGraph graph = graphData.getFunctionGraph();
+		Collection<FGVertex> vertices = graph.getVertices();
+
+		FGVertex start = getFocusedVertex();
+
+		Iterator<FGVertex> it = vertices.iterator();
+		FGVertex v1 = it.next();
+		pickVertex(v1);
+
+		FGVertex v2 = it.next();
+		pickVertex(v2);
+
+		FGVertex v3 = it.next();
+		pickVertex(v3);
+
+		assertInHistory(start, v1, v2);
+	}
+
+	@Test
+	public void testNavigationHistory_NavigationEventsOption() throws Exception {
+
+		setNavigationHistoryOption(NavigationHistoryChoices.NAVIGATION_EVENTS);
+
+		FGVertex start = getFocusedVertex();
+
+		FGVertex v1 = vertex("01004178");
+		pickVertex(v1);
+
+		FGVertex v2 = vertex("01004192");
+		pickVertex(v2);
+
+		FGVertex v3 = vertex("010041a4");
+		pickVertex(v3);
+
+		assertInHistory(start);
+		assertNotInHistory(v1, v2);
+
+		//
+		// Now leave the function and verify the old function is in the history
+		//
+		Address ghidra = getAddress("0x01002cf5");
+		goTo(ghidra);
+
+		Address foo = getAddress("0x01002339");
+		goTo(foo);
+
+		assertInHistory(start.getVertexAddress(), ghidra);
+	}
+
+//==================================================================================================
+// Private Methods
+//==================================================================================================	
+
+	private void assertNotInHistory(FGVertex... vertices) {
+
+		List<Address> vertexAddresses =
+			Arrays.stream(vertices)
+					.map(v -> v.getVertexAddress())
+					.collect(Collectors.toList());
+		assertNotInHistory(vertexAddresses);
+	}
+
+	private void assertNotInHistory(List<Address> addresses) {
+
+		GoToService goTo = tool.getService(GoToService.class);
+		Navigatable navigatable = goTo.getDefaultNavigatable();
+
+		NavigationHistoryService service = tool.getService(NavigationHistoryService.class);
+		List<LocationMemento> locations = service.getPreviousLocations(navigatable);
+
+		List<Address> actualAddresses =
+			locations.stream()
+					.map(memento -> memento.getProgramLocation().getAddress())
+					.collect(Collectors.toList());
+
+		for (Address a : addresses) {
+			assertFalse("Vertex address should not be in the history list: " + a + ".\nHistory: " +
+				actualAddresses + "\nNavigated vertices: " + Arrays.asList(addresses),
+				actualAddresses.contains(a));
+		}
+	}
+
+	private void assertInHistory(FGVertex... vertices) {
+
+		List<Address> vertexAddresses =
+			Arrays.stream(vertices)
+					.map(v -> v.getVertexAddress())
+					.collect(Collectors.toList());
+		assertInHistory(vertexAddresses);
+	}
+
+	private void assertInHistory(Address... addresses) {
+		assertInHistory(Arrays.asList(addresses));
+	}
+
+	private void assertInHistory(List<Address> addresses) {
+
+		GoToService goTo = tool.getService(GoToService.class);
+		Navigatable navigatable = goTo.getDefaultNavigatable();
+
+		NavigationHistoryService service = tool.getService(NavigationHistoryService.class);
+		List<LocationMemento> locations = service.getPreviousLocations(navigatable);
+		assertTrue("Vertex locations not added to history", addresses.size() <= locations.size());
+
+		List<Address> actualAddresses =
+			locations.stream()
+					.map(memento -> memento.getProgramLocation().getAddress())
+					.collect(Collectors.toList());
+
+		for (Address a : addresses) {
+
+			assertTrue("Vertex address should be in the history list: " + a + ".\nHistory: " +
+				actualAddresses + "\nNavigated vertices: " + addresses,
+				actualAddresses.contains(a));
+		}
+	}
+
+	private void setNavigationHistoryOption(NavigationHistoryChoices choice) throws Exception {
+		FGController controller = getFunctionGraphController();
+		FunctionGraphOptions options = controller.getFunctionGraphOptions();
+		setInstanceField("navigationHistoryChoice", options, choice);
+	}
+
+	private void doTestLabelChangeAtVertexEntryUpdatesTitle() {
 		// get the graph contents
 		FGData graphData = getFunctionGraphData();
 		assertNotNull(graphData);
@@ -727,7 +859,7 @@ public class FunctionGraphPlugin1Test extends AbstractFunctionGraphTest {
 		assertTrue(updatedTitle.indexOf(testName.getMethodName()) != -1);
 	}
 
-	protected void doTestRelayout(boolean fullReload) throws Exception {
+	private void doTestRelayout(boolean fullReload) throws Exception {
 
 		//
 		// This test covers navigation, which relies on the provider being focused to work

Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/FunctionGraphPluginScreenShots.java

@@ -15,7 +15,7 @@
  */
 package help.screenshot;
 
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.*;
 
 import java.awt.*;
 import java.awt.geom.Point2D;
@@ -40,7 +40,6 @@ import docking.widgets.dialogs.MultiLineInputDialog;
 import edu.uci.ics.jung.graph.Graph;
 import edu.uci.ics.jung.visualization.VisualizationServer;
 import edu.uci.ics.jung.visualization.VisualizationViewer;
-import edu.uci.ics.jung.visualization.picking.PickedState;
 import generic.test.TestUtils;
 import ghidra.app.cmd.function.DeleteFunctionCmd;
 import ghidra.app.cmd.label.AddLabelCmd;
@@ -774,24 +773,6 @@ public class FunctionGraphPluginScreenShots extends AbstractFunctionGraphTest {
 		pickVertices(new HashSet<>(Arrays.asList(vertices)));
 	}
 
-	private void pickVertices(final Set<FGVertex> vertices) {
-		runSwing(() -> {
-			PickedState<FGVertex> pickedState = getPickedState();
-			pickedState.clear();
-
-			for (FGVertex vertex : vertices) {
-				pickedState.pick(vertex, true);
-			}
-		});
-	}
-
-	private PickedState<FGVertex> getPickedState() {
-		FGComponent functionGraphViewer = getGraphComponent();
-		VisualizationViewer<FGVertex, FGEdge> primaryViewer =
-			functionGraphViewer.getPrimaryViewer();
-		return primaryViewer.getPickedVertexState();
-	}
-
 	private JComponent getComponent(final FGVertex vertex) {
 		final AtomicReference<JComponent> reference = new AtomicReference<>();
 		runSwing(() -> reference.set(vertex.getComponent()));