GP-0 test fixes related to open/close functions.

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/listingpanel/ProgramBigListingModel.java

@@ -167,7 +167,7 @@ public class ProgramBigListingModel implements ListingModel, FormatModelListener
 			format = formatMgr.getDividerModel();
 			format.addLayouts(list, 0, new AddressProxy(this, addr));
 		}
-		if (cu != null && function != null) {
+		if (cu != null) {
 			format = formatMgr.getPlateFormat();
 			format.addLayouts(list, 0, new CodeUnitProxy(this, program, cu));
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/util/InMemoryOpenCloseManager.java

@@ -25,24 +25,17 @@ import ghidra.program.model.address.Address;
  * state can be set and then a set of address is kept for the locations that are the opposite
  * of the default.
  */
-public class InMemoryOpenCloseManager {
+public class InMemoryOpenCloseManager implements OpenCloseManager {
 	private boolean openByDefault = true;
 	private Set<Address> addresses = new HashSet<>();
 
-	/**
-	 * Checks if the state is "open" for the given address.
-	 * @param address the address to test
-	 * @return true if the state of the given address is "open"
-	 */
+	@Override
 	public boolean isOpen(Address address) {
 		boolean contains = addresses.contains(address);
 		return openByDefault ? !contains : contains;
 	}
 
-	/**
-	 * Sets the state at the given address to be "open".
-	 * @param address the address to set "open"
-	 */
+	@Override
 	public void open(Address address) {
 		if (openByDefault) {
 			addresses.remove(address);
@@ -52,10 +45,7 @@ public class InMemoryOpenCloseManager {
 		}
 	}
 
-	/**
-	 * Sets the state at the given address to be "closed".
-	 * @param address the address to set "closed"
-	 */
+	@Override
 	public void close(Address address) {
 		if (openByDefault) {
 			addresses.add(address);
@@ -65,27 +55,18 @@ public class InMemoryOpenCloseManager {
 		}
 	}
 
-	/**
-	 * Checks if the default state is "open".
-	 * @return true if the default state for addresses is "open"
-	 */
+	@Override
 	public boolean isOpenByDefault() {
 		return openByDefault;
 	}
 
-	/**
-	 * Sets all address to "open" (Makes "open" the default state and clears all individual
-	 * settings.
-	 */
+	@Override
 	public void openAll() {
 		openByDefault = true;
 		addresses.clear();
 	}
 
-	/**
-	 * Sets all address to "closed" (Makes "closed" the default state and clears all individual
-	 * settings.
-	 */
+	@Override
 	public void closeAll() {
 		openByDefault = false;
 		addresses.clear();

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/util/OpenCloseManager.java

@@ -0,0 +1,62 @@
+/* ###
+ * 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.util.viewer.util;
+
+import ghidra.program.model.address.Address;
+
+/**
+ * Interface for tracking the open/close state at an address.
+ */
+public interface OpenCloseManager {
+
+	/**
+	 * Checks if the state is "open" for the given address.
+	 * @param address the address to test
+	 * @return true if the state of the given address is "open"
+	 */
+	public boolean isOpen(Address address);
+
+	/**
+	 * Sets the state at the given address to be "open".
+	 * @param address the address to set "open"
+	 */
+	public void open(Address address);
+
+	/**
+	 * Sets the state at the given address to be "closed".
+	 * @param address the address to set "closed"
+	 */
+	public void close(Address address);
+
+	/**
+	 * Checks if the default state is "open".
+	 * @return true if the default state for addresses is "open"
+	 */
+	public boolean isOpenByDefault();
+
+	/**
+	 * Sets all address to "open" (Makes "open" the default state and clears all individual
+	 * settings.
+	 */
+	public void openAll();
+
+	/**
+	 * Sets all address to "closed" (Makes "closed" the default state and clears all individual
+	 * settings.
+	 */
+	public void closeAll();
+
+}

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/util/PersistentOpenCloseManager.java

@@ -16,7 +16,6 @@
 package ghidra.app.util.viewer.util;
 
 import ghidra.program.model.address.Address;
-import ghidra.program.model.listing.Program;
 import ghidra.program.model.listing.ProgramUserData;
 import ghidra.program.model.util.VoidPropertyMap;
 
@@ -25,7 +24,7 @@ import ghidra.program.model.util.VoidPropertyMap;
  * open/close state for that address. Currently used for persisting the open/close state
  * of functions in the listing.
  */
-public class PersistentOpenCloseManager {
+public class PersistentOpenCloseManager implements OpenCloseManager {
 	private boolean openByDefault = true;
 	private VoidPropertyMap booleanProperty;
 	private ProgramUserData programUserData;
@@ -36,9 +35,9 @@ public class PersistentOpenCloseManager {
 	private boolean cachedResult;
 	private String defaultOpenClosePropertyname;
 
-	public PersistentOpenCloseManager(Program program, String owner, String propertyName) {
+	public PersistentOpenCloseManager(ProgramUserData data, String owner, String propertyName) {
 		this.defaultOpenClosePropertyname = propertyName + "Default";
-		programUserData = program.getProgramUserData();
+		programUserData = data;
 
 		int tx = programUserData.startTransaction();
 		try {
@@ -55,11 +54,7 @@ public class PersistentOpenCloseManager {
 		openByDefault = functionState.equals("Open");
 	}
 
-	/**
-	 * Checks if the state is "open" for the given address.
-	 * @param address the address to test
-	 * @return true if the state of the given address is "open"
-	 */
+	@Override
 	public boolean isOpen(Address address) {
 		if (address.equals(cachedAddress)) {
 			return cachedResult;
@@ -70,10 +65,7 @@ public class PersistentOpenCloseManager {
 		return cachedResult;
 	}
 
-	/**
-	 * Sets the state at the given address to be "open".
-	 * @param address the address to set "open"
-	 */
+	@Override
 	public void open(Address address) {
 		cachedAddress = null;
 		if (openByDefault) {
@@ -84,10 +76,7 @@ public class PersistentOpenCloseManager {
 		}
 	}
 
-	/**
-	 * Sets the state at the given address to be "closed".
-	 * @param address the address to set "closed"
-	 */
+	@Override
 	public void close(Address address) {
 		cachedAddress = null;
 		if (openByDefault) {
@@ -98,18 +87,12 @@ public class PersistentOpenCloseManager {
 		}
 	}
 
-	/**
-	 * Checks if the default state is "open".
-	 * @return true if the default state for addresses is "open"
-	 */
+	@Override
 	public boolean isOpenByDefault() {
 		return openByDefault;
 	}
 
-	/**
-	 * Sets all address to "open" (Makes "open" the default state and clears all individual
-	 * settings.
-	 */
+	@Override
 	public void openAll() {
 		cachedAddress = null;
 		openByDefault = true;
@@ -117,10 +100,7 @@ public class PersistentOpenCloseManager {
 		programUserData.setStringProperty(defaultOpenClosePropertyname, "Open");
 	}
 
-	/**
-	 * Sets all address to "closed" (Makes "closed" the default state and clears all individual
-	 * settings.
-	 */
+	@Override
 	public void closeAll() {
 		cachedAddress = null;
 		openByDefault = false;

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/util/ProgramOpenCloseManager.java

@@ -22,8 +22,7 @@ import javax.swing.event.ChangeListener;
 
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSetView;
-import ghidra.program.model.listing.Data;
-import ghidra.program.model.listing.Program;
+import ghidra.program.model.listing.*;
 import ghidra.util.task.TaskMonitor;
 
 /**
@@ -33,13 +32,12 @@ import ghidra.util.task.TaskMonitor;
 public class ProgramOpenCloseManager {
 	private DataOpenCloseManager dataOpenCloseManager = new DataOpenCloseManager();
 	private List<ChangeListener> listeners = new ArrayList<>();
-	private InMemoryOpenCloseManager variablesOpenCloseManager;
-	private PersistentOpenCloseManager functionsOpenCloseManager;
+	private OpenCloseManager variablesOpenCloseManager;
+	private OpenCloseManager functionsOpenCloseManager;
 
 	public ProgramOpenCloseManager(Program program) {
-		// open/close functions states are persisted and specific to a program
-		functionsOpenCloseManager = new PersistentOpenCloseManager(program,
-			getClass().getSimpleName(), "Function Open State");
+
+		functionsOpenCloseManager = createFunctionsOpenCloseManager(program);
 
 		// open/close variables states are tool based so users can
 		// choose whether to see them by default. They can be opened or closed
@@ -211,4 +209,15 @@ public class ProgramOpenCloseManager {
 		}
 	}
 
+	private OpenCloseManager createFunctionsOpenCloseManager(Program program) {
+		// We prefer to use a persistent open close manager, but that requires ProgramUserData.
+		// Currently not all implementations of Program support ProgramUserData, so if not, just
+		// use the in-memory OpenCloseManager which isn't persistent.
+		ProgramUserData programUserData = program.getProgramUserData();
+		if (programUserData != null) {
+			return new PersistentOpenCloseManager(programUserData, getClass().getSimpleName(),
+				"Function Open State");
+		}
+		return new InMemoryOpenCloseManager();
+	}
 }

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/codebrowser/HeaderActionsTest.java

@@ -95,7 +95,7 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		pressContinueOnResetFormatDialog("Reset Format?");
 
 		functionFormat = formatManager.getFunctionFormat();
-		assertEquals(3, functionFormat.getNumFactorys(0));
+		assertEquals(4, functionFormat.getNumFactorys(0));
 
 	}
 
@@ -125,7 +125,7 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		FormatManager formatManager = header.getFormatManager();
 		FieldFormatModel functionFormat = formatManager.getFunctionFormat();
 		FieldFactory[] factorys = functionFormat.getFactorys(0);
-		assertEquals(3, factorys.length);
+		assertEquals(4, factorys.length);
 
 		selectHeaderField(factorys[0]);
 
@@ -145,9 +145,10 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		FormatManager formatManager = header.getFormatManager();
 		FieldFormatModel functionFormat = formatManager.getFunctionFormat();
 		FieldFactory[] factories = functionFormat.getFactorys(0);
-		assertTrue(factories[0] instanceof SpacerFieldFactory);
-		assertTrue(factories[1] instanceof FunctionSignatureFieldFactory);
-		assertEquals(200, factories[1].getStartX());
+		assertTrue(factories[0] instanceof FunctionOpenCloseFieldFactory);
+		assertTrue(factories[1] instanceof SpacerFieldFactory);
+		assertTrue(factories[2] instanceof FunctionSignatureFieldFactory);
+		assertEquals(220, factories[2].getStartX());
 
 		selectHeaderField(factories[0]);
 		FieldHeaderLocation loc = new FieldHeaderLocation(functionFormat, factories[0], 0, 0);
@@ -159,9 +160,10 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		functionFormat = formatManager.getFunctionFormat();
 		factories = functionFormat.getFactorys(0);
 		assertTrue(factories[0] instanceof SpacerFieldFactory);
-		assertTrue(factories[1] instanceof SpacerFieldFactory);
-		assertTrue(factories[2] instanceof FunctionSignatureFieldFactory);
-		assertEquals(300, factories[2].getStartX());
+		assertTrue(factories[1] instanceof FunctionOpenCloseFieldFactory);
+		assertTrue(factories[2] instanceof SpacerFieldFactory);
+		assertTrue(factories[3] instanceof FunctionSignatureFieldFactory);
+		assertEquals(320, factories[3].getStartX());
 	}
 
 	@Test
@@ -169,10 +171,10 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		FormatManager formatManager = header.getFormatManager();
 		FieldFormatModel functionFormat = formatManager.getFunctionFormat();
 		FieldFactory[] factories = functionFormat.getFactorys(0);
-		assertTrue(factories[0] instanceof SpacerFieldFactory);
-		selectHeaderField(factories[0]);
+		assertTrue(factories[1] instanceof SpacerFieldFactory);
+		selectHeaderField(factories[1]);
 
-		FieldHeaderLocation loc = new FieldHeaderLocation(functionFormat, factories[0], 0, 0);
+		FieldHeaderLocation loc = new FieldHeaderLocation(functionFormat, factories[1], 0, 0);
 		ActionContext context = createContext(provider, loc);
 
 		DockingAction headerAction = getHeaderAction("SetTextAction");
@@ -181,8 +183,8 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 
 		functionFormat = formatManager.getFunctionFormat();
 		factories = functionFormat.getFactorys(0);
-		assertTrue(factories[0] instanceof SpacerFieldFactory);
-		assertEquals("Hello", getText((SpacerFieldFactory) factories[0]));
+		assertTrue(factories[1] instanceof SpacerFieldFactory);
+		assertEquals("Hello", getText((SpacerFieldFactory) factories[1]));
 
 	}
 
@@ -218,18 +220,18 @@ public class HeaderActionsTest extends AbstractGhidraHeadedIntegrationTest {
 		FieldFormatModel functionFormat = formatManager.getFunctionFormat();
 		FieldFactory[] factories = functionFormat.getFactorys(0);
 		selectHeaderField(factories[0]);
-		assertEquals(3, factories.length);
-		assertTrue(factories[1] instanceof FunctionSignatureFieldFactory);
+		assertEquals(4, factories.length);
+		assertTrue(factories[2] instanceof FunctionSignatureFieldFactory);
 
-		FieldHeaderLocation loc = new FieldHeaderLocation(functionFormat, factories[1], 0, 1);
+		FieldHeaderLocation loc = new FieldHeaderLocation(functionFormat, factories[2], 0, 1);
 		ActionContext context = createContext(provider, loc);
 
 		DockingAction headerAction = getHeaderAction("Remove Field");
 		performAction(headerAction, context, true);
 
 		factories = functionFormat.getFactorys(0);
-		assertTrue(!(factories[1] instanceof FunctionSignatureFieldFactory));
-		assertEquals(2, factories.length);
+		assertTrue(!(factories[2] instanceof FunctionSignatureFieldFactory));
+		assertEquals(3, factories.length);
 	}
 
 	@Test

Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/util/PersistentOpenCloseManagerTest.java

@@ -24,6 +24,7 @@ import ghidra.program.database.ProgramBuilder;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.listing.Program;
+import ghidra.program.model.listing.ProgramUserData;
 import ghidra.test.AbstractGhidraHeadedIntegrationTest;
 import ghidra.test.TestEnv;
 import ghidra.util.task.TaskMonitor;
@@ -32,7 +33,7 @@ public class PersistentOpenCloseManagerTest extends AbstractGhidraHeadedIntegrat
 	private DomainFile df;
 	private AddressSpace space;
 	private Program program;
-	private PersistentOpenCloseManager openCloseMgr;
+	private OpenCloseManager openCloseMgr;
 	private TestEnv env;
 
 	@Before
@@ -47,7 +48,8 @@ public class PersistentOpenCloseManagerTest extends AbstractGhidraHeadedIntegrat
 		df = rootFolder.createFile("test", program, TaskMonitor.DUMMY);
 
 		space = program.getAddressFactory().getDefaultAddressSpace();
-		openCloseMgr = new PersistentOpenCloseManager(program, "test", "test");
+		ProgramUserData programUserData = program.getProgramUserData();
+		openCloseMgr = new PersistentOpenCloseManager(programUserData, "test", "test");
 	}
 
 	@After
@@ -120,7 +122,8 @@ public class PersistentOpenCloseManagerTest extends AbstractGhidraHeadedIntegrat
 		program.release(this);
 		program = (Program) df.getDomainObject(this, false, false, TaskMonitor.DUMMY);
 		space = program.getAddressFactory().getDefaultAddressSpace();
-		openCloseMgr = new PersistentOpenCloseManager(program, "test", "test");
+		ProgramUserData programUserData = program.getProgramUserData();
+		openCloseMgr = new PersistentOpenCloseManager(programUserData, "test", "test");
 
 		assertTrue(openCloseMgr.isOpen(addr(0)));
 		assertFalse(openCloseMgr.isOpen(addr(100)));