From 560ef1701dbb02acacc40976b1bd4023888a24ff Mon Sep 17 00:00:00 2001 From: Jeanette Winzenburg Date: Mon, 6 Apr 2020 03:57:19 +0000 Subject: [PATCH 01/13] 8241455: Memory leak on replacing selection/focusModel Reviewed-by: arapte, aghaisas --- .../java/javafx/scene/control/ChoiceBox.java | 22 +- .../java/javafx/scene/control/TabPane.java | 9 +- .../javafx/scene/control/TreeTableView.java | 9 +- .../java/javafx/scene/control/TreeView.java | 18 +- .../SelectionFocusModelMemoryTest.java | 297 ++++++++++++++++++ 5 files changed, 337 insertions(+), 18 deletions(-) create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/SelectionFocusModelMemoryTest.java diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java index 55faf26a25a..a865beeb958 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2020, 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 @@ -29,9 +29,11 @@ import javafx.beans.property.ObjectPropertyBase; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.value.ChangeListener; +import javafx.beans.value.WeakChangeListener; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; +import javafx.collections.WeakListChangeListener; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.ReadOnlyBooleanWrapper; import javafx.event.ActionEvent; @@ -503,6 +505,9 @@ public void hide() { // package for testing static class ChoiceBoxSelectionModel extends SingleSelectionModel { private final ChoiceBox choiceBox; + private ChangeListener> itemsObserver; + private ListChangeListener itemsContentObserver; + private WeakListChangeListener weakItemsContentObserver; public ChoiceBoxSelectionModel(final ChoiceBox cb) { if (cb == null) { @@ -520,7 +525,7 @@ public ChoiceBoxSelectionModel(final ChoiceBox cb) { */ // watching for changes to the items list content - final ListChangeListener itemsContentObserver = c -> { + itemsContentObserver = c -> { if (choiceBox.getItems() == null || choiceBox.getItems().isEmpty()) { setSelectedIndex(-1); } else if (getSelectedIndex() == -1 && getSelectedItem() != null) { @@ -530,17 +535,18 @@ public ChoiceBoxSelectionModel(final ChoiceBox cb) { } } }; + weakItemsContentObserver = new WeakListChangeListener<>(itemsContentObserver); if (this.choiceBox.getItems() != null) { - this.choiceBox.getItems().addListener(itemsContentObserver); + this.choiceBox.getItems().addListener(weakItemsContentObserver); } // watching for changes to the items list - ChangeListener> itemsObserver = (valueModel, oldList, newList) -> { + itemsObserver = (valueModel, oldList, newList) -> { if (oldList != null) { - oldList.removeListener(itemsContentObserver); + oldList.removeListener(weakItemsContentObserver); } if (newList != null) { - newList.addListener(itemsContentObserver); + newList.addListener(weakItemsContentObserver); } setSelectedIndex(-1); if (getSelectedItem() != null) { @@ -550,7 +556,9 @@ public ChoiceBoxSelectionModel(final ChoiceBox cb) { } } }; - this.choiceBox.itemsProperty().addListener(itemsObserver); + // TBD: use pattern as f.i. in listView selectionModel (invalidationListener to really + // get all changes - including list of same content - of the list-valued property) + this.choiceBox.itemsProperty().addListener(new WeakChangeListener<>(itemsObserver)); } // API Implementation diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java index 3180afbfc0d..4cf54c4f055 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2020, 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 @@ -41,6 +41,7 @@ import javafx.beans.value.WritableValue; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; +import javafx.collections.WeakListChangeListener; import javafx.geometry.Side; import javafx.scene.AccessibleAttribute; import javafx.scene.AccessibleRole; @@ -672,6 +673,8 @@ public StyleableProperty getStyleableProperty(TabPane n) { static class TabPaneSelectionModel extends SingleSelectionModel { private final TabPane tabPane; + private ListChangeListener itemsContentObserver; + public TabPaneSelectionModel(final TabPane t) { if (t == null) { throw new NullPointerException("TabPane can not be null"); @@ -679,7 +682,7 @@ public TabPaneSelectionModel(final TabPane t) { this.tabPane = t; // watching for changes to the items list content - final ListChangeListener itemsContentObserver = c -> { + itemsContentObserver = c -> { while (c.next()) { for (Tab tab : c.getRemoved()) { if (tab != null && !tabPane.getTabs().contains(tab)) { @@ -710,7 +713,7 @@ public TabPaneSelectionModel(final TabPane t) { } }; if (this.tabPane.getTabs() != null) { - this.tabPane.getTabs().addListener(itemsContentObserver); + this.tabPane.getTabs().addListener(new WeakListChangeListener<>(itemsContentObserver)); } } diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java index af4798ced41..feae614d865 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2020, 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 @@ -3406,12 +3406,13 @@ public TreeTableViewFocusModel(final TreeTableView treeTableView) { TreeTablePosition pos = new TreeTablePosition<>(treeTableView, focusRow, null); setFocusedCell(pos); - treeTableView.showRootProperty().addListener(o -> { + showRootListener = obs -> { if (isFocused(0)) { focus(-1); focus(0); } - }); + }; + treeTableView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener)); focusedCellProperty().addListener(o -> { treeTableView.notifyAccessibleAttributeChanged(AccessibleAttribute.FOCUS_ITEM); @@ -3425,6 +3426,8 @@ public TreeTableViewFocusModel(final TreeTableView treeTableView) { private final WeakChangeListener> weakRootPropertyListener = new WeakChangeListener<>(rootPropertyListener); + private final InvalidationListener showRootListener; + private void updateTreeEventListener(TreeItem oldRoot, TreeItem newRoot) { if (oldRoot != null && weakTreeItemListener != null) { oldRoot.removeEventHandler(TreeItem.expandedItemCountChangeEvent(), weakTreeItemListener); diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java index 0c474a80a2f..a239bf242e3 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2020, 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 @@ -32,6 +32,8 @@ import javafx.application.Platform; import javafx.beans.DefaultProperty; +import javafx.beans.InvalidationListener; +import javafx.beans.WeakInvalidationListener; import javafx.beans.property.BooleanProperty; import javafx.beans.property.DoubleProperty; import javafx.beans.property.ObjectProperty; @@ -1295,9 +1297,10 @@ public TreeViewBitSetSelectionModel(final TreeView treeView) { this.treeView = treeView; this.treeView.rootProperty().addListener(weakRootPropertyListener); - this.treeView.showRootProperty().addListener(o -> { + showRootListener = o -> { shiftSelection(0, treeView.isShowRoot() ? 1 : -1, null); - }); + }; + this.treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener)); updateTreeEventListener(null, treeView.getRoot()); @@ -1310,6 +1313,7 @@ private void updateTreeEventListener(TreeItem oldRoot, TreeItem newRoot) { } if (newRoot != null) { + //PENDING why create a new weak eventHandler? weakTreeItemListener = new WeakEventHandler<>(treeItemListener); newRoot.addEventHandler(TreeItem.expandedItemCountChangeEvent(), weakTreeItemListener); } @@ -1456,6 +1460,7 @@ private void updateTreeEventListener(TreeItem oldRoot, TreeItem newRoot) { private WeakEventHandler> weakTreeItemListener; + private InvalidationListener showRootListener; /*********************************************************************** @@ -1595,12 +1600,13 @@ public TreeViewFocusModel(final TreeView treeView) { focus(0); } - treeView.showRootProperty().addListener(o -> { + showRootListener = obs -> { if (isFocused(0)) { focus(-1); focus(0); } - }); + }; + treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener)); focusedIndexProperty().addListener(o -> { treeView.notifyAccessibleAttributeChanged(AccessibleAttribute.FOCUS_ITEM); @@ -1614,6 +1620,8 @@ public TreeViewFocusModel(final TreeView treeView) { private final WeakChangeListener> weakRootPropertyListener = new WeakChangeListener<>(rootPropertyListener); + private final InvalidationListener showRootListener; + private void updateTreeEventListener(TreeItem oldRoot, TreeItem newRoot) { if (oldRoot != null && weakTreeItemListener != null) { oldRoot.removeEventHandler(TreeItem.expandedItemCountChangeEvent(), weakTreeItemListener); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/SelectionFocusModelMemoryTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/SelectionFocusModelMemoryTest.java new file mode 100644 index 00000000000..f0bba4a8df1 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/SelectionFocusModelMemoryTest.java @@ -0,0 +1,297 @@ +/* + * Copyright (c) 2020, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ + +package test.javafx.scene.control; + +import java.lang.ref.WeakReference; +import java.util.Arrays; +import java.util.Collection; + +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.junit.Assert.*; + +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; +import javafx.scene.Scene; +import javafx.scene.control.ChoiceBox; +import javafx.scene.control.ChoiceBoxShim; +import javafx.scene.control.ComboBox; +import javafx.scene.control.ComboBoxShim; +import javafx.scene.control.Control; +import javafx.scene.control.FocusModel; +import javafx.scene.control.ListView; +import javafx.scene.control.ListViewShim; +import javafx.scene.control.MultipleSelectionModel; +import javafx.scene.control.SelectionModel; +import javafx.scene.control.SingleSelectionModel; +import javafx.scene.control.Tab; +import javafx.scene.control.TabPane; +import javafx.scene.control.TabPaneShim; +import javafx.scene.control.TableView; +import javafx.scene.control.TableView.TableViewFocusModel; +import javafx.scene.control.TableView.TableViewSelectionModel; +import javafx.scene.control.TableViewShim; +import javafx.scene.control.TreeItem; +import javafx.scene.control.TreeTableView; +import javafx.scene.control.TreeTableView.TreeTableViewFocusModel; +import javafx.scene.control.TreeTableView.TreeTableViewSelectionModel; +import javafx.scene.control.TreeTableViewShim; +import javafx.scene.control.TreeView; +import javafx.scene.control.TreeViewShim; +import javafx.scene.layout.Pane; +import javafx.scene.layout.VBox; +import javafx.stage.Stage; + +/** + * Testing for potential memory leaks in xxSelectionModel and xxFocusModel ( + * https://bugs.openjdk.java.net/browse/JDK-8241455). + * Might happen, when the concrete selection/focusModel registers strong listeners on any of the + * control's properties. + *

+ * Parameterized in not/showing the control before replacing the model (aka: + * no/skin that might have a reference to any property of the old model as well). + * Note that failing/passing tests with skin reveal the mis/behavior on part on + * the skin - added here for convenience (and because it is simple). + * + */ +@RunWith(Parameterized.class) +public class SelectionFocusModelMemoryTest { + private Scene scene; + private Stage stage; + private Pane root; + + private boolean showBeforeReplaceSM; + +//---------- focusModel + + @Test + public void testTreeViewFocusModel() { + TreeItem root = new TreeItem<>("root"); + ObservableList data = FXCollections.observableArrayList("Apple", "Orange", "Banana"); + data.forEach(text -> root.getChildren().add(new TreeItem<>(text))); + TreeView control = new TreeView<>(root); + WeakReference> weakRef = new WeakReference<>(control.getFocusModel()); + FocusModel> replacingSm = TreeViewShim.get_TreeViewFocusModel(control); + maybeShowControl(control); + control.setFocusModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("focusModel must be gc'ed", weakRef.get()); + } + + @Test + public void testTreeTableViewFocusModel() { + TreeItem root = new TreeItem<>("root"); + ObservableList data = FXCollections.observableArrayList("Apple", "Orange", "Banana"); + data.forEach(text -> root.getChildren().add(new TreeItem<>(text))); + TreeTableView control = new TreeTableView<>(root); + WeakReference> weakRef = new WeakReference<>(control.getFocusModel()); + TreeTableViewFocusModel replacingSm = new TreeTableViewFocusModel<>(control); + maybeShowControl(control); + control.setFocusModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("focusModel must be gc'ed", weakRef.get()); + } + + @Test + public void testTableViewFocusModel() { + TableView control = new TableView<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getFocusModel()); + TableViewFocusModel replacingSm = new TableViewFocusModel<>(control); + maybeShowControl(control); + control.setFocusModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("focusModel must be gc'ed", weakRef.get()); + } + + @Test + public void testListViewFocusModel() { + ListView control = new ListView<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getFocusModel()); + FocusModel replacingSm = ListViewShim.getListViewFocusModel(control); + maybeShowControl(control); + control.setFocusModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("focusModel must be gc'ed", weakRef.get()); + } + +//------------------------ selectionModel + + @Test + public void testTreeViewSelectionModel() { + TreeItem root = new TreeItem<>("root"); + ObservableList data = FXCollections.observableArrayList("Apple", "Orange", "Banana"); + data.forEach(text -> root.getChildren().add(new TreeItem<>(text))); + TreeView control = new TreeView<>(root); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + MultipleSelectionModel> replacingSm = TreeViewShim.get_TreeViewBitSetSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testTreeTableViewSelectionModel() { + TreeItem root = new TreeItem<>("root"); + ObservableList data = FXCollections.observableArrayList("Apple", "Orange", "Banana"); + data.forEach(text -> root.getChildren().add(new TreeItem<>(text))); + TreeTableView control = new TreeTableView<>(root); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + TreeTableViewSelectionModel replacingSm = (TreeTableViewSelectionModel) TreeTableViewShim.get_TreeTableViewArrayListSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testTableViewSelectionModel() { + TableView control = new TableView<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + TableViewSelectionModel replacingSm = TableViewShim.get_TableViewArrayListSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testListViewSelectionModel() { + ListView control = new ListView<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + MultipleSelectionModel replacingSm = ListViewShim.getListViewBitSetSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testTabPaneSelectionModel() { + // FIXME + // can't formally ignore just one parameter, so backing out if showBeforeReplaceSM + if (showBeforeReplaceSM) return; //@Ignore("8241737") + TabPane control = new TabPane(); + ObservableList data = FXCollections.observableArrayList("Apple", "Orange", "Banana"); + data.forEach(text -> control.getTabs().add(new Tab(text))); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + SingleSelectionModel replacingSm = TabPaneShim.getTabPaneSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testComboBoxSelectionModel() { + ComboBox control = new ComboBox<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + SingleSelectionModel replacingSm = ComboBoxShim.get_ComboBoxSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + @Test + public void testChoiceBoxSelectionModel() { + // FIXME + // can't formally ignore just one parameter, so backing out if showBeforeReplaceSM + // will be fixed as side-effect of skin cleanup + if (showBeforeReplaceSM) return; //@Ignore("8087555") + ChoiceBox control = new ChoiceBox<>(FXCollections.observableArrayList("Apple", "Orange", "Banana")); + WeakReference> weakRef = new WeakReference<>(control.getSelectionModel()); + SingleSelectionModel replacingSm = ChoiceBoxShim.get_ChoiceBoxSelectionModel(control); + maybeShowControl(control); + control.setSelectionModel(replacingSm); + attemptGC(weakRef, 10); + assertNull("selectionModel must be gc'ed", weakRef.get()); + } + + private void attemptGC(WeakReference weakRef, int n) { + // Attempt gc n times + for (int i = 0; i < n; i++) { + System.gc(); + System.runFinalization(); + + if (weakRef.get() == null) { + break; + } + try { + Thread.sleep(500); + } catch (InterruptedException e) { + System.err.println("InterruptedException occurred during Thread.sleep()"); + } + } + } + + protected void maybeShowControl(Control control) { + if (!showBeforeReplaceSM) return; + show(control); + } + +// ------------- parameterized + + @Parameterized.Parameters + public static Collection data() { + // show the control before replacing the selectionModel + Object[][] data = new Object[][] { + {false}, + {true }, + }; + return Arrays.asList(data); + } + + public SelectionFocusModelMemoryTest(boolean showBeforeReplaceSM) { + this.showBeforeReplaceSM = showBeforeReplaceSM; + } + +//------------------ setup + + private void show(Control node) { + if (root == null) { + root = new VBox(); + scene = new Scene(root); + stage = new Stage(); + stage.setScene(scene); + } + root.getChildren().add(node); + if (!stage.isShowing()) { + stage.show(); + } + } + + @After + public void cleanup() { + if (stage != null) { + stage.hide(); + } + } + +} From 247a65d2aad0332af0e35fa41f0aaaa8ced89cc2 Mon Sep 17 00:00:00 2001 From: Kevin Rushforth Date: Mon, 6 Apr 2020 15:10:58 +0000 Subject: [PATCH 02/13] 8236971: [macos] Gestures handled incorrectly due to missing events Reviewed-by: mpaus, arapte --- .../src/main/native-glass/mac/GlassView2D.m | 20 ++--- .../src/main/native-glass/mac/GlassView3D.m | 20 ++--- .../main/native-glass/mac/GlassViewDelegate.h | 15 +++- .../main/native-glass/mac/GlassViewDelegate.m | 74 ++++++++++++++++++- 4 files changed, 97 insertions(+), 32 deletions(-) diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassView2D.m b/modules/javafx.graphics/src/main/native-glass/mac/GlassView2D.m index c6878103819..e79f78e6b8a 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassView2D.m +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassView2D.m @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2020, 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 @@ -215,32 +215,22 @@ - (void)otherMouseUp:(NSEvent *)theEvent - (void)rotateWithEvent:(NSEvent *)theEvent { - [self->delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_ROTATE]; + [self->delegate doRotateWithEvent:theEvent]; } - (void)swipeWithEvent:(NSEvent *)theEvent { - [self->delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_SWIPE]; + [self->delegate doSwipeWithEvent:theEvent]; } - (void)magnifyWithEvent:(NSEvent *)theEvent { - [self->delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_MAGNIFY]; -} - -- (void)endGestureWithEvent:(NSEvent *)theEvent -{ - [self->delegate sendJavaGestureEndEvent:theEvent]; -} - -- (void)beginGestureWithEvent:(NSEvent *)theEvent -{ - [self->delegate sendJavaGestureBeginEvent:theEvent]; + [self->delegate doMagnifyWithEvent:theEvent]; } - (void)scrollWheel:(NSEvent *)theEvent { - [self->delegate sendJavaMouseEvent:theEvent]; + [self->delegate doScrollWheel:theEvent]; } - (BOOL)performKeyEquivalent:(NSEvent *)theEvent diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassView3D.m b/modules/javafx.graphics/src/main/native-glass/mac/GlassView3D.m index b4ccd9c1a3c..9001f78d91d 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassView3D.m +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassView3D.m @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2020, 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 @@ -441,33 +441,23 @@ - (void)otherMouseUp:(NSEvent *)theEvent - (void)rotateWithEvent:(NSEvent *)theEvent { - [self->_delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_ROTATE]; + [self->_delegate doRotateWithEvent:theEvent]; } - (void)swipeWithEvent:(NSEvent *)theEvent { - [self->_delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_SWIPE]; + [self->_delegate doSwipeWithEvent:theEvent]; } - (void)magnifyWithEvent:(NSEvent *)theEvent { - [self->_delegate sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_MAGNIFY]; -} - -- (void)endGestureWithEvent:(NSEvent *)theEvent -{ - [self->_delegate sendJavaGestureEndEvent:theEvent]; -} - -- (void)beginGestureWithEvent:(NSEvent *)theEvent -{ - [self->_delegate sendJavaGestureBeginEvent:theEvent]; + [self->_delegate doMagnifyWithEvent:theEvent]; } - (void)scrollWheel:(NSEvent *)theEvent { MOUSELOG("scrollWheel"); - [self->_delegate sendJavaMouseEvent:theEvent]; + [self->_delegate doScrollWheel:theEvent]; } - (BOOL)performKeyEquivalent:(NSEvent *)theEvent diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.h b/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.h index 5a06a290a38..88bda213f5d 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.h +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2020, 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 @@ -31,6 +31,14 @@ #import "GlassDragSource.h" #import "GlassAccessible.h" +// Bit mask for tracking gesture begin / end +typedef enum GestureMaskType { + GESTURE_MASK_SCROLL = 1 << 0, + GESTURE_MASK_SWIPE = 1 << 1, + GESTURE_MASK_ROTATE = 1 << 2, + GESTURE_MASK_MAGNIFY = 1 << 3, +} GestureMaskType; + // helper class that implements the custom GlassView functionality @interface GlassViewDelegate : NSObject { @@ -51,6 +59,7 @@ int mouseDownMask; // bit 0 - left, 1 - right, 2 - other button BOOL gestureInProgress; + GestureMaskType gesturesBeganMask; NSEvent *lastEvent; @@ -84,6 +93,10 @@ - (void)sendJavaGestureEvent:(NSEvent *)theEvent type:(int)type; - (void)sendJavaGestureBeginEvent:(NSEvent *)theEvent; - (void)sendJavaGestureEndEvent:(NSEvent *)theEvent; +- (void)doRotateWithEvent:(NSEvent *)theEvent; +- (void)doSwipeWithEvent:(NSEvent *)theEvent; +- (void)doMagnifyWithEvent:(NSEvent *)theEvent; +- (void)doScrollWheel:(NSEvent *)theEvent; - (NSDragOperation)sendJavaDndEvent:(id )info type:(jint)type; diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m b/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m index 004bf8aacc0..54da01544eb 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2020, 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 @@ -69,6 +69,11 @@ // Tracks pressed modifier keys static NSUInteger s_modifierFlags = 0; +@interface GlassViewDelegate (hidden) +- (void)maybeBeginGestureWithEvent:(NSEvent *)theEvent withMask:(GestureMaskType)theMask; +- (void)maybeEndGestureWithEvent:(NSEvent *)theEvent withMask:(GestureMaskType)theMask; +@end + // Extracted from class-dump utility output for NSEvent class @interface NSEvent (hidden) @@ -150,6 +155,7 @@ - (id)initWithView:(NSView*)view withJview:(jobject)jview self->mouseDownMask = 0; self->gestureInProgress = NO; + self->gesturesBeganMask = 0; self->nativeFullScreenModeWindow = nil; @@ -845,6 +851,72 @@ - (void)sendJavaGestureEndEvent:(NSEvent *)theEvent GLASS_CHECK_EXCEPTION(env); } +/* + * This method is a replacement for the deprecated beginGestureWithEvent + * method, which is no longer delivered to a View by macOS. This + * is called for each gesture event to track the beginning of a + * gesture using the phase of the event. We call sendJavaGestureBeginEvent + * if there are no other gestures active. + */ +- (void)maybeBeginGestureWithEvent:(NSEvent *)theEvent withMask:(GestureMaskType)theMask +{ + NSEventPhase phase = [theEvent phase]; + if (phase == NSEventPhaseBegan) { + if (gesturesBeganMask == 0) { + [self sendJavaGestureBeginEvent:theEvent]; + } + gesturesBeganMask |= theMask; + } +} + +/* + * This method is a replacement for the deprecated endGestureWithEvent + * method, which is no longer delivered to a View by macOS. This + * is called for each gesture event to track the end of a + * gesture using the phase of the event. We call sendJavaGestureEndEvent + * if there are no other gestures active. + */ +- (void)maybeEndGestureWithEvent:(NSEvent *)theEvent withMask:(GestureMaskType)theMask +{ + NSEventPhase phase = [theEvent phase]; + if (phase == NSEventPhaseEnded || phase == NSEventPhaseCancelled) { + if ((gesturesBeganMask & theMask) != 0) { + gesturesBeganMask &= ~theMask; + if (gesturesBeganMask == 0) { + [self sendJavaGestureEndEvent:theEvent]; + } + } + } +} + +- (void)doRotateWithEvent:(NSEvent *)theEvent +{ + [self maybeBeginGestureWithEvent:theEvent withMask:GESTURE_MASK_ROTATE]; + [self sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_ROTATE]; + [self maybeEndGestureWithEvent:theEvent withMask:GESTURE_MASK_ROTATE]; +} + +- (void)doSwipeWithEvent:(NSEvent *)theEvent +{ + [self maybeBeginGestureWithEvent:theEvent withMask:GESTURE_MASK_SWIPE]; + [self sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_SWIPE]; + [self maybeEndGestureWithEvent:theEvent withMask:GESTURE_MASK_SWIPE]; +} + +- (void)doMagnifyWithEvent:(NSEvent *)theEvent +{ + [self maybeBeginGestureWithEvent:theEvent withMask:GESTURE_MASK_MAGNIFY]; + [self sendJavaGestureEvent:theEvent type:com_sun_glass_ui_mac_MacGestureSupport_GESTURE_MAGNIFY]; + [self maybeEndGestureWithEvent:theEvent withMask:GESTURE_MASK_MAGNIFY]; +} + +- (void)doScrollWheel:(NSEvent *)theEvent +{ + [self maybeBeginGestureWithEvent:theEvent withMask:GESTURE_MASK_SCROLL]; + [self sendJavaMouseEvent:theEvent]; + [self maybeEndGestureWithEvent:theEvent withMask:GESTURE_MASK_SCROLL]; +} + - (NSDragOperation)sendJavaDndEvent:(id )info type:(jint)type { GET_MAIN_JENV; From 418675a249d6a64baaca8be3e21184641fa4fcb1 Mon Sep 17 00:00:00 2001 From: Ambarish Rapte Date: Tue, 7 Apr 2020 01:55:58 +0000 Subject: [PATCH 03/13] 8236840: Memory leak when switching ButtonSkin Reviewed-by: fastegal, kcr --- .../javafx/scene/control/skin/ButtonSkin.java | 49 +++--- .../scene/control/skin/ButtonSkinTest.java | 160 +++++++++++++++++- 2 files changed, 189 insertions(+), 20 deletions(-) diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java index a03131fdb63..24a483aec39 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2020, 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 @@ -28,6 +28,8 @@ import com.sun.javafx.scene.NodeHelper; import com.sun.javafx.scene.control.behavior.BehaviorBase; import com.sun.javafx.scene.control.skin.Utils; +import javafx.beans.value.ChangeListener; +import javafx.beans.value.WeakChangeListener; import javafx.scene.Scene; import javafx.scene.control.Button; import javafx.scene.control.ContextMenu; @@ -78,6 +80,25 @@ public class ButtonSkin extends LabeledSkinBase