GP-5126 Corrected Program Change Bar update issues

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/misc/MyProgramChangesDisplayPlugin.java

@@ -102,9 +102,16 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 	private int serverVersion = -1;
 	private int localVersion = -1;
 
-	private boolean programChangedLocally;
+	// currentProgram object changed; affects currentMyChangeMarks
-	private boolean programChangedRemotely;
+	private boolean currentProgramChanged;
-	private boolean programSaved;
+
+	// domain file updated on server; affects currentOtherChangeMarks
+	private boolean domainFileChangedRemotely;
+
+	// domain file updated locally; affects currentChangesSinceCheckoutMarks
+	private boolean domainFileChangedLocally;
+
+	// flag to force update of currentConflictChangeMarks
 	private boolean updateConflicts;
 
 	public MyProgramChangesDisplayPlugin(PluginTool tool) {
@@ -181,9 +188,9 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 
 		serverVersion = -1;
 		localVersion = -1;
-		programChangedLocally = false;
+		currentProgramChanged = false;
-		programChangedRemotely = false;
+		domainFileChangedRemotely = false;
-		programSaved = false;
+		domainFileChangedLocally = false;
 		program.removeTransactionListener(transactionListener);
 		program.removeListener(this);
 		disposeMarkerSets(program);
@@ -191,9 +198,9 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 
 	private void intializeChangeMarkers() {
 		// set all the triggers for updating markers when initializing
-		programChangedLocally = true;
+		currentProgramChanged = true;
-		programChangedRemotely = true;
+		domainFileChangedRemotely = true;
-		programSaved = true;
+		domainFileChangedLocally = true;
 		updateConflicts = true;
 		updateChangeMarkers();
 	}
@@ -273,24 +280,25 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 
 		ProgramChangeSet changeSet = currentProgram.getChanges();
 
-		if (programChangedLocally) {
+		if (currentProgramChanged) {
 			currentMyChangeMarks
 					.setAddressSetCollection(changeSet.getAddressSetCollectionSinceLastSave());
 		}
 
 		if (isTrackingServerChanges()) {
-			if (programSaved || programChangedRemotely) {
+			if (domainFileChangedLocally) {
 				currentChangesSinceCheckoutMarks
 						.setAddressSetCollection(changeSet.getAddressSetCollectionSinceCheckout());
 			}
 
-			if (programChangedRemotely) {
+			if (domainFileChangedRemotely) {
 				currentOtherChangeMarks
 						.setAddressSetCollection(new SingleAddressSetCollection(otherChangeSet));
 			}
 
-			// only update conflict markers when server changeSet changes or we end a transaction
+			// Update conflict markers when forced by server version change,
-			if (programChangedRemotely || updateConflicts) {
+			// local version change (merge may have occured) or a transaction has ended
+			if (updateConflicts) {
 				AddressSet intersect = changeSet.getAddressSetCollectionSinceCheckout()
 						.getCombinedAddressSet()
 						.intersect(otherChangeSet);
@@ -299,9 +307,9 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 			}
 		}
 
-		programChangedLocally = false;
+		currentProgramChanged = false;
-		programChangedRemotely = false;
+		domainFileChangedRemotely = false;
-		programSaved = false;
+		domainFileChangedLocally = false;
 		updateConflicts = false;
 	}
 
@@ -310,30 +318,43 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 	 * checkout and launch update thread if necessary.
 	 */
 	private void updateForDomainFileChanged() {
+
 		DomainFile df = currentProgram.getDomainFile();
+		if (!df.isCheckedOut()) {
+			// Only currentMyChangeMarks are maintained using domain object change listener
+			// when file is not checked-out
+			return;
+		}
 
 		int latestServerVersion = df.getLatestVersion();
 		int latestLocalVersion = df.getVersion();
-		// if the server version changes, schedule thread to get server changeSet
+
-		// which will trigger an marker update for both the other and conflict marker sets.
+		boolean localVersionChanged = localVersion != latestLocalVersion;
-		if (df.isCheckedOut() && serverVersion != latestServerVersion) {
+		boolean serverVersionChanged = serverVersion != latestServerVersion;
+		if (!localVersionChanged && !serverVersionChanged) {
+			return; // No update to change bars
+		}
+
+		localVersion = latestLocalVersion;
 		serverVersion = latestServerVersion;
-			localVersion = latestLocalVersion;
+
-			if (serverVersion == localVersion) {
+		domainFileChangedLocally |= localVersionChanged;
-				otherChangeSet = new AddressSet();
+		domainFileChangedRemotely |= serverVersionChanged;
-				programChangedRemotely = true;
-				updateManager.update();
-			}
-			else {
-				scheduleUpdatesFromServer(currentProgram);
-			}
-		}
-		// else just the local version changed, update conflict sets.
-		else if (latestLocalVersion != localVersion) {
-			localVersion = latestLocalVersion;
 		updateConflicts = true;
-			updateManager.update();
+
+		if (localVersion == serverVersion) {
+			// When server and local versions match otherChangeSet is empty
+			otherChangeSet = new AddressSet();
+			domainFileChangedRemotely = true;
 		}
+		else if (serverVersionChanged) {
+			// Use UpdateChangeSetJob to compute the otherChangeSet
+			// GUI update deferred to UpdateChangeSetJob
+			scheduleUpdatesFromServer(currentProgram);
+			return;
+		}
+
+		updateManager.update();
 	}
 
 	private void scheduleUpdatesFromServer(Program p) {
@@ -350,9 +371,9 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 
 	@Override
 	public void domainObjectChanged(DomainObjectChangedEvent ev) {
-		programChangedLocally = true;
+		currentProgramChanged = true;
 		if (ev.contains(DomainObjectEvent.SAVED)) {
-			programSaved = true;
+			domainFileChangedLocally = true;
 		}
 
 		updateManager.update();
@@ -428,6 +449,10 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 		@Override
 		public void run(TaskMonitor monitor) throws CancelledException {
 
+			if (localVersion == serverVersion) {
+				return; // skip update if versions now match
+			}
+
 			monitor.checkCancelled(); // plugin was shut down while we were scheduled
 
 			ProgramChangeSet changes = null;
@@ -456,7 +481,8 @@ public class MyProgramChangesDisplayPlugin extends ProgramPlugin implements Doma
 			}
 
 			otherChangeSet = remoteChanges;
-			programChangedRemotely = true;
+			domainFileChangedRemotely = true;
+			updateConflicts = true;
 			updateManager.update();
 		}
 	}

Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFileData.java

@@ -89,6 +89,7 @@ public class GhidraFileData {
 	private Icon disabledIcon;
 
 	private AtomicBoolean busy = new AtomicBoolean();
+	private boolean mergeInProgress = false;
 
 // TODO: Many of the old methods assumed that the state was up-to-date due to
 // refreshing ... we are relying on non-refreshed data to be dropped from cache map and no
@@ -196,6 +197,9 @@ public class GhidraFileData {
 	}
 
 	private void statusChanged(boolean fileIDset) throws IOException {
+		if (mergeInProgress) {
+			return;
+		}
 		icon = null;
 		disabledIcon = null;
 		fileIDset |= refresh();
@@ -1099,6 +1103,7 @@ public class GhidraFileData {
 		}
 
 		DomainObjectAdapterDB inUseDomainObj = null;
+		mergeInProgress = true;
 		projectData.mergeStarted();
 		try {
 			inUseDomainObj = getAndLockInUseDomainObjectForMergeUpdate("checkin");
@@ -1195,6 +1200,7 @@ public class GhidraFileData {
 		finally {
 			unlockDomainObject(inUseDomainObj);
 			busy.set(false);
+			mergeInProgress = false;
 			projectData.mergeEnded();
 			parent.deleteLocalFolderIfEmpty();
 			parent.fileChanged(name);
@@ -1430,6 +1436,7 @@ public class GhidraFileData {
 		}
 
 		DomainObjectAdapterDB inUseDomainObj = null;
+		mergeInProgress = true;
 		projectData.mergeStarted();
 		try {
 			ContentHandler<?> contentHandler = getContentHandler();
@@ -1556,6 +1563,7 @@ public class GhidraFileData {
 		finally {
 			unlockDomainObject(inUseDomainObj);
 			busy.set(false);
+			mergeInProgress = false;
 			projectData.mergeEnded();
 			parent.deleteLocalFolderIfEmpty();
 			parent.fileChanged(name);
@@ -1914,6 +1922,7 @@ public class GhidraFileData {
 
 		FolderItem tmpItem = null;
 		DomainObjectAdapterDB inUseDomainObj = null;
+		mergeInProgress = true;
 		projectData.mergeStarted();
 		try {
 			inUseDomainObj = getAndLockInUseDomainObjectForMergeUpdate("merge");
@@ -2011,6 +2020,7 @@ public class GhidraFileData {
 		finally {
 			unlockDomainObject(inUseDomainObj);
 			busy.set(false);
+			mergeInProgress = false;
 			try {
 				if (tmpItem != null) {
 					try {