8350976: MenuBarSkin: exception initializing in a background thread

Reviewed-by: lkostyra, jdv

Author: Andy Goryachev

modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2025, 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
@@ -35,7 +35,7 @@
 import java.util.Optional;
 import java.util.WeakHashMap;
 import java.util.stream.Collectors;
-
+import javafx.application.Platform;
 import javafx.beans.InvalidationListener;
 import javafx.beans.property.DoubleProperty;
 import javafx.beans.property.ObjectProperty;
@@ -77,7 +77,7 @@
 import javafx.stage.Stage;
 import javafx.stage.Window;
 import javafx.util.Pair;
-
+import javafx.util.Subscription;
 import com.sun.javafx.menu.MenuBase;
 import com.sun.javafx.scene.ParentHelper;
 import com.sun.javafx.scene.SceneHelper;
@@ -130,7 +130,7 @@ public class MenuBarSkin extends SkinBase<MenuBar> {
     private WeakChangeListener<Boolean> weakMenuVisibilityChangeListener;
     private ListenerHelper sceneListenerHelper;
     private IDisconnectable windowFocusHelper;
-
+    private volatile Subscription windowSubscription;
     private boolean pendingDismiss = false;
     private boolean altKeyPressed = false;
 
@@ -229,17 +229,36 @@ public MenuBarSkin(final MenuBar control) {
 
         ListenerHelper lh = ListenerHelper.get(this);
 
+        if (Platform.isFxApplicationThread()) {
+            if (Toolkit.getToolkit().getSystemMenu().isSupported()) {
+                lh.addInvalidationListener(control.useSystemMenuBarProperty(), (v) -> {
+                    rebuildUI();
+                });
+            }
+        } else {
+            // delay rebuildUI() until after MenuBar becomes a part of the scene graph
+            // this subscription will be removed by cleanUpListeners()
+            windowSubscription = getSkinnable()
+                .sceneProperty()
+                .flatMap(Scene::windowProperty)
+                .subscribe(w -> {
+                    if (w != null) {
+                        if (Toolkit.getToolkit().getSystemMenu().isSupported()) {
+                            lh.addInvalidationListener(control.useSystemMenuBarProperty(), (v) -> {
+                                rebuildUI();
+                            });
+                        }
+                        // this method will unsubscribe on first run
+                        rebuildUI();
+                    }
+                });
+        }
+
         rebuildUI();
         lh.addListChangeListener(control.getMenus(), (v) -> {
             rebuildUI();
         });
 
-        if (Toolkit.getToolkit().getSystemMenu().isSupported()) {
-            lh.addInvalidationListener(control.useSystemMenuBarProperty(), (v) -> {
-                rebuildUI();
-            });
-        }
-
         // When the mouse leaves the menu, the last hovered item should lose
         // it's focus so that it is no longer selected. This code returns focus
         // to the MenuBar itself, such that keyboard navigation can continue.
@@ -783,6 +802,12 @@ private void updateActionListeners(MenuItem item, boolean add) {
     }
 
     private void cleanUpListeners() {
+        Subscription sub = windowSubscription;
+        if (sub != null) {
+            sub.unsubscribe();
+            windowSubscription = null;
+        }
+
         getSkinnable().focusedProperty().removeListener(weakMenuBarFocusedPropertyListener);
 
         for (Menu m : getSkinnable().getMenus()) {
@@ -817,6 +842,9 @@ private void cleanUpListeners() {
     }
 
     private void rebuildUI() {
+        if (!Platform.isFxApplicationThread()) {
+            return;
+        }
         cleanUpListeners();
 
         if (Toolkit.getToolkit().getSystemMenu().isSupported()) {

tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java

@@ -87,6 +87,8 @@
 import javafx.scene.control.Hyperlink;
 import javafx.scene.control.Label;
 import javafx.scene.control.ListView;
+import javafx.scene.control.Menu;
+import javafx.scene.control.MenuBar;
 import javafx.scene.control.MenuButton;
 import javafx.scene.control.MenuItem;
 import javafx.scene.control.Pagination;
@@ -124,6 +126,7 @@
 import javafx.scene.control.skin.HyperlinkSkin;
 import javafx.scene.control.skin.LabelSkin;
 import javafx.scene.control.skin.ListViewSkin;
+import javafx.scene.control.skin.MenuBarSkin;
 import javafx.scene.control.skin.MenuButtonSkin;
 import javafx.scene.control.skin.PaginationSkin;
 import javafx.scene.control.skin.ProgressIndicatorSkin;
@@ -209,7 +212,6 @@
  *
  * Notable exceptions to this rule:
  * - HTMLEditor
- * - MenuBar
  * - WebView
  *
  * The test creates a visible node on the JavaFX application thread, and at the same time,
@@ -744,6 +746,25 @@ public void mediaView() {
         });
     }
 
+    @Test
+    public void menuBar() {
+        assumeFalse(SKIP_TEST);
+        test(() -> {
+            MenuBar c = new MenuBar();
+            // exercise skin installation and disposal code paths
+            c.setSkin(new MenuBarSkin(c));
+            c.setSkin(new MenuBarSkin(c));
+            c.setUseSystemMenuBar(nextBoolean());
+            return c;
+        }, (c) -> {
+            c.getMenus().setAll(new Menu("MenuBar"));
+            accessControl(c);
+            if (Platform.isFxApplicationThread()) {
+                c.setUseSystemMenuBar(nextBoolean());
+            }
+        });
+    }
+
     @Test
     public void menuButton() {
         assumeFalse(SKIP_TEST);