From 5bd6c7454738d0a8a395cfe27a364cdb325eb37c Mon Sep 17 00:00:00 2001
From: Alexey Semenyuk <asemenyuk@openjdk.org>
Date: Thu, 8 Apr 2021 15:44:11 +0000
Subject: [PATCH] 8236127: Use value of --icon CLI option to set icon for exe
 installers

Reviewed-by: almatvee, herrick
---
 .../jdk/jpackage/internal/ValidOptions.java   |   4 +-
 .../resources/HelpResources.properties        |   6 +-
 .../resources/HelpResources_ja.properties     |   6 +-
 .../resources/HelpResources_zh_CN.properties  |   6 +-
 .../internal/ExecutableRebrander.java         |  40 ++--
 .../jdk/jpackage/test/LinuxHelper.java        |   4 +-
 .../jpackage/share/AppImagePackageTest.java   |  18 +-
 .../windows/WinInstallerIconTest.java         | 175 ++++++++++++++++++
 8 files changed, 225 insertions(+), 34 deletions(-)
 create mode 100644 test/jdk/tools/jpackage/windows/WinInstallerIconTest.java

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java
index fe7022f1c86..deec3bb7d10 100644
--- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java
+++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -70,6 +70,7 @@ class ValidOptions {
         options.put(CLIOptions.VENDOR.getId(), USE.ALL);
         options.put(CLIOptions.COPYRIGHT.getId(), USE.ALL);
         options.put(CLIOptions.PACKAGE_TYPE.getId(), USE.ALL);
+        options.put(CLIOptions.ICON.getId(), USE.ALL);
 
         options.put(CLIOptions.INPUT.getId(), USE.LAUNCHER);
         options.put(CLIOptions.MODULE.getId(), USE.LAUNCHER);
@@ -77,7 +78,6 @@ class ValidOptions {
         options.put(CLIOptions.ADD_MODULES.getId(), USE.LAUNCHER);
         options.put(CLIOptions.MAIN_JAR.getId(), USE.LAUNCHER);
         options.put(CLIOptions.APPCLASS.getId(), USE.LAUNCHER);
-        options.put(CLIOptions.ICON.getId(), USE.LAUNCHER);
         options.put(CLIOptions.ARGUMENTS.getId(), USE.LAUNCHER);
         options.put(CLIOptions.JAVA_OPTIONS.getId(), USE.LAUNCHER);
         options.put(CLIOptions.ADD_LAUNCHER.getId(), USE.LAUNCHER);
diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
index db22cbcebb8..c985096c3f9 100644
--- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
+++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
@@ -70,6 +70,9 @@ Generic Options:\n\
 \  --help -h \n\
 \          Print the usage text with a list and description of each valid\n\
 \          option for the current platform to the output stream, and exit\n\
+\  --icon <icon file path>\n\
+\          Path of the icon of the application package\n\
+\          (absolute path or relative to the current directory)\n\
 \  --name -n <name>\n\
 \          Name of the application and/or package\n\
 \  --dest -d <destination path>\n\
@@ -120,9 +123,6 @@ Generic Options:\n\
 \          --strip-native-commands.\n\
 \n\
 \Options for creating the application image:\n\
-\  --icon <icon file path>\n\
-\          Path of the icon of the application package\n\
-\          (absolute path or relative to the current directory)\n\
 \  --input -i <input path>\n\
 \          Path of the input directory that contains the files to be packaged\n\
 \          (absolute path or relative to the current directory)\n\
diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_ja.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_ja.properties
index 24bb78a3487..a488c84d7e7 100644
--- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_ja.properties
+++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_ja.properties
@@ -70,6 +70,9 @@ Generic Options:\n\
 \  --help -h \n\
 \          Print the usage text with a list and description of each valid\n\
 \          option for the current platform to the output stream, and exit\n\
+\  --icon <icon file path>\n\
+\          Path of the icon of the application package\n\
+\          (absolute path or relative to the current directory)\n\
 \  --name -n <name>\n\
 \          Name of the application and/or package\n\
 \  --dest -d <destination path>\n\
@@ -120,9 +123,6 @@ Generic Options:\n\
 \          --strip-native-commands.\n\
 \n\
 \Options for creating the application image:\n\
-\  --icon <icon file path>\n\
-\          Path of the icon of the application package\n\
-\          (absolute path or relative to the current directory)\n\
 \  --input -i <input path>\n\
 \          Path of the input directory that contains the files to be packaged\n\
 \          (absolute path or relative to the current directory)\n\
diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_zh_CN.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_zh_CN.properties
index 437b38df99b..8b970667bbc 100644
--- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_zh_CN.properties
+++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_zh_CN.properties
@@ -70,6 +70,9 @@ Generic Options:\n\
 \  --help -h \n\
 \          Print the usage text with a list and description of each valid\n\
 \          option for the current platform to the output stream, and exit\n\
+\  --icon <icon file path>\n\
+\          Path of the icon of the application package\n\
+\          (absolute path or relative to the current directory)\n\
 \  --name -n <name>\n\
 \          Name of the application and/or package\n\
 \  --dest -d <destination path>\n\
@@ -120,9 +123,6 @@ Generic Options:\n\
 \          --strip-native-commands.\n\
 \n\
 \Options for creating the application image:\n\
-\  --icon <icon file path>\n\
-\          Path of the icon of the application package\n\
-\          (absolute path or relative to the current directory)\n\
 \  --input -i <input path>\n\
 \          Path of the input directory that contains the files to be packaged\n\
 \          (absolute path or relative to the current directory)\n\
diff --git a/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/ExecutableRebrander.java b/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/ExecutableRebrander.java
index 4b11d0f5349..2a2bf2d86b9 100644
--- a/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/ExecutableRebrander.java
+++ b/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/ExecutableRebrander.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -38,6 +38,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.ResourceBundle;
+import java.util.function.Supplier;
 import static jdk.jpackage.internal.OverridableResource.createResource;
 import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
 import static jdk.jpackage.internal.StandardBundlerParam.COPYRIGHT;
@@ -45,6 +46,7 @@ import static jdk.jpackage.internal.StandardBundlerParam.DESCRIPTION;
 import static jdk.jpackage.internal.StandardBundlerParam.TEMP_ROOT;
 import static jdk.jpackage.internal.StandardBundlerParam.VENDOR;
 import static jdk.jpackage.internal.StandardBundlerParam.VERSION;
+import static jdk.jpackage.internal.WindowsAppImageBuilder.ICON_ICO;
 
 
 final class ExecutableRebrander {
@@ -63,34 +65,38 @@ final class ExecutableRebrander {
 
     void rebrandInstaller(Map<String, ? super Object> params, Path target)
             throws IOException {
-        if (!target.isAbsolute()) {
-            rebrandInstaller(params, target.toAbsolutePath());
-            return;
-        }
-        rebrandExecutable(params, target, (resourceLock) -> {
-            rebrandProperties(resourceLock, createResource(
-                    INSTALLER_PROPERTIES_TEMPLATE, params).setPublicName(
-                            INSTALLER_PROPERTIES_RESOURE_DIR_ID),
-                    createSubstituteData(params), target);
-        });
+        Path icon = ICON_ICO.fetchFrom(params);
+        rebrandExecutable(params, icon, () -> {
+            return createResource(INSTALLER_PROPERTIES_TEMPLATE, params).setPublicName(
+                    INSTALLER_PROPERTIES_RESOURE_DIR_ID);
+        }, target);
     }
 
     void rebrandLauncher(Map<String, ? super Object> params, Path icon,
             Path target) throws IOException {
+        rebrandExecutable(params, icon, () -> {
+            return createResource(
+                    LAUNCHER_PROPERTIES_TEMPLATE, params).setPublicName(
+                            APP_NAME.fetchFrom(params) + ".properties");
+        }, target);
+    }
+
+    private void rebrandExecutable(Map<String, ? super Object> params, Path icon,
+            Supplier<OverridableResource> resourceSupplier, Path target) throws
+            IOException {
         if (!target.isAbsolute() || (icon != null && !icon.isAbsolute())) {
             Path absIcon = null;
             if (icon != null) {
                 absIcon = icon.toAbsolutePath();
             }
-            rebrandLauncher(params, absIcon, target.toAbsolutePath());
+            rebrandExecutable(params, absIcon, resourceSupplier,
+                    target.toAbsolutePath());
             return;
         }
         rebrandExecutable(params, target, (resourceLock) -> {
-            rebrandProperties(resourceLock, createResource(
-                    LAUNCHER_PROPERTIES_TEMPLATE, params).setPublicName(
-                            APP_NAME.fetchFrom(params) + ".properties"),
-                    createSubstituteData(params), target);
-
+            rebrandProperties(resourceLock, resourceSupplier.get(),
+                    createSubstituteData(
+                            params), target);
             if (icon != null) {
                 iconSwap(resourceLock, icon.toString());
             }
diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java
index 8147c035d47..aede136e1d0 100644
--- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java
+++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java
@@ -282,8 +282,8 @@ public class LinuxHelper {
                     actualCriticalRuntimePaths);
         } else {
             // AppImagePackageTest.testEmpty() will have no dependencies,
-            // but will have more then 0 and less than 1K content size.
-            checkPrerequisites = packageSize > 1;
+            // but will have more then 0 and less than 5K content size when --icon is used.
+            checkPrerequisites = packageSize > 5;
         }
 
         List<String> prerequisites = LinuxHelper.getPrerequisitePackages(cmd);
diff --git a/test/jdk/tools/jpackage/share/AppImagePackageTest.java b/test/jdk/tools/jpackage/share/AppImagePackageTest.java
index 9065494e0e9..ae9f6a21ffc 100644
--- a/test/jdk/tools/jpackage/share/AppImagePackageTest.java
+++ b/test/jdk/tools/jpackage/share/AppImagePackageTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,10 +25,10 @@ import java.nio.file.Path;
 import java.nio.file.Files;
 import java.io.IOException;
 import java.util.List;
+import jdk.jpackage.test.Annotations.Parameter;
 import jdk.jpackage.test.TKit;
 import jdk.jpackage.test.JPackageCommand;
 import jdk.jpackage.test.PackageTest;
-import jdk.jpackage.test.PackageType;
 import jdk.jpackage.test.RunnablePackageTest.Action;
 import jdk.jpackage.test.Annotations.Test;
 
@@ -68,7 +68,9 @@ public class AppImagePackageTest {
     }
 
     @Test
-    public static void testEmpty() throws IOException {
+    @Parameter("true")
+    @Parameter("false")
+    public static void testEmpty(boolean withIcon) throws IOException {
         final String name = "EmptyAppImagePackageTest";
         final String imageName = name + (TKit.isOSX() ? ".app" : "");
         Path appImageDir = TKit.createTempDirectory(null).resolve(imageName);
@@ -81,6 +83,9 @@ public class AppImagePackageTest {
         new PackageTest()
         .addInitializer(cmd -> {
             cmd.addArguments("--app-image", appImageDir);
+            if (withIcon) {
+                cmd.addArguments("--icon", iconPath("icon"));
+            }
             cmd.removeArgumentWithValue("--input");
 
             // on mac, with --app-image and without --mac-package-identifier,
@@ -88,7 +93,12 @@ public class AppImagePackageTest {
             if (TKit.isOSX()) {
                 cmd.addArguments("--mac-package-identifier", name);
             }
-        }).run(new Action[] { Action.CREATE, Action.UNPACK });
+        }).run(Action.CREATE, Action.UNPACK);
         // default: {CREATE, UNPACK, VERIFY}, but we can't verify foreign image
     }
+
+    private static Path iconPath(String name) {
+        return TKit.TEST_SRC_ROOT.resolve(Path.of("resources", name
+                + TKit.ICON_SUFFIX));
+    }
 }
diff --git a/test/jdk/tools/jpackage/windows/WinInstallerIconTest.java b/test/jdk/tools/jpackage/windows/WinInstallerIconTest.java
new file mode 100644
index 00000000000..3cbadbc25c3
--- /dev/null
+++ b/test/jdk/tools/jpackage/windows/WinInstallerIconTest.java
@@ -0,0 +1,175 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.awt.Graphics;
+import java.awt.image.BufferedImage;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.function.Consumer;
+import javax.swing.Icon;
+import javax.swing.filechooser.FileSystemView;
+import jdk.jpackage.test.PackageTest;
+import jdk.jpackage.test.Annotations.Test;
+import jdk.jpackage.test.JPackageCommand;
+import jdk.jpackage.test.PackageType;
+import static jdk.jpackage.test.RunnablePackageTest.Action.CREATE;
+import jdk.jpackage.test.TKit;
+
+/**
+ * Test that --icon also changes icon of exe installer.
+ */
+
+/*
+ * @test
+ * @summary jpackage with --icon parameter for exe installer
+ * @library ../helpers
+ * @key jpackagePlatformPackage
+ * @build jdk.jpackage.test.*
+ * @build WinInstallerIconTest
+ * @requires (os.family == "windows")
+ * @modules jdk.jpackage/jdk.jpackage.internal
+ * @run main/othervm/timeout=360 -Xmx512m  jdk.jpackage.test.Main
+ *  --jpt-run=WinInstallerIconTest
+ */
+public class WinInstallerIconTest {
+
+    @Test
+    public void test() throws IOException {
+        Path customIcon = iconPath("icon");
+
+        BufferedImage[] defaultInstallerIconImg = new BufferedImage[1];
+
+        // Create installer with the default icon
+        createInstaller(null, "WithDefaultIcon", installerIconImg -> {
+            defaultInstallerIconImg[0] = installerIconImg;
+        }, null, null);
+
+        BufferedImage[] customInstallerIconImg = new BufferedImage[1];
+
+        // Create installer with custom icon.
+        // This installer icon should differ from the icon
+        // of the installer created with the default icon.
+        createInstaller(customIcon, "2", installerIconImg -> {
+            customInstallerIconImg[0] = installerIconImg;
+        }, null, defaultInstallerIconImg[0]);
+
+        // Create installer with custom icon again.
+        // This installer icon should differ from the icon
+        // of the installer created with the default icon and should have
+        // the same icon as the icon of installer created with custom icon.
+        createInstaller(customIcon, null, null,
+                customInstallerIconImg[0], defaultInstallerIconImg[0]);
+    }
+
+    private void createInstaller(Path icon, String nameSuffix,
+            Consumer<BufferedImage> installerIconImgConsumer,
+            BufferedImage expectedInstallerIconImg,
+            BufferedImage unexpectedInstallerIconImg) throws IOException {
+
+        PackageTest test = new PackageTest()
+                .forTypes(PackageType.WIN_EXE)
+                .addInitializer(JPackageCommand::setFakeRuntime)
+                .configureHelloApp();
+        if (icon != null) {
+            test.addInitializer(cmd -> cmd.addArguments("--icon", icon));
+        }
+
+        if (nameSuffix != null) {
+            test.addInitializer(cmd -> {
+                String name = cmd.name() + nameSuffix;
+                cmd.setArgumentValue("--name", name);
+            });
+        }
+
+        Path installerExePath[] = new Path[1];
+
+        test.addBundleVerifier(cmd -> {
+            installerExePath[0] = cmd.outputBundle();
+
+            Icon actualIcon = FileSystemView.getFileSystemView().getSystemIcon(
+                    installerExePath[0].toFile());
+
+            BufferedImage actualInstallerIconImg = loadIcon(actualIcon);
+
+            if (installerIconImgConsumer != null) {
+                installerIconImgConsumer.accept(actualInstallerIconImg);
+            }
+
+            if (expectedInstallerIconImg != null) {
+                TKit.assertTrue(imageEquals(expectedInstallerIconImg,
+                        actualInstallerIconImg), String.format(
+                                "Check icon of %s installer is matching expected value",
+                                installerExePath[0]));
+            }
+
+            if (unexpectedInstallerIconImg != null) {
+                TKit.assertFalse(imageEquals(unexpectedInstallerIconImg,
+                        actualInstallerIconImg), String.format(
+                                "Check icon of %s installer is NOT matching unexpected value",
+                                installerExePath[0]));
+            }
+        });
+
+        test.run(CREATE);
+
+        if (installerExePath[0] != null && nameSuffix != null) {
+            TKit.deleteIfExists(installerExePath[0]);
+        }
+    }
+
+    private BufferedImage loadIcon(Icon icon) {
+        TKit.assertNotEquals(0, icon.getIconWidth(),
+                "Check icon has not empty width");
+        TKit.assertNotEquals(0, icon.getIconHeight(),
+                "Check icon has not empty height");
+        BufferedImage img = new BufferedImage(
+                icon.getIconWidth(),
+                icon.getIconHeight(),
+                BufferedImage.TYPE_INT_RGB);
+        Graphics g = img.createGraphics();
+        icon.paintIcon(null, g, 0, 0);
+        g.dispose();
+        return img;
+    }
+
+    private static boolean imageEquals(BufferedImage imgA, BufferedImage imgB) {
+        if (imgA.getWidth() == imgB.getWidth() && imgA.getHeight()
+                == imgB.getHeight()) {
+            for (int x = 0; x < imgA.getWidth(); x++) {
+                for (int y = 0; y < imgA.getHeight(); y++) {
+                    if (imgA.getRGB(x, y) != imgB.getRGB(x, y)) {
+                        return false;
+                    }
+                }
+            }
+        } else {
+            return false;
+        }
+        return true;
+    }
+
+    private static Path iconPath(String name) {
+        return TKit.TEST_SRC_ROOT.resolve(Path.of("resources", name
+                + TKit.ICON_SUFFIX));
+    }
+}