Fix NPE with UnitHelp. (#11977)

This fixes the NPE with Unit Help while preserving the functionality where the progress dialog stays shown until Unit Help is opened. (This is particularly noticeable on the Warcraft map where otherwise there's a 9s delay on my machine after the progress dialog is closed and before unit help is shown.)

The NPE was caused by my previous attempt at this doing Swing work on a background thread (which apparently worked fine on Mac, but not Windows). Instead, this PR reworks the logic with a more complicated solution that allows plumbing the expensive Swing work to be done to the background runner, so that it can run it before it closes the progress dialog.

game-app/game-core/src/main/java/games/strategy/engine/framework/ui/background/BackgroundTaskRunner.java

@@ -4,13 +4,16 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.base.Throwables;
+import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import javax.swing.JFrame;
 import javax.swing.SwingUtilities;
 import javax.swing.SwingWorker;
+import lombok.Getter;
 import lombok.experimental.UtilityClass;
 import org.triplea.java.Interruptibles;
 import org.triplea.java.function.ThrowingSupplier;
@@ -19,7 +22,7 @@
 /** Provides methods for running tasks in the background to avoid blocking the UI. */
 @UtilityClass
 public final class BackgroundTaskRunner {
-  private static JFrame mainFrame;
+  @Getter private static JFrame mainFrame;
 
   public static void setMainFrame(final JFrame mainFrame) {
     checkState(BackgroundTaskRunner.mainFrame == null);
@@ -72,7 +75,14 @@ public static void runInBackground(final String message, final Runnable backgrou
    */
   public static <T> T runInBackgroundAndReturn(
       final String message, final Supplier<T> backgroundAction) throws InterruptedException {
-    return runInBackgroundAndReturn(message, backgroundAction::get, RuntimeException.class);
+    return runInBackgroundAndReturn(message, backgroundAction::get, null, RuntimeException.class);
+  }
+
+  public static <T> T runInBackgroundAndReturn(
+      Consumer<T> runOnEdtBeforeDialogClose, String message, Supplier<T> backgroundAction)
+      throws InterruptedException {
+    return runInBackgroundAndReturn(
+        message, backgroundAction::get, runOnEdtBeforeDialogClose, RuntimeException.class);
   }
 
   /**
@@ -88,6 +98,9 @@ public static <T> T runInBackgroundAndReturn(
    * @param <E> The type of exception thrown by the background action.
    * @param message The message displayed to the user while the background action is running.
    * @param backgroundAction The action to run in the background.
+   * @param runOnEdtBeforeDialogClose Work to be done on EDT with the result, before closing the
+   *     dialog. Some Swing operations can be slow (e.g. layout of lots of HTML like unit help), so
+   *     this can be used to ensure we only close the progress dialog once this work has been done.
    * @param exceptionType The type of exception thrown by the background action.
    * @return The value returned by {@code backgroundAction}.
    * @throws IllegalStateException If this method is not called from the EDT.
@@ -98,6 +111,7 @@ public static <T> T runInBackgroundAndReturn(
   public static <T, E extends Exception> T runInBackgroundAndReturn(
       final String message,
       final ThrowingSupplier<T, E> backgroundAction,
+      final Consumer<T> runOnEdtBeforeDialogClose,
       final Class<E> exceptionType)
       throws E, InterruptedException {
     checkState(SwingUtilities.isEventDispatchThread());
@@ -117,16 +131,18 @@ protected T doInBackground() throws Exception {
 
           @Override
           protected void done() {
-            waitDialog.setVisible(false);
-            waitDialog.dispose();
-
             try {
-              resultRef.set(get());
+              T t = get();
+              resultRef.set(t);
+              Optional.ofNullable(runOnEdtBeforeDialogClose).ifPresent(c -> c.accept(t));
             } catch (final ExecutionException e) {
               exceptionRef.set(e.getCause());
             } catch (final InterruptedException e) {
               Thread.currentThread().interrupt();
               exceptionRef.set(e);
+            } finally {
+              waitDialog.setVisible(false);
+              waitDialog.dispose();
             }
           }
         };

game-app/game-core/src/main/java/games/strategy/triplea/ui/menubar/help/UnitHelpMenu.java

@@ -5,12 +5,9 @@
 import games.strategy.triplea.ui.UiContext;
 import javax.swing.Action;
 import javax.swing.BorderFactory;
-import javax.swing.JDialog;
 import javax.swing.JEditorPane;
 import javax.swing.JScrollPane;
 import lombok.experimental.UtilityClass;
-import org.triplea.java.Interruptibles;
-import org.triplea.java.Interruptibles.Result;
 import org.triplea.swing.SwingAction;
 
 @UtilityClass
@@ -20,21 +17,26 @@ class UnitHelpMenu {
   Action buildMenu(final GameData gameData, final UiContext uiContext) {
     return SwingAction.of(
         unitHelpTitle,
-        e -> {
-          final Result<JDialog> result =
-              Interruptibles.awaitResult(
-                  () ->
-                      BackgroundTaskRunner.runInBackgroundAndReturn(
-                          "Calculating Data",
-                          () -> {
-                            String text = UnitStatsTable.getUnitStatsTable(gameData, uiContext);
-                            JEditorPane editorPane = new JEditorPane("text/html", text);
-                            editorPane.setEditable(false);
-                            JScrollPane scroll = new JScrollPane(editorPane);
-                            scroll.setBorder(BorderFactory.createEmptyBorder());
-                            return InformationDialog.createDialog(scroll, unitHelpTitle);
-                          }));
-          result.result.orElseThrow().setVisible(true);
+        event -> {
+          try {
+            BackgroundTaskRunner.runInBackgroundAndReturn(
+                UnitHelpMenu::showDialog,
+                "Calculating Data",
+                () -> UnitStatsTable.getUnitStatsTable(gameData, uiContext));
+          } catch (InterruptedException e) {
+            // Nothing to do.
+          }
         });
   }
+
+  private static void showDialog(String text) {
+    final JEditorPane editorPane = new JEditorPane();
+    editorPane.setContentType("text/html");
+    editorPane.setEditable(false);
+    final JScrollPane scroll = new JScrollPane(editorPane);
+    scroll.setBorder(BorderFactory.createEmptyBorder());
+    editorPane.setText(text);
+    editorPane.setCaretPosition(0);
+    InformationDialog.createDialog(scroll, unitHelpTitle).setVisible(true);
+  }
 }

game-app/game-headed/src/main/java/games/strategy/engine/lobby/client/login/LobbyLogin.java

@@ -235,6 +235,7 @@ private void forgotPassword(final ForgotPasswordPanel panel) {
                                   .username(panel.getUserName())
                                   .email(panel.getEmail())
                                   .build()),
+                  null,
                   IOException.class)
               .getResponseMessage();
       DialogBuilder.builder()