Merge remote-tracking branch

'origin/GP-5807-dragonmacher-file-choose-slowness--SQUASHED'
(Closes #8284, Closes #4725)

Ghidra/Framework/Docking/data/docking.theme.properties

@@ -102,6 +102,8 @@ icon.dragon.256 = GhidraIcon256.png
 icon.help.navigation.aid.disabled.overlay = icon.not.allowed
 icon.help.navigation.aid.enabled = software-update-available.png
 
+icon.filechooser.default.directory = icon.folder.closed
+icon.filechooser.default.file = [icon]laf.icon.FileView.fileIcon
 icon.filechooser.places.my.computer = computer.png
 icon.filechooser.places.desktop = desktop.png
 icon.filechooser.places.home = user-home.png

Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/DirectoryList.java

@@ -35,6 +35,7 @@ import generic.theme.GThemeDefaults.Colors;
 import ghidra.util.exception.AssertException;
 
 class DirectoryList extends GList<File> implements GhidraFileChooserDirectoryModelIf {
+	private static final int DEFAULT_CELL_WIDTH = 200;
 	private static final int DEFAULT_ICON_SIZE = 16;
 	private static final int MIN_HEIGHT_PADDING = 5;
 
@@ -86,7 +87,20 @@ class DirectoryList extends GList<File> implements GhidraFileChooserDirectoryMod
 		FontMetrics metrics = cellRenderer.getFontMetrics(font);
 		setFixedCellHeight(Math.max(metrics.getHeight(), DEFAULT_ICON_SIZE) +
 			Math.max(metrics.getHeight() / 3, MIN_HEIGHT_PADDING));
-		setFixedCellWidth(-1);
+	}
+
+	@Override
+	public int getFixedCellWidth() {
+		//
+		// This code is called from within the Java List UI to calculate the preferred dimension.
+		// We can prevent the UI from looping over all files by setting a non-negative value for 
+		// our cell width and height.  Here we return a non-negative width value when we have a  
+		// large number of files.  The height is always set to a non-negative value.
+		//
+		if (chooser.hasBigData()) {
+			return DEFAULT_CELL_WIDTH;
+		}
+		return -1;
 	}
 
 	private void build() {

Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/FileListCellRenderer.java

@@ -16,11 +16,15 @@
 package docking.widgets.filechooser;
 
 import java.awt.Component;
+import java.awt.Dimension;
 import java.io.File;
 
 import javax.swing.JList;
 
+import org.bouncycastle.crypto.generators.DHBasicKeyPairGenerator;
+
 import docking.widgets.list.GListCellRenderer;
+import ghidra.util.Msg;
 import ghidra.util.filechooser.GhidraFileChooserModel;
 
 class FileListCellRenderer extends GListCellRenderer<File> {
@@ -46,6 +50,19 @@ class FileListCellRenderer extends GListCellRenderer<File> {
 		super.getListCellRendererComponent(list, file, index, isSelected, cellHasFocus);
 		setIcon(model.getIcon(file));
 
+		// The file chooser's list will sometimes set a fixed width.  When that happens, the text
+		// may get clipped.  When we get clipped text, add a tooltip to show the full text.
+		int fixedWidth = list.getFixedCellWidth();
+		if (fixedWidth > 0) {
+			Dimension d = getPreferredSize();
+			if (d.getWidth() > fixedWidth) {
+				setToolTipText(getText());
+			}	
+			else {
+				setToolTipText(null);
+			}
+		}
+
 		return this;
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java

@@ -76,6 +76,11 @@ import util.HistoryList;
  */
 public class GhidraFileChooser extends ReusableDialogComponentProvider implements FileFilter {
 
+	/**
+	 * Somewhat arbitrary file count threshold to signal when slow operations should be avoided.
+	 */
+	private static final int BIG_DATA_THRESHOLD = 200;
+
 	static final String UP_BUTTON_NAME = "UP_BUTTON";
 	private static final Color FOREROUND_COLOR = new GColor("color.fg.filechooser");
 	private static final Color BACKGROUND_COLOR = new GColor("color.bg.filechooser");
@@ -218,7 +223,7 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 	 * @param parent the parent component
 	 */
 	public GhidraFileChooser(Component parent) {
-		this(new LocalFileChooserModel(), parent);
+		this(null, parent);
 	}
 
 	/**
@@ -238,8 +243,10 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 	}
 
 	private void init(GhidraFileChooserModel newModel) {
+		if (newModel == null) {
+			newModel = new LocalFileChooserModel(() -> this);
+		}
 		this.fileChooserModel = newModel;
-		this.fileChooserModel.setModelUpdateCallback(modelUpdater::update);
 
 		history.setAllowDuplicates(true);
 
@@ -809,6 +816,17 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 		}
 	}
 
+	/**
+	 * Returns whether the current file data is a large number of files.   Exactly what constitutes 
+	 * large is up to the client.  This method allows the framework to avoid expensive operations
+	 * when the data set is large.
+	 * 
+	 * @return true if big data
+	 */
+	public boolean hasBigData() {
+		return directoryListModel.getSize() > BIG_DATA_THRESHOLD;
+	}
+
 	/**
 	 * Sets the text used in the <code>OK</code> button
 	 *
@@ -935,22 +953,12 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 			throw new AssertException("Expected a directory and did not get one: " + directory);
 		}
 
-		File currentDirectory = currentDirectory();
-
 		// if we are forcing the update, then just do it! ...or, if the new dir is not already
 		// the current dir, then we need to update
+		File currentDirectory = currentDirectory();
 		if (force || !directory.equals(currentDirectory)) {
 			worker.schedule(new UpdateDirectoryContentsJob(directory, null, addToHistory));
-			return;
 		}
-
-		// we only get here if the new dir is the current dir and we are not forcing an update
-		// TODO this code causes unexpected behavior when in 'directories only' mode in that
-		// this will cause the current directory to change.  The behavior can be seen by
-		// putting this code back in and then running the tests.   No tests are failing with this
-		// code removed.  We are leaving this code here for a couple releases in case we find
-		// a code path that requires it.
-		// setSelectedFileAndUpdateDisplay((isFilesOnly() ? null : directory));
 	}
 
 	boolean pendingUpdate() {
@@ -986,13 +994,15 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 	private void setDirectoryList(File directory, List<File> files) {
 		// if the visible listing is still the same directory as this incoming list of files
 		if (currentDirectory().equals(directory)) {
+
 			// recompute list cell dims before causing an update to the model
 			directoryTableModel.setFiles(files);
 			directoryTable.scrollRectToVisible(new Rectangle(0, 0, 0, 0));
 			directoryListModel.setFiles(files);
 			directoryList.scrollRectToVisible(new Rectangle(0, 0, 0, 0));
 		}
-		updateShortCutPanel();
+
+		updateShortcutPanel();
 	}
 
 	/**
@@ -1351,6 +1361,10 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 		});
 	}
 
+	void rootInfoUpdated() {
+		modelUpdater.update();
+	}
+
 	GhidraFileChooserModel getModel() {
 		return fileChooserModel;
 	}
@@ -1507,7 +1521,7 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 		updateDirAndSelectFile(currentDir, currentSelectedFile, true, false);
 	}
 
-	private void updateShortCutPanel() {
+	private void updateShortcutPanel() {
 		// make sure that if one of the shortcut buttons is selected, the directory matches that button
 		File currentDirectory = currentDirectory();
 		checkShortCutButton(myComputerButton, currentDirectory);

Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/LocalFileChooserModel.java

@@ -20,6 +20,7 @@ import java.io.FileFilter;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
 
 import javax.swing.Icon;
 import javax.swing.filechooser.FileSystemView;
@@ -31,10 +32,11 @@ import utility.function.Callback;
 
 /**
  * A default implementation of the file chooser model that browses the local file system.
- * 
  */
 public class LocalFileChooserModel implements GhidraFileChooserModel {
 	private static final Icon PROBLEM_FILE_ICON = Icons.WARNING_ICON;
+	private static final Icon ICON_DIR_DEFAULT = new GIcon("icon.filechooser.default.directory");
+	private static final Icon ICON_FILE_DEFAULT = new GIcon("icon.filechooser.default.file");
 	private static final Icon PENDING_ROOT_ICON = new GIcon("icon.drive");
 
 	private static final FileSystemRootInfo FS_ROOT_INFO = new FileSystemRootInfo();
@@ -48,19 +50,18 @@ public class LocalFileChooserModel implements GhidraFileChooserModel {
 	 * next time the user hits refresh or navigates into a directory. 
 	 */
 	private Map<File, Icon> fileIconMap = new HashMap<>();
-
 	private Callback callback;
+	private Supplier<GhidraFileChooser> chooserSupplier;
+
+	LocalFileChooserModel(Supplier<GhidraFileChooser> chooserSupplier) {
+		this.chooserSupplier = chooserSupplier;
+	}
 
 	@Override
 	public char getSeparator() {
 		return File.separatorChar;
 	}
 
-	@Override
-	public void setModelUpdateCallback(Callback callback) {
-		this.callback = callback;
-	}
-
 	@Override
 	public File getHomeDirectory() {
 		return new File(System.getProperty("user.home"));
@@ -119,10 +120,25 @@ public class LocalFileChooserModel implements GhidraFileChooserModel {
 		if (FS_ROOT_INFO.isRoot(file)) {
 			return FS_ROOT_INFO.getRootIcon(file);
 		}
-		Icon result = (file != null && file.exists())
+
-				? fileIconMap.computeIfAbsent(file, this::getSystemIcon)
+		if (file == null || !file.exists()) {
-				: null;
+			return PROBLEM_FILE_ICON;
-		return (result != null) ? result : PROBLEM_FILE_ICON;
+		}
+
+		//
+		// This code is called from within the Java List UI to calculate the preferred dimension.
+		// This becomes an issue with a large number of files, since the UI gets the icon for every
+		// file in the directory.  Directories can have 10s of thousands of files.   In such a 
+		// scenario,  pulling icons from the file system view can cause the UI to lockup 
+		// indeterminately.  Instead of delegating to the system in that case, we can just return 
+		// default icons for files and folders, which seems like a reasonable workaround.
+		//
+		GhidraFileChooser gfc = chooserSupplier.get();
+		if (gfc.hasBigData()) {
+			return file.isDirectory() ? ICON_DIR_DEFAULT : ICON_FILE_DEFAULT;
+		}
+
+		return fileIconMap.computeIfAbsent(file, this::getSystemIcon);
 	}
 
 	private Icon getSystemIcon(File file) {
@@ -132,7 +148,7 @@ public class LocalFileChooserModel implements GhidraFileChooserModel {
 		catch (Exception e) {
 			// ignore, return null
 		}
-		return null;
+		return PROBLEM_FILE_ICON;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/test.slow/java/docking/widgets/filechooser/GhidraFileChooserTest.java

@@ -19,6 +19,7 @@
 package docking.widgets.filechooser;
 
 import static docking.widgets.filechooser.GhidraFileChooserMode.*;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
@@ -34,6 +35,7 @@ import java.util.Queue;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 
 import javax.swing.*;
 import javax.swing.table.JTableHeader;
@@ -1385,7 +1387,8 @@ public class GhidraFileChooserTest extends AbstractDockingTest {
 		waitForSwing();
 
 		// use a stubbed chooser model that has no Desktop
-		GhidraFileChooserModel gfcm = new LocalFileChooserModel() {
+		Supplier<GhidraFileChooser> gfc = () -> chooser;
+		GhidraFileChooserModel gfcm = new LocalFileChooserModel(gfc) {
 			@Override
 			public File getDesktopDirectory() {
 				return null;
@@ -1419,7 +1422,8 @@ public class GhidraFileChooserTest extends AbstractDockingTest {
 		// use a stubbed chooser model that has a non-native Desktop value
 		final File fakeUserDesktopDir = createTempDirectory("faked_desktop_dir");
 
-		GhidraFileChooserModel gfcm = new LocalFileChooserModel() {
+		Supplier<GhidraFileChooser> gfc = () -> chooser;
+		GhidraFileChooserModel gfcm = new LocalFileChooserModel(gfc) {
 			@Override
 			public File getDesktopDirectory() {
 				return fakeUserDesktopDir;
@@ -2117,11 +2121,6 @@ public class GhidraFileChooserTest extends AbstractDockingTest {
 	}
 
 	private File selectFile(DirectoryList list, int index) {
-
-		// TODO debug - remove when all tests passing on server
-		int size = list.getModel().getSize();
-		Msg.debug(this, "selectFile() - new index: " + index + "; list size: " + size);
-
 		runSwing(() -> list.setSelectedIndex(index));
 		return runSwing(() -> list.getSelectedFile());
 	}

Ghidra/Framework/Gui/src/main/java/ghidra/util/filechooser/GhidraFileChooserModel.java

@@ -21,8 +21,6 @@ import java.util.List;
 
 import javax.swing.Icon;
 
-import utility.function.Callback;
-
 /**
  * Interface for the GhidraFileChooser data model.
  * This allows the GhidraFileChooser to operate
@@ -30,12 +28,6 @@ import utility.function.Callback;
  * just the local file system.
  */
 public interface GhidraFileChooserModel {
-	/**
-	 * Set the model update callback.
-	 * 
-	 * @param callback the new model update callback handler
-	 */
-	public void setModelUpdateCallback(Callback callback);
 
 	/**
 	 * Returns the home directory.